Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(linter): change no-unused-vars to nursery #4588

Merged
merged 1 commit into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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