Skip to content

Commit

Permalink
Fix handling of empty documents (FuelLabs#3930)
Browse files Browse the repository at this point in the history
Closes FuelLabs#3923 

The name here is slightly misleading. This does clean up how we handle
it to a degree, but mostly due to a conflict of what is considered
documentable. We don't have html rendering for `ImplTrait`s just yet,
and because they were considered documentable but not handled we were
getting a panic.

While working this out, I found another bug: FuelLabs#3929

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
eureka-cpu and JoshuaBatty authored Jan 30, 2023
1 parent ac06168 commit f6616ed
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 54 deletions.
55 changes: 28 additions & 27 deletions forc-plugins/forc-doc/src/descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,33 +202,34 @@ impl Descriptor {
raw_attributes: attrs_opt,
}))
}
ImplTrait(ref decl_id) => {
// TODO: figure out how to use this, likely we don't want to document this directly.
//
// This declaration type may make more sense to document as part of another declaration
// much like how we document method functions for traits or fields on structs.
let impl_trait = decl_engine.get_impl_trait(decl_id.clone(), &decl_id.span())?;
let item_name = impl_trait.trait_name.suffix;
Ok(Descriptor::Documentable(Document {
module_info: module_info.clone(),
item_header: ItemHeader {
module_info: module_info.clone(),
friendly_name: ty_decl.friendly_name(),
item_name: item_name.clone(),
},
item_body: ItemBody {
module_info,
ty_decl: ty_decl.clone(),
item_name,
code_str: parse::parse_format::<sway_ast::ItemImpl>(
impl_trait.span.as_str(),
),
attrs_opt: None, // no attributes field
item_context: ItemContext { context: None },
},
raw_attributes: None,
}))
}
// Uncomment this when we decide how to handle ImplTraits
// ImplTrait(ref decl_id) => {
// TODO: figure out how to use this, likely we don't want to document this directly.
//
// This declaration type may make more sense to document as part of another declaration
// much like how we document method functions for traits or fields on structs.
// let impl_trait = decl_engine.get_impl_trait(decl_id.clone(), &decl_id.span())?;
// let item_name = impl_trait.trait_name.suffix;
// Ok(Descriptor::Documentable(Document {
// module_info: module_info.clone(),
// item_header: ItemHeader {
// module_info: module_info.clone(),
// friendly_name: ty_decl.friendly_name(),
// item_name: item_name.clone(),
// },
// item_body: ItemBody {
// module_info,
// ty_decl: ty_decl.clone(),
// item_name,
// code_str: parse::parse_format::<sway_ast::ItemImpl>(
// impl_trait.span.as_str(),
// ),
// attrs_opt: None, // no attributes field
// item_context: ItemContext { context: None },
// },
// raw_attributes: None,
// }))
// }
FunctionDeclaration(ref decl_id) => {
let fn_decl = decl_engine.get_function(decl_id.clone(), &decl_id.span())?;
if !document_private_items && fn_decl.visibility.is_private() {
Expand Down
2 changes: 1 addition & 1 deletion forc-plugins/forc-doc/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl Renderable for Document {
}
}
pub(crate) type ModulePrefix = String;
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq)]
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
pub(crate) struct ModuleInfo(pub(crate) Vec<ModulePrefix>);
impl ModuleInfo {
/// The current module.
Expand Down
53 changes: 27 additions & 26 deletions forc-plugins/forc-doc/src/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl RenderedDocumentation {
module_map.insert(location.clone(), doc_links);
}
}
// create links to child modules
// Create links to child modules.
if let Some(parent_module) = doc.module_info.parent() {
let module_link = DocLink {
name: location.clone(),
Expand All @@ -153,7 +153,7 @@ impl RenderedDocumentation {
}
}
}
// above we check for the module a link belongs to, here we want _all_ links so the check is much more shallow
// Above we check for the module a link belongs to, here we want _all_ links so the check is much more shallow.
match doc.item_body.ty_decl {
StructDeclaration(_) => match all_docs.links.get_mut(&BlockTitle::Structs) {
Some(links) => links.push(doc.link()),
Expand Down Expand Up @@ -231,31 +231,32 @@ impl RenderedDocumentation {

// ModuleIndex(s)
for (_, doc_links) in module_map {
let module_info = match doc_links.values().last() {
Some(doc_links) => match doc_links.first() {
Some(doc_link) => doc_link.module_info.clone(),
None => panic!("document is empty"),
},
None => panic!("document is empty"),
let module_info_opt = match doc_links.values().last() {
Some(doc_links) => doc_links
.first()
.map(|doc_link| doc_link.module_info.clone()),
// No module to be documented
None => None,
};
rendered_docs.0.push(RenderedDocument {
module_info: module_info.clone(),
html_filename: INDEX_FILENAME.to_string(),
file_contents: HTMLString::from(
ModuleIndex {
version_opt: None,
module_info,
module_docs: DocLinks {
style: DocStyle::ModuleIndex,
links: doc_links.to_owned(),
},
}
.render(),
),
})
if let Some(module_info) = module_info_opt {
rendered_docs.0.push(RenderedDocument {
module_info: module_info.clone(),
html_filename: INDEX_FILENAME.to_string(),
file_contents: HTMLString::from(
ModuleIndex {
version_opt: None,
module_info,
module_docs: DocLinks {
style: DocStyle::ModuleIndex,
links: doc_links.to_owned(),
},
}
.render(),
),
})
}
}
}

// AllDocIndex
rendered_docs.0.push(RenderedDocument {
module_info: root_module.clone(),
Expand Down Expand Up @@ -717,7 +718,7 @@ impl Renderable for TyTraitFn {
}
}
/// Used for creating links between docs.
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq)]
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
pub(crate) struct DocLink {
pub(crate) name: String,
pub(crate) module_info: ModuleInfo,
Expand Down Expand Up @@ -829,7 +830,7 @@ impl Renderable for DocLinks {
}
/// Represents all of the possible titles
/// belonging to an index or sidebar.
#[derive(Clone, Ord, PartialOrd, Eq, PartialEq)]
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
enum BlockTitle {
Modules,
Structs,
Expand Down

0 comments on commit f6616ed

Please sign in to comment.