fix(expand): show references to ModuleDefId recursing on parents

tooling/nargo_cli/build.rs

@@ -119,7 +119,7 @@
 ];
 
 /// `nargo interpret` ignored tests, either because they don't currently work or
 /// becuase they are too slow to run.
 const IGNORED_INTERPRET_EXECUTION_TESTS: [&str; 1] = [
     // slow
     "regression_4709",
@@ -151,7 +151,7 @@
 /// might not be worth it.
 /// Others are ignored because of existing bugs in `nargo expand`.
 /// As the bugs are fixed these tests should be removed from this list.
-const IGNORED_NARGO_EXPAND_EXECUTION_TESTS: [&str; 9] = [
+const IGNORED_NARGO_EXPAND_EXECUTION_TESTS: [&str; 6] = [
     // There's nothing special about this program but making it work with a custom entry would involve
     // having to parse the Nargo.toml file, etc., which is not worth it
     "custom_entry",
@@ -160,12 +160,6 @@
     // There's no "src/main.nr" here so it's trickier to make this work
     "overlapping_dep_and_mod",
     // bug
-    "poseidonsponge_x5_254",
-    // bug
-    "regression_5045",
-    // bug
-    "regression_7744",
-    // bug
     "trait_associated_constant",
     // There's no "src/main.nr" here so it's trickier to make this work
     "workspace",
@@ -190,7 +184,8 @@
     "overlapping_dep_and_mod",
     // bug
     "primitive_trait_method_call_multiple_candidates",
-    // bug
+    // this one works, but copying its `Nargo.toml` file to somewhere else doesn't work
+    // because it references another project by a relative path
     "reexports",
     // bug
     "regression_7038",
@@ -811,7 +806,7 @@
     writeln!(test_file, "}}").unwrap();
 }
 
 /// Here we check, for every program in `test_programs/exeuction_success`, that:
 /// 1. `nargo expand` works on it
 /// 2. That the output of the original program is the same as the output of the expanded program
 ///    (that is, we run `nargo execute` on the original program and the expanded program and compare the output)

tooling/nargo_cli/src/cli/expand_cmd/printer.rs

@@ -15,10 +15,7 @@ use noirc_frontend::{
         stmt::{HirLetStatement, HirPattern},
         traits::{ResolvedTraitBound, TraitConstraint},
     },
-    modules::{
-        get_parent_module, module_def_id_is_visible, module_def_id_to_reference_id,
-        relative_module_full_path,
-    },
+    modules::{get_parent_module, module_def_id_is_visible, module_def_id_to_reference_id},
     node_interner::{FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TypeAliasId},
     shared::Visibility,
     token::{FunctionAttributeKind, LocatedToken, SecondaryAttribute, SecondaryAttributeKind},
@@ -1049,8 +1046,6 @@ impl<'context, 'string> ItemPrinter<'context, 'string> {
             }
         }
 
-        let mut reexport = None;
-
         let is_visible = module_def_id_is_visible(
             module_def_id,
             self.module_id,
@@ -1061,63 +1056,33 @@ impl<'context, 'string> ItemPrinter<'context, 'string> {
             self.dependencies,
         );
         if !is_visible {
-            reexport = self.interner.get_reexports(module_def_id).first();
-
-            // If we can't find a reexport for the main item, check if the parent module
-            // has a reexport, then use the item through that reexported module
-            if reexport.is_none() {
-                if let Some(module_def_id_parent_module) =
-                    get_parent_module(self.interner, module_def_id)
-                {
-                    let module_def_id_parent = ModuleDefId::ModuleId(module_def_id_parent_module);
-                    let visibility = self.module_def_id_visibility(module_def_id_parent);
-                    self.show_reference_to_module_def_id(
-                        module_def_id_parent,
-                        visibility,
-                        use_import,
-                    );
-                    self.push_str("::");
-                    let name = self.module_def_id_name(module_def_id);
-                    self.push_str(&name);
-                    return name;
-                }
+            if let Some(reexport) = self.interner.get_reexports(module_def_id).first() {
+                self.show_reference_to_module_def_id(
+                    ModuleDefId::ModuleId(reexport.module_id),
+                    reexport.visibility,
+                    true,
+                );
+                self.push_str("::");
+                self.push_str(reexport.name.as_str());
+                return reexport.name.to_string();
             }
         }
 
-        if let Some(reexport) = reexport {
-            self.show_reference_to_module_def_id(
-                ModuleDefId::ModuleId(reexport.module_id),
-                reexport.visibility,
-                true,
-            );
-            self.push_str("::");
-            self.push_str(reexport.name.as_str());
-            return reexport.name.to_string();
-        }
-
-        if let Some(full_path) = relative_module_full_path(
-            module_def_id,
-            self.module_id,
-            current_module_parent_id,
-            self.interner,
-        ) {
-            if !full_path.is_empty() {
-                // `relative_module_full_path` for a module returns the full path to that module
-                // so we need to remove the last segment
-                if matches!(module_def_id, ModuleDefId::ModuleId(..)) {
-                    let mut full_path = full_path.split("::").collect::<Vec<_>>();
-                    full_path.pop();
-                    let full_path = full_path.join("::");
-                    if !full_path.is_empty() {
-                        self.push_str(&full_path);
-                        self.push_str("::");
-                    }
-                } else {
-                    self.push_str(&full_path);
-                    self.push_str("::");
-                }
+        // Recurse on the parent module, but only if the parent module isn't the current module
+        // (if so, we can already reach the definition just by printing its name)
+        let module_def_id_parent_module = get_parent_module(self.interner, module_def_id);
+        if module_def_id_parent_module != Some(self.module_id) {
+            if let Some(module_def_id_parent_module) = module_def_id_parent_module {
+                let visibility = self
+                    .module_def_id_visibility(ModuleDefId::ModuleId(module_def_id_parent_module));
+                self.show_reference_to_module_def_id(
+                    ModuleDefId::ModuleId(module_def_id_parent_module),
+                    visibility,
+                    use_import,
+                );
+                self.push_str("::");
             }
-        };
+        }
 
         let name = self.module_def_id_name(module_def_id);
         self.push_str(&name);