fix(doc): analyze sub-modules imports before self

tooling/nargo_cli/src/cli/doc_cmd.rs

@@ -46,25 +46,25 @@ pub(crate) fn run(args: DocCommand, workspace: Workspace) -> Result<(), CliError
 
     let mut crates = Vec::new();
     // Maps a crate's root file to its crate
-    let mut all_dependencies = HashMap::new();
+    let mut dependencies = HashMap::new();
     let mut ids = HashMap::new();
     for package in &workspace {
-        let (krate, dependencies) = package_crate(
+        let krate = package_crate(
             &workspace_file_manager,
             &parsed_files,
             package,
             &args.compile_options,
             &mut ids,
+            &mut dependencies,
         )?;
         crates.push(krate);
-        all_dependencies.extend(dependencies);
     }
     // Crates in the workspace might depend on other crates in the workspace.
     // Remove them from `all_dependencies`.
     for krate in &crates {
-        all_dependencies.remove(&krate.root_file);
+        dependencies.remove(&krate.root_file);
     }
-    let dependencies = all_dependencies.into_values().collect::<Vec<_>>();
+    let dependencies = dependencies.into_values().collect::<Vec<_>>();
 
     let name = workspace.root_dir.file_name().unwrap().to_string_lossy().to_string();
     let crates = nargo_doc::items::Workspace { crates, name, dependencies };
@@ -97,24 +97,24 @@ fn package_crate(
     package: &Package,
     compile_options: &CompileOptions,
     ids: &mut ItemIds,
-) -> Result<(Crate, HashMap<String, Crate>), CompileError> {
+    dependencies: &mut HashMap<String, Crate>,
+) -> Result<Crate, CompileError> {
     let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package);
 
     check_crate_and_report_errors(&mut context, crate_id, compile_options)?;
 
     let module =
         crate_module(crate_id, &context.crate_graph, &context.def_maps, &context.def_interner, ids);
 
-    let mut dependencies = HashMap::new();
-    collect_dependencies(&context, Some(package), crate_id, file_manager, &mut dependencies, ids)?;
+    collect_dependencies(&context, Some(package), crate_id, file_manager, dependencies, ids)?;
 
     let root_file = &context.crate_graph[crate_id].root_file_id;
     let root_file = file_manager.path(*root_file).unwrap().display().to_string();
 
     let name = package.name.to_string();
     let version = package.version.clone();
     let krate = Crate { name, version, root_module: module, root_file };
-    Ok((krate, dependencies))
+    Ok(krate)
 }
 
 fn collect_dependencies(
@@ -130,6 +130,10 @@ fn collect_dependencies(
         let crate_data = &context.crate_graph[crate_id];
         let root_file = crate_data.root_file_id;
         let root_file = file_manager.path(root_file).unwrap().display().to_string();
+        if dependencies.contains_key(&root_file) {
+            continue;
+        }
+
         let module = crate_module(
             crate_id,
             &context.crate_graph,

tooling/nargo_doc/src/html.rs

@@ -234,6 +234,7 @@ impl HTMLCreator {
         if !sidebar {
             self.render_reexports(items);
         }
+        self.render_contracts(items, sidebar, nesting);
         self.render_modules(items, sidebar, nesting);
         self.render_structs(items, sidebar, nesting);
         self.render_traits(items, sidebar, nesting);
@@ -250,6 +251,18 @@ impl HTMLCreator {
         }
     }
 
+    fn render_contracts(
+        &mut self,
+        items: &[(ItemVisibility, Item)],
+        sidebar: bool,
+        nesting: usize,
+    ) {
+        let modules = get_contracts(items);
+        if !modules.is_empty() {
+            self.render_list("Contracts", "contracts", sidebar, nesting, &modules);
+        }
+    }
+
     fn render_modules(&mut self, items: &[(ItemVisibility, Item)], sidebar: bool, nesting: usize) {
         let modules = get_modules(items);
         if !modules.is_empty() {
@@ -429,12 +442,14 @@ impl HTMLCreator {
     fn create_module(&mut self, parent_module: &Module, module: &Module) {
         self.current_path.push(module.name.clone());
 
-        self.html_start(&format!("Module {}", module.name));
+        let kind = if module.is_contract { "Contract" } else { "Module" };
+
+        self.html_start(&format!("{kind} {}", module.name));
         self.sidebar_start();
         self.render_module_sidebar(parent_module, module);
         self.sidebar_end();
         self.main_start(false);
-        self.h1(&format!("Module <span id=\"mod\" class=\"module\">{}</span>", module.name));
+        self.h1(&format!("{kind} <span id=\"mod\" class=\"module\">{}</span>", module.name));
         self.render_comments(&module.comments, 1);
         self.render_items(&module.items, false, 0);
         self.main_end();
@@ -447,8 +462,9 @@ impl HTMLCreator {
     }
 
     fn render_module_sidebar(&mut self, parent_module: &Module, module: &Module) {
-        self.h2(&format!("<a href=\"#mod\">Module {}</a>", module.name));
-        self.render_module_items_sidebar("Module items", module);
+        let kind = if module.is_contract { "Contract" } else { "Module" };
+        self.h2(&format!("<a href=\"#mod\">{kind} {}</a>", module.name));
+        self.render_module_items_sidebar(&format!("{kind} items"), module);
         self.render_module_contents_sidebar(parent_module, 1);
     }
 
@@ -461,6 +477,9 @@ impl HTMLCreator {
         if !get_reexports(&module.items).is_empty() {
             self.output.push_str("<li><a href=\"#re-exports\">Re-exports</a></li>\n");
         }
+        if !get_contracts(&module.items).is_empty() {
+            self.output.push_str("<li><a href=\"#contracts\">Contracts</a></li>\n");
+        }
         if !get_modules(&module.items).is_empty() {
             self.output.push_str("<li><a href=\"#modules\">Modules</a></li>\n");
         }
@@ -1490,7 +1509,23 @@ fn get_modules(items: &[(ItemVisibility, Item)]) -> Vec<&Module> {
         .filter_map(|(visibility, item)| {
             if visibility == &ItemVisibility::Public {
                 if let Item::Module(module) = item {
-                    if module.has_public_items() {
+                    if !module.is_contract && module.has_public_items() {
+                        return Some(module);
+                    }
+                }
+            }
+            None
+        })
+        .collect()
+}
+
+fn get_contracts(items: &[(ItemVisibility, Item)]) -> Vec<&Module> {
+    items
+        .iter()
+        .filter_map(|(visibility, item)| {
+            if visibility == &ItemVisibility::Public {
+                if let Item::Module(module) = item {
+                    if module.is_contract && module.has_public_items() {
                         return Some(module);
                     }
                 }

tooling/nargo_doc/src/items.rs

@@ -78,6 +78,7 @@ pub struct Module {
     pub name: String,
     pub items: Vec<(ItemVisibility, Item)>,
     pub comments: Option<String>,
+    pub is_contract: bool,
 }
 
 impl Module {

tooling/nargo_doc/src/lib.rs

@@ -48,7 +48,7 @@ struct DocItemBuilder<'a> {
     /// Maps a ModuleDefId to the item it converted to.
     /// This is needed because if an item is publicly exported, but the item
     /// isn't publicly visible (because its parent module is private) then we'll
-    /// include the item directly undert the module that publicly exports it.
+    /// include the item directly under the module that publicly exports it.
     /// We do this by looking up the item in this map.
     module_def_id_to_item: HashMap<ModuleDefId, ConvertedItem>,
     module_imports: HashMap<ModuleId, Vec<expand_items::Import>>,
@@ -287,8 +287,9 @@ impl DocItemBuilder<'_> {
             .map(|(visibility, item)| (visibility, self.convert_item(item, visibility)))
             .collect();
         self.module_imports.insert(module.id, module.imports);
+        let is_contract = module.is_contract;
         let id = self.get_module_id(module.id);
-        Module { id, module_id: module.id, name, comments, items }
+        Module { id, module_id: module.id, name, comments, items, is_contract }
     }
 
     fn convert_impl(&mut self, impl_: expand_items::Impl) -> Impl {
@@ -577,6 +578,15 @@ impl DocItemBuilder<'_> {
     /// Goes over a module's imports. If an import is a re-export of a private item,
     /// the item is added to the module's items.
     fn process_module_reexports(&mut self, module: &mut Module) {
+        // Process this module's sub-modules first because when we actually process the
+        // imports we might add a publicly exported module into this module's items,
+        // visiting it twice.
+        for (_visibility, item) in &mut module.items {
+            if let Item::Module(sub_module) = item {
+                self.process_module_reexports(sub_module);
+            }
+        }
+
         let imports = self.module_imports.remove(&module.module_id).unwrap();
         for import in imports {
             if import.visibility == ItemVisibility::Private {
@@ -607,12 +617,6 @@ impl DocItemBuilder<'_> {
                 }),
             ));
         }
-
-        for (_visibility, item) in &mut module.items {
-            if let Item::Module(sub_module) = item {
-                self.process_module_reexports(sub_module);
-            }
-        }
     }
 
     fn doc_comments(&self, id: ReferenceId) -> Option<String> {

tooling/nargo_doc/src/styles.css

@@ -73,7 +73,7 @@ h2 {
     border-bottom: 1px solid var(--header-border-color);
 }
 
-ul {
+ul.item-list, ul.sidebar-list {
     list-style-type: none;
     width: 100%;
     margin: 0;