Skip to content

Commit

Permalink
fix(linter): change no-unused-vars to nursery (#4588)
Browse files Browse the repository at this point in the history
Also sets `^_` as the default `varsIgnorePattern` unless a configuration object is provided.
  • Loading branch information
DonIsaac committed Jul 31, 2024
1 parent b58ed80 commit fe1356d
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 11 deletions.
6 changes: 3 additions & 3 deletions apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ declare_oxc_lint!(
/// var global_var = 42;
/// ```
NoUnusedVars,
correctness
nursery
);

impl Deref for NoUnusedVars {
Expand Down
56 changes: 49 additions & 7 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 `^_`:
Expand All @@ -37,13 +40,16 @@ 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,

/// 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 `^_`:
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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 `^_`:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -390,6 +417,9 @@ impl From<Value> 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<Regex> =
parse_unicode_rule(config.get("varsIgnorePattern"), "varsIgnorePattern");

Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit fe1356d

Please sign in to comment.