From 9d776d46c3a8efeef6db1521d879f1731cc2bd01 Mon Sep 17 00:00:00 2001 From: connorshea <2977353+connorshea@users.noreply.github.com> Date: Thu, 22 Jan 2026 15:36:47 +0000 Subject: [PATCH] fix(linter): Update `import/no-cycle` rule to error on invalid config options. (#18330) This requires removing some test cases that used invalid configurations and were relying on the fallback to default values. This feels like a more correct behavior. --- .../oxc_linter/src/rules/import/no_cycle.rs | 40 +++++++++++------- .../src/snapshots/import_no_cycle.snap | 42 ------------------- 2 files changed, 25 insertions(+), 57 deletions(-) diff --git a/crates/oxc_linter/src/rules/import/no_cycle.rs b/crates/oxc_linter/src/rules/import/no_cycle.rs index ce909947bc57f..7323bf2b931c2 100644 --- a/crates/oxc_linter/src/rules/import/no_cycle.rs +++ b/crates/oxc_linter/src/rules/import/no_cycle.rs @@ -63,7 +63,7 @@ fn format_cycle(stack: &[(CompactStr, PathBuf)], cwd: &Path) -> String { // #[derive(Debug, Clone, JsonSchema, Deserialize)] -#[serde(rename_all = "camelCase", default)] +#[serde(rename_all = "camelCase", default, deny_unknown_fields)] pub struct NoCycle { /// Maximum dependency depth to traverse max_depth: u32, @@ -91,8 +91,8 @@ declare_oxc_lint!( /// /// Ensures that there is no resolvable path back to this module via its dependencies. /// - /// This includes cycles of depth 1 (imported module imports me) to "∞" (or Infinity), - /// if the `maxDepth` option is not set. + /// This includes cycles of depth 1 (imported module imports me) to an effectively + /// infinite value, if the `maxDepth` option is not set. /// /// ### Why is this bad? /// @@ -136,9 +136,7 @@ declare_oxc_lint!( impl Rule for NoCycle { fn from_configuration(value: serde_json::Value) -> Result { - Ok(serde_json::from_value::>(value) - .unwrap_or_default() - .into_inner()) + serde_json::from_value::>(value).map(DefaultRuleConfig::into_inner) } fn run_once(&self, ctx: &LintContext<'_>) { @@ -329,8 +327,13 @@ fn test() { (r#"import one, { two, three } from "./es6/depth-three-star""#, None), (r#"import { bar } from "./es6/depth-three-indirect""#, None), (r#"import { bar } from "./es6/depth-three-indirect""#, None), - (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":null}]))), - (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":"∞"}]))), + // effectively unlimited: + (r#"import { foo } from "./es6/depth-two""#, None), + // Use default value, effectively unlimited: + (r#"import { foo } from "./es6/depth-two""#, Some(json!([]))), + // These are not valid config options and just fell back to the default value previously: + // (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":null}]))), + // (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":"∞"}]))), ( r#"import { foo } from "./es6/depth-one""#, Some(json!([{"allowUnsafeDynamicCyclicDependency":true}])), @@ -384,19 +387,26 @@ fn test() { r#"import { bar } from "./es6/depth-three-indirect""#, Some(json!([{"allowUnsafeDynamicCyclicDependency":true}])), ), + // Equivalent to the commented tests below. ( r#"import { foo } from "./es6/depth-two""#, - Some(json!([{"allowUnsafeDynamicCyclicDependency":true,"maxDepth":null}])), - ), - ( - r#"import { foo } from "./es6/depth-two""#, - Some(json!([{"allowUnsafeDynamicCyclicDependency":true,"maxDepth":"∞"}])), + Some(json!([{"allowUnsafeDynamicCyclicDependency":true}])), ), + // These are not valid config options and just fell back to the default value previously: + // ( + // r#"import { foo } from "./es6/depth-two""#, + // Some(json!([{"allowUnsafeDynamicCyclicDependency":true,"maxDepth":null}])), + // ), + // ( + // r#"import { foo } from "./es6/depth-two""#, + // Some(json!([{"allowUnsafeDynamicCyclicDependency":true,"maxDepth":"∞"}])), + // ), // TODO: dynamic import // (r#"import("./es6/depth-three-star")"#, None), // (r#"import("./es6/depth-three-indirect")"#, None), - (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":null}]))), - (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":"∞"}]))), + // These are not valid config options and just fell back to the default value previously: + // (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":null}]))), + // (r#"import { foo } from "./es6/depth-two""#, Some(json!([{"maxDepth":"∞"}]))), // TODO: dynamic import // (r#"function bar(){ return import("./es6/depth-one"); } // #2265 5"#, None), // (r#"import { foo } from "./es6/depth-one-dynamic"; // #2265 6"#, None), diff --git a/crates/oxc_linter/src/snapshots/import_no_cycle.snap b/crates/oxc_linter/src/snapshots/import_no_cycle.snap index aa5dd04233de8..8902539dfd788 100644 --- a/crates/oxc_linter/src/snapshots/import_no_cycle.snap +++ b/crates/oxc_linter/src/snapshots/import_no_cycle.snap @@ -299,48 +299,6 @@ source: crates/oxc_linter/src/tester.rs │ ../depth-zero (fixtures/import/cycles/depth-zero.js) ╰─────────╯ imports the current file - ⚠ eslint-plugin-import(no-cycle): Dependency cycle detected - ╭─[cycles/depth-zero.js:1:21] - 1 │ import { foo } from "./es6/depth-two" - · ───────────────── - ╰──── - help: Refactor to remove the cycle. Consider extracting shared code into a separate module that both files can import. - note: These paths form a cycle: - ╭──▶ ./es6/depth-two (fixtures/import/cycles/es6/depth-two.js) - │ ⬇ imports - │ ./depth-one (fixtures/import/cycles/es6/depth-one.js) - │ ⬇ imports - │ ../depth-zero (fixtures/import/cycles/depth-zero.js) - ╰─────────╯ imports the current file - - ⚠ eslint-plugin-import(no-cycle): Dependency cycle detected - ╭─[cycles/depth-zero.js:1:21] - 1 │ import { foo } from "./es6/depth-two" - · ───────────────── - ╰──── - help: Refactor to remove the cycle. Consider extracting shared code into a separate module that both files can import. - note: These paths form a cycle: - ╭──▶ ./es6/depth-two (fixtures/import/cycles/es6/depth-two.js) - │ ⬇ imports - │ ./depth-one (fixtures/import/cycles/es6/depth-one.js) - │ ⬇ imports - │ ../depth-zero (fixtures/import/cycles/depth-zero.js) - ╰─────────╯ imports the current file - - ⚠ eslint-plugin-import(no-cycle): Dependency cycle detected - ╭─[cycles/depth-zero.js:1:21] - 1 │ import { foo } from "./es6/depth-two" - · ───────────────── - ╰──── - help: Refactor to remove the cycle. Consider extracting shared code into a separate module that both files can import. - note: These paths form a cycle: - ╭──▶ ./es6/depth-two (fixtures/import/cycles/es6/depth-two.js) - │ ⬇ imports - │ ./depth-one (fixtures/import/cycles/es6/depth-one.js) - │ ⬇ imports - │ ../depth-zero (fixtures/import/cycles/depth-zero.js) - ╰─────────╯ imports the current file - ⚠ eslint-plugin-import(no-cycle): Dependency cycle detected ╭─[cycles/depth-zero.js:1:21] 1 │ import { foo } from "./intermediate-ignore"