Disable cyclical module imports (#6477)

crates/nu-parser/src/errors.rs

@@ -120,6 +120,10 @@ pub enum ParseError {
     )]
     ModuleNotFound(#[label = "module not found"] Span),
 
+    #[error("Cyclical module import.")]
+    #[diagnostic(code(nu::parser::cyclical_module_import), url(docsrs), help("{0}"))]
+    CyclicalModuleImport(String, #[label = "detected cyclical module import"] Span),
+
     #[error("Active overlay not found.")]
     #[diagnostic(code(nu::parser::active_overlay_not_found), url(docsrs))]
     ActiveOverlayNotFound(#[label = "not an active overlay"] Span),
@@ -341,6 +345,7 @@ impl ParseError {
             ParseError::VariableNotFound(s) => *s,
             ParseError::VariableNotValid(s) => *s,
             ParseError::ModuleNotFound(s) => *s,
+            ParseError::CyclicalModuleImport(_, s) => *s,
             ParseError::ModuleOrOverlayNotFound(s) => *s,
             ParseError::ActiveOverlayNotFound(s) => *s,
             ParseError::OverlayPrefixMismatch(_, _, s) => *s,

crates/nu-parser/src/parse_keywords.rs

@@ -1660,8 +1660,8 @@ pub fn parse_use(
         if let Some(module_id) = working_set.find_module(&import_pattern.head.name) {
             (import_pattern, working_set.get_module(module_id).clone())
         } else {
-            // TODO: Do not close over when loading module from file?
             // It could be a file
+            // TODO: Do not close over when loading module from file?
 
             let (module_filename, err) =
                 unescape_unquote_string(&import_pattern.head.name, import_pattern.head.span);
@@ -1670,6 +1670,37 @@ pub fn parse_use(
                 if let Some(module_path) =
                     find_in_dirs(&module_filename, working_set, &cwd, LIB_DIRS_ENV)
                 {
+                    if let Some(i) = working_set
+                        .parsed_module_files
+                        .iter()
+                        .rposition(|p| p == &module_path)
+                    {
+                        let mut files: Vec<String> = working_set
+                            .parsed_module_files
+                            .split_off(i)
+                            .iter()
+                            .map(|p| p.to_string_lossy().to_string())
+                            .collect();
+
+                        files.push(module_path.to_string_lossy().to_string());
+
+                        let msg = files.join("\nuses ");
+
+                        return (
+                            Pipeline::from_vec(vec![Expression {
+                                expr: Expr::Call(call),
+                                span: call_span,
+                                ty: Type::Any,
+                                custom_completion: None,
+                            }]),
+                            vec![],
+                            Some(ParseError::CyclicalModuleImport(
+                                msg,
+                                import_pattern.head.span,
+                            )),
+                        );
+                    }
+
                     let module_name = if let Some(stem) = module_path.file_stem() {
                         stem.to_string_lossy().to_string()
                     } else {
@@ -1690,7 +1721,7 @@ pub fn parse_use(
                         working_set.add_file(module_filename, &contents);
                         let span_end = working_set.next_span_start();
 
-                        // Change currently parsed directory
+                        // Change the currently parsed directory
                         let prev_currently_parsed_cwd = if let Some(parent) = module_path.parent() {
                             let prev = working_set.currently_parsed_cwd.clone();
 
@@ -1701,6 +1732,10 @@ pub fn parse_use(
                             working_set.currently_parsed_cwd.clone()
                         };
 
+                        // Add the file to the stack of parsed module files
+                        working_set.parsed_module_files.push(module_path);
+
+                        // Parse the module
                         let (block, module, err) = parse_module_block(
                             working_set,
                             Span::new(span_start, span_end),
@@ -1708,6 +1743,9 @@ pub fn parse_use(
                         );
                         error = error.or(err);
 
+                        // Remove the file from the stack of parsed module files
+                        working_set.parsed_module_files.pop();
+
                         // Restore the currently parsed directory back
                         working_set.currently_parsed_cwd = prev_currently_parsed_cwd;
 

crates/nu-protocol/src/engine/engine_state.rs

@@ -777,6 +777,8 @@ pub struct StateWorkingSet<'a> {
     pub type_scope: TypeScope,
     /// Current working directory relative to the file being parsed right now
     pub currently_parsed_cwd: Option<PathBuf>,
+    /// All previously parsed module files. Used to protect against circular imports.
+    pub parsed_module_files: Vec<PathBuf>,
 }
 
 /// A temporary placeholder for expression types. It is used to keep track of the input types
@@ -952,6 +954,7 @@ impl<'a> StateWorkingSet<'a> {
             external_commands: vec![],
             type_scope: TypeScope::default(),
             currently_parsed_cwd: None,
+            parsed_module_files: vec![],
         }
     }
 

tests/modules/mod.rs

@@ -341,3 +341,95 @@ fn module_import_env_2() {
         assert_eq!(actual.out, "foo");
     })
 }
+
+#[test]
+fn module_cyclical_imports_0() {
+    Playground::setup("module_cyclical_imports_0", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                    use eggs.nu
+                "#,
+        )]);
+
+        let inp = &[r#"module eggs { use spam.nu }"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("module not found"));
+    })
+}
+
+#[test]
+fn module_cyclical_imports_1() {
+    Playground::setup("module_cyclical_imports_1", |dirs, sandbox| {
+        sandbox.with_files(vec![FileWithContentToBeTrimmed(
+            "spam.nu",
+            r#"
+                    use spam.nu
+                "#,
+        )]);
+
+        let inp = &[r#"use spam.nu"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("cyclical"));
+    })
+}
+
+#[test]
+fn module_cyclical_imports_2() {
+    Playground::setup("module_cyclical_imports_2", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "spam.nu",
+                r#"
+                    use eggs.nu
+                "#,
+            )])
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "eggs.nu",
+                r#"
+                    use spam.nu
+                "#,
+            )]);
+
+        let inp = &[r#"use spam.nu"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("cyclical"));
+    })
+}
+
+#[test]
+fn module_cyclical_imports_3() {
+    Playground::setup("module_cyclical_imports_3", |dirs, sandbox| {
+        sandbox
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "spam.nu",
+                r#"
+                    use eggs.nu
+                "#,
+            )])
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "eggs.nu",
+                r#"
+                    use bacon.nu
+                "#,
+            )])
+            .with_files(vec![FileWithContentToBeTrimmed(
+                "bacon.nu",
+                r#"
+                    use spam.nu
+                "#,
+            )]);
+
+        let inp = &[r#"use spam.nu"#];
+
+        let actual = nu!(cwd: dirs.test(), pipeline(&inp.join("; ")));
+
+        assert!(actual.err.contains("cyclical"));
+    })
+}