From e88cf1bfd6d4df118138b38014975f13fb74710a Mon Sep 17 00:00:00 2001 From: camchenry <1514176+camchenry@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:37:45 +0000 Subject: [PATCH] fix(linter): make `overrides` globs relative to config path (#7407) - fixes https://github.com/oxc-project/oxc/issues/7365 currently, we are matching globs on the absolute path to the file, which means that in order to properly match, all globs would need to start with `**/`. this is not consistent with the behavior in ESLint, which is defined here: https://eslint.org/docs/v8.x/use/configure/configuration-files#how-do-overrides-work > **The patterns are applied against the file path relative to the directory of the config file.** For example, if your config file has the path `/Users/john/workspace/any-project/.eslintrc.js` and the file you want to lint has the path `/Users/john/workspace/any-project/lib/util.js`, then the pattern provided in `.eslintrc.js` is executed against the relative path `lib/util.js`. This PR adds the ability to store the path to the configuration file along with the configuration data, so that we can later use it to resolve the relative paths of the files in the overrides section. I'm not exactly sure if this will still make sense with nested configuration files, but I think it makes sense to store a path to each configuration file, alongside where the overrides are stored. --- .../overrides/directories-config.json | 19 ++++++++++++++++++ apps/oxlint/fixtures/overrides/lib/index.ts | 1 + .../fixtures/overrides/lib/tests/index.js | 1 + apps/oxlint/fixtures/overrides/src/oxlint.js | 1 + .../fixtures/overrides/src/tests/index.js | 1 + apps/oxlint/src/lint.rs | 9 +++++++++ crates/oxc_linter/src/builder.rs | 3 ++- crates/oxc_linter/src/config/flat.rs | 19 +++++++++++++++++- crates/oxc_linter/src/config/mod.rs | 6 ++++++ crates/oxc_linter/src/config/overrides.rs | 20 +++++++++++++++++++ crates/oxc_linter/src/config/oxlintrc.rs | 13 ++++++++++-- 11 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 apps/oxlint/fixtures/overrides/directories-config.json create mode 100644 apps/oxlint/fixtures/overrides/lib/index.ts create mode 100644 apps/oxlint/fixtures/overrides/lib/tests/index.js create mode 100644 apps/oxlint/fixtures/overrides/src/oxlint.js create mode 100644 apps/oxlint/fixtures/overrides/src/tests/index.js diff --git a/apps/oxlint/fixtures/overrides/directories-config.json b/apps/oxlint/fixtures/overrides/directories-config.json new file mode 100644 index 000000000..02c53eda8 --- /dev/null +++ b/apps/oxlint/fixtures/overrides/directories-config.json @@ -0,0 +1,19 @@ +{ + "rules": { + "no-debugger": "off" + }, + "overrides": [ + { + "files": ["lib/*.{js,ts}", "src/*"], + "rules": { + "no-debugger": "error" + } + }, + { + "files": ["**/tests/*"], + "rules": { + "no-debugger": "warn" + } + } + ] +} diff --git a/apps/oxlint/fixtures/overrides/lib/index.ts b/apps/oxlint/fixtures/overrides/lib/index.ts new file mode 100644 index 000000000..eab746921 --- /dev/null +++ b/apps/oxlint/fixtures/overrides/lib/index.ts @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/overrides/lib/tests/index.js b/apps/oxlint/fixtures/overrides/lib/tests/index.js new file mode 100644 index 000000000..eab746921 --- /dev/null +++ b/apps/oxlint/fixtures/overrides/lib/tests/index.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/overrides/src/oxlint.js b/apps/oxlint/fixtures/overrides/src/oxlint.js new file mode 100644 index 000000000..eab746921 --- /dev/null +++ b/apps/oxlint/fixtures/overrides/src/oxlint.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/fixtures/overrides/src/tests/index.js b/apps/oxlint/fixtures/overrides/src/tests/index.js new file mode 100644 index 000000000..eab746921 --- /dev/null +++ b/apps/oxlint/fixtures/overrides/src/tests/index.js @@ -0,0 +1 @@ +debugger; diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 78e2f1b8b..9cd252be2 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -695,4 +695,13 @@ mod test { assert_eq!(result.number_of_warnings, 0); assert_eq!(result.number_of_errors, 1); } + + #[test] + fn test_overrides_directories() { + let result = + test(&["-c", "fixtures/overrides/directories-config.json", "fixtures/overrides"]); + assert_eq!(result.number_of_files, 7); + assert_eq!(result.number_of_warnings, 2); + assert_eq!(result.number_of_errors, 2); + } } diff --git a/crates/oxc_linter/src/builder.rs b/crates/oxc_linter/src/builder.rs index 873c4179d..dfbe82727 100644 --- a/crates/oxc_linter/src/builder.rs +++ b/crates/oxc_linter/src/builder.rs @@ -94,9 +94,10 @@ impl LinterBuilder { categories, rules: oxlintrc_rules, overrides, + path, } = oxlintrc; - let config = LintConfig { plugins, settings, env, globals }; + let config = LintConfig { plugins, settings, env, globals, path: Some(path) }; let options = LintOptions::default(); let rules = if start_empty { FxHashSet::default() } else { Self::warn_correctness(plugins) }; diff --git a/crates/oxc_linter/src/config/flat.rs b/crates/oxc_linter/src/config/flat.rs index 1dfd48d59..dea2b9129 100644 --- a/crates/oxc_linter/src/config/flat.rs +++ b/crates/oxc_linter/src/config/flat.rs @@ -92,8 +92,24 @@ impl ConfigStore { let mut overrides_to_apply: Vec = Vec::new(); let mut hasher = FxBuildHasher.build_hasher(); + // Compute the path of the file relative to the configuration file for glob matching. Globs should match + // relative to the location of the configuration file. + // - path: /some/path/like/this/to/file.js + // - config_path: /some/path/like/.oxlintrc.json + // => relative_path: this/to/file.js + // TODO: Handle nested configuration file paths. + let relative_path = if let Some(config_path) = &self.base.config.path { + if let Some(parent) = config_path.parent() { + path.strip_prefix(parent).unwrap_or(path) + } else { + path + } + } else { + path + }; + for (id, override_config) in self.overrides.iter_enumerated() { - if override_config.files.is_match(path) { + if override_config.files.is_match(relative_path) { overrides_to_apply.push(id); id.hash(&mut hasher); } @@ -285,6 +301,7 @@ mod test { env: OxlintEnv::default(), settings: OxlintSettings::default(), globals: OxlintGlobals::default(), + path: None, }; let overrides = from_json!([{ "files": ["*.jsx", "*.tsx"], diff --git a/crates/oxc_linter/src/config/mod.rs b/crates/oxc_linter/src/config/mod.rs index cad1a00c7..54f012be7 100644 --- a/crates/oxc_linter/src/config/mod.rs +++ b/crates/oxc_linter/src/config/mod.rs @@ -8,6 +8,8 @@ mod plugins; mod rules; mod settings; +use std::path::PathBuf; + pub(crate) use self::flat::ResolvedLinterState; pub use self::{ env::OxlintEnv, @@ -29,6 +31,8 @@ pub(crate) struct LintConfig { pub(crate) env: OxlintEnv, /// Enabled or disabled specific global variables. pub(crate) globals: OxlintGlobals, + /// Absolute path to the configuration file (may be `None` if there is no file). + pub(crate) path: Option, } impl From for LintConfig { @@ -38,6 +42,7 @@ impl From for LintConfig { settings: config.settings, env: config.env, globals: config.globals, + path: Some(config.path), } } } @@ -58,6 +63,7 @@ mod test { let fixture_path = env::current_dir().unwrap().join("fixtures/eslint_config.json"); let config = Oxlintrc::from_file(&fixture_path).unwrap(); assert!(!config.rules.is_empty()); + assert!(config.path.ends_with("fixtures/eslint_config.json")); } #[test] diff --git a/crates/oxc_linter/src/config/overrides.rs b/crates/oxc_linter/src/config/overrides.rs index dc4ca11ac..6a6422258 100644 --- a/crates/oxc_linter/src/config/overrides.rs +++ b/crates/oxc_linter/src/config/overrides.rs @@ -153,6 +153,26 @@ impl JsonSchema for GlobSet { } mod test { + #[test] + fn test_globset() { + use super::*; + use serde_json::{from_value, json}; + + let config: OxlintOverride = from_value(json!({ + "files": ["*.tsx",], + })) + .unwrap(); + assert!(config.files.globs.is_match("/some/path/foo.tsx")); + assert!(!config.files.globs.is_match("/some/path/foo.ts")); + + let config: OxlintOverride = from_value(json!({ + "files": ["lib/*.ts",], + })) + .unwrap(); + assert!(config.files.globs.is_match("lib/foo.ts")); + assert!(!config.files.globs.is_match("src/foo.ts")); + } + #[test] fn test_parsing_plugins() { use super::*; diff --git a/crates/oxc_linter/src/config/oxlintrc.rs b/crates/oxc_linter/src/config/oxlintrc.rs index fc96ea1b2..1fc3f64e3 100644 --- a/crates/oxc_linter/src/config/oxlintrc.rs +++ b/crates/oxc_linter/src/config/oxlintrc.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; use oxc_diagnostics::OxcDiagnostic; use schemars::JsonSchema; @@ -85,6 +85,9 @@ pub struct Oxlintrc { /// Add, remove, or otherwise reconfigure rules for specific files or groups of files. #[serde(skip_serializing_if = "OxlintOverrides::is_empty")] pub overrides: OxlintOverrides, + /// Absolute path to the configuration file. + #[serde(skip)] + pub path: PathBuf, } impl Oxlintrc { @@ -116,10 +119,15 @@ impl Oxlintrc { OxcDiagnostic::error(format!("Failed to parse eslint config {path:?}.\n{err}")) })?; - let config = Self::deserialize(&json).map_err(|err| { + let mut config = Self::deserialize(&json).map_err(|err| { OxcDiagnostic::error(format!("Failed to parse config with error {err:?}")) })?; + // Get absolute path from `path` + let absolute_path = path.canonicalize().unwrap_or_else(|_| path.to_path_buf()); + + config.path = absolute_path; + Ok(config) } } @@ -137,6 +145,7 @@ mod test { assert!(config.rules.is_empty()); assert_eq!(config.settings, OxlintSettings::default()); assert_eq!(config.env, OxlintEnv::default()); + assert_eq!(config.path, PathBuf::default()); } #[test]