From fe1356d3a0de667a058a6cb297ac886de315a4e9 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Wed, 31 Jul 2024 16:21:23 +0000 Subject: [PATCH] fix(linter): change no-unused-vars to nursery (#4588) Also sets `^_` as the default `varsIgnorePattern` unless a configuration object is provided. --- apps/oxlint/src/lint/mod.rs | 6 +- .../src/rules/eslint/no_unused_vars/mod.rs | 2 +- .../rules/eslint/no_unused_vars/options.rs | 56 ++++++++++++++++--- 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/apps/oxlint/src/lint/mod.rs b/apps/oxlint/src/lint/mod.rs index e19d40bbbac73..dd78c771e3e02 100644 --- a/apps/oxlint/src/lint/mod.rs +++ b/apps/oxlint/src/lint/mod.rs @@ -427,7 +427,7 @@ mod test { ]; let result = test(args); assert_eq!(result.number_of_files, 1); - assert_eq!(result.number_of_warnings, 3); + assert_eq!(result.number_of_warnings, 2); assert_eq!(result.number_of_errors, 0); } @@ -441,7 +441,7 @@ mod test { ]; let result = test(args); assert_eq!(result.number_of_files, 1); - assert_eq!(result.number_of_warnings, 2); + assert_eq!(result.number_of_warnings, 1); assert_eq!(result.number_of_errors, 0); } @@ -477,7 +477,7 @@ mod test { let args = &["fixtures/svelte/debugger.svelte"]; let result = test(args); assert_eq!(result.number_of_files, 1); - assert_eq!(result.number_of_warnings, 2); + assert_eq!(result.number_of_warnings, 1); assert_eq!(result.number_of_errors, 0); } diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs index 5c236926a3d1c..f319e64720c13 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs @@ -136,7 +136,7 @@ declare_oxc_lint!( /// var global_var = 42; /// ``` NoUnusedVars, - correctness + nursery ); impl Deref for NoUnusedVars { diff --git a/crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs b/crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs index 8f666efc4a48e..8180502d98ce6 100644 --- a/crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs +++ b/crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs @@ -5,7 +5,7 @@ use regex::Regex; use serde_json::Value; /// See [ESLint - no-unused-vars config schema](https://github.com/eslint/eslint/blob/53b1ff047948e36682fade502c949f4e371e53cd/lib/rules/no-unused-vars.js#L61) -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] #[must_use] #[non_exhaustive] pub struct NoUnusedVarsOptions { @@ -21,6 +21,9 @@ pub struct NoUnusedVarsOptions { /// Specifies exceptions to this rule for unused variables. Variables whose /// names match this pattern will be ignored. /// + /// By default, this pattern is `^_` unless options are configured with an + /// object. In this case it will default to [`None`]. + /// /// ## Example /// /// Examples of **correct** code for this option when the pattern is `^_`: @@ -37,6 +40,7 @@ pub struct NoUnusedVarsOptions { /// 1. `after-used` - Unused positional arguments that occur before the last /// used argument will not be checked, but all named arguments and all /// positional arguments after the last used argument will be checked. + /// This is the default setting. /// 2. `all` - All named arguments must be used. /// 3. `none` - Do not check arguments. pub args: ArgsOption, @@ -44,6 +48,8 @@ pub struct NoUnusedVarsOptions { /// Specifies exceptions to this rule for unused arguments. Arguments whose /// names match this pattern will be ignored. /// + /// By default this pattern is [`None`]. + /// /// ## Example /// /// Examples of **correct** code for this option when the pattern is `^_`: @@ -60,6 +66,8 @@ pub struct NoUnusedVarsOptions { /// object, but by default the sibling properties are marked as "unused". /// With this option enabled the rest property's siblings are ignored. /// + /// By default this option is `false`. + /// /// ## Example /// Examples of **correct** code when this option is set to `true`: /// ```js @@ -72,10 +80,10 @@ pub struct NoUnusedVarsOptions { pub ignore_rest_siblings: bool, /// Used for `catch` block validation. - /// It has two settings: - /// * `none` - do not check error objects. This is the default setting - /// * `all` - all named arguments must be used` /// + /// It has two settings: + /// * `none` - do not check error objects. This is the default setting. + /// * `all` - all named arguments must be used`. #[doc(hidden)] /// `none` corresponds to `false`, while `all` corresponds to `true`. pub caught_errors: CaughtErrors, @@ -101,6 +109,8 @@ pub struct NoUnusedVarsOptions { /// not be checked for usage. Variables declared within array destructuring /// whose names match this pattern will be ignored. /// + /// By default this pattern is [`None`]. + /// /// ## Example /// /// Examples of **correct** code for this option, when the pattern is `^_`: @@ -192,6 +202,23 @@ pub struct NoUnusedVarsOptions { pub report_used_ignore_pattern: bool, } +impl Default for NoUnusedVarsOptions { + fn default() -> Self { + Self { + vars: VarsOption::default(), + vars_ignore_pattern: Some(Regex::new("^_").unwrap()), + args: ArgsOption::default(), + args_ignore_pattern: None, + ignore_rest_siblings: false, + caught_errors: CaughtErrors::default(), + caught_errors_ignore_pattern: None, + destructured_array_ignore_pattern: None, + ignore_class_with_static_init_block: false, + report_used_ignore_pattern: false, + } + } +} + #[derive(Debug, Default, Clone, PartialEq, Eq)] pub enum VarsOption { /// All variables are checked for usage, including those in the global scope. @@ -390,6 +417,9 @@ impl From for NoUnusedVarsOptions { }) .unwrap_or_default(); + // NOTE: when a configuration object is provided, do not provide + // a default ignore pattern here. They've opted into configuring + // this rule, and we'll give them full control over it. let vars_ignore_pattern: Option = parse_unicode_rule(config.get("varsIgnorePattern"), "varsIgnorePattern"); @@ -472,7 +502,7 @@ mod tests { fn test_options_default() { let rule = NoUnusedVarsOptions::default(); assert_eq!(rule.vars, VarsOption::All); - assert!(rule.vars_ignore_pattern.is_none()); + assert!(rule.vars_ignore_pattern.is_some_and(|v| v.as_str() == "^_")); assert_eq!(rule.args, ArgsOption::AfterUsed); assert!(rule.args_ignore_pattern.is_none()); assert_eq!(rule.caught_errors, CaughtErrors::all()); @@ -521,13 +551,25 @@ mod tests { assert!(rule.report_used_ignore_pattern); } + #[test] + fn test_options_from_sparse_object() { + let rule: NoUnusedVarsOptions = json!([ + { + "argsIgnorePattern": "^_", + } + ]) + .into(); + // option object provided, no default varsIgnorePattern + assert!(rule.vars_ignore_pattern.is_none()); + assert!(rule.args_ignore_pattern.unwrap().as_str() == "^_"); + } + #[test] fn test_options_from_null() { let opts = NoUnusedVarsOptions::from(json!(null)); let default = NoUnusedVarsOptions::default(); assert_eq!(opts.vars, default.vars); - assert!(opts.vars_ignore_pattern.is_none()); - assert!(default.vars_ignore_pattern.is_none()); + assert!(default.vars_ignore_pattern.unwrap().as_str() == "^_"); assert_eq!(opts.args, default.args); assert!(opts.args_ignore_pattern.is_none());