Skip to content

Commit

Permalink
feat(linter/eslint): show ignore patterns in eslint/no-unused-vars
Browse files Browse the repository at this point in the history
…diagnostic messages (oxc-project#6696)

Inform users what pattern an unused variable must match in diagnostic help messages.

```
  ⚠ eslint(no-unused-vars): Variable 'a' is declared but never used. Unused variables should start with a '_'.
   ╭─[no_unused_vars.tsx:1:5]
 1 │ var a=10
   ·     ┬
   ·     ╰── 'a' is declared here
   ╰────
  help: Consider removing this declaration.
```
  • Loading branch information
DonIsaac committed Oct 20, 2024
1 parent e9976d4 commit 01a35bb
Show file tree
Hide file tree
Showing 16 changed files with 266 additions and 209 deletions.
86 changes: 62 additions & 24 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,61 +1,95 @@
use std::fmt;

use cow_utils::CowUtils;
use oxc_diagnostics::OxcDiagnostic;
use oxc_semantic::SymbolFlags;
use oxc_span::{GetSpan, Span};

use super::Symbol;
use super::{options::IgnorePattern, Symbol};

fn pronoun_for_symbol(symbol_flags: SymbolFlags) -> &'static str {
fn pronoun_for_symbol(
symbol_flags: SymbolFlags,
) -> (/* singular */ &'static str, /* plural */ &'static str) {
if symbol_flags.is_function() {
"Function"
("Function", "functions")
} else if symbol_flags.is_class() {
"Class"
("Class", "classes")
} else if symbol_flags.is_interface() {
"Interface"
("Interface", "interfaces")
} else if symbol_flags.is_type_alias() {
"Type alias"
("Type alias", "type aliases")
} else if symbol_flags.is_enum() {
"Enum"
("Enum", "enums")
} else if symbol_flags.is_enum_member() {
"Enum member"
("Enum member", "enum members")
} else if symbol_flags.is_type_import() {
"Type"
("Type", "types")
} else if symbol_flags.is_import() {
"Identifier"
("Identifier", "identifiers")
} else if symbol_flags.is_catch_variable() {
("Catch parameter", "caught errors")
} else {
"Variable"
("Variable", "variables")
}
}

pub fn used_ignored(symbol: &Symbol<'_, '_>) -> OxcDiagnostic {
let pronoun = pronoun_for_symbol(symbol.flags());
pub fn used_ignored<R>(symbol: &Symbol<'_, '_>, pat: &IgnorePattern<R>) -> OxcDiagnostic
where
R: fmt::Display,
{
let (pronoun_singular, _) = pronoun_for_symbol(symbol.flags());
let name = symbol.name();

OxcDiagnostic::warn(format!("{pronoun} '{name}' is marked as ignored but is used."))
let help_suffix = match pat {
IgnorePattern::None => ".".into(),
IgnorePattern::Default => {
name.strip_prefix('_').map_or(".".into(), |name| format!(" to '{name}'."))
}
IgnorePattern::Some(ref r) => {
format!(" to match the pattern /{r}/.")
}
};

OxcDiagnostic::warn(format!("{pronoun_singular} '{name}' is marked as ignored but is used."))
.with_label(symbol.span().label(format!("'{name}' is declared here")))
.with_help(format!("Consider renaming this {}.", pronoun.cow_to_lowercase()))
.with_help(format!(
"Consider renaming this {}{help_suffix}",
pronoun_singular.cow_to_lowercase()
))
}
/// Variable 'x' is declared but never used.
pub fn declared(symbol: &Symbol<'_, '_>) -> OxcDiagnostic {
pub fn declared<R>(symbol: &Symbol<'_, '_>, pat: &IgnorePattern<R>) -> OxcDiagnostic
where
R: fmt::Display,
{
let (verb, help) = if symbol.flags().is_catch_variable() {
("caught", "Consider handling this error.")
} else {
("declared", "Consider removing this declaration.")
};
let pronoun = pronoun_for_symbol(symbol.flags());
let name = symbol.name();
let (pronoun, pronoun_plural) = pronoun_for_symbol(symbol.flags());
let suffix = pat.diagnostic_help(pronoun_plural);

OxcDiagnostic::warn(format!("{pronoun} '{name}' is {verb} but never used."))
OxcDiagnostic::warn(format!("{pronoun} '{name}' is {verb} but never used.{suffix}"))
.with_label(symbol.span().label(format!("'{name}' is declared here")))
.with_help(help)
}

/// Variable 'x' is assigned a value but never used.
pub fn assign(symbol: &Symbol<'_, '_>, assign_span: Span) -> OxcDiagnostic {
let pronoun = pronoun_for_symbol(symbol.flags());
pub fn assign<R>(
symbol: &Symbol<'_, '_>,
assign_span: Span,
pat: &IgnorePattern<R>,
) -> OxcDiagnostic
where
R: fmt::Display,
{
let name = symbol.name();
let (pronoun, pronoun_plural) = pronoun_for_symbol(symbol.flags());
let suffix = pat.diagnostic_help(pronoun_plural);

OxcDiagnostic::warn(format!("{pronoun} '{name}' is assigned a value but never used."))
OxcDiagnostic::warn(format!("{pronoun} '{name}' is assigned a value but never used.{suffix}"))
.with_labels([
symbol.span().label(format!("'{name}' is declared here")),
assign_span.label("it was last assigned here"),
Expand All @@ -64,17 +98,21 @@ pub fn assign(symbol: &Symbol<'_, '_>, assign_span: Span) -> OxcDiagnostic {
}

/// Parameter 'x' is declared but never used.
pub fn param(symbol: &Symbol<'_, '_>) -> OxcDiagnostic {
pub fn param<R>(symbol: &Symbol<'_, '_>, pat: &IgnorePattern<R>) -> OxcDiagnostic
where
R: fmt::Display,
{
let name = symbol.name();
let suffix = pat.diagnostic_help("parameters");

OxcDiagnostic::warn(format!("Parameter '{name}' is declared but never used."))
OxcDiagnostic::warn(format!("Parameter '{name}' is declared but never used.{suffix}"))
.with_label(symbol.span().label(format!("'{name}' is declared here")))
.with_help("Consider removing this parameter.")
}

/// Identifier 'x' imported but never used.
pub fn imported(symbol: &Symbol<'_, '_>) -> OxcDiagnostic {
let pronoun = pronoun_for_symbol(symbol.flags());
let (pronoun, _) = pronoun_for_symbol(symbol.flags());
let name = symbol.name();

OxcDiagnostic::warn(format!("{pronoun} '{name}' is imported but never used."))
Expand Down
25 changes: 14 additions & 11 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod usage;

use std::ops::Deref;

use options::NoUnusedVarsOptions;
use options::{IgnorePattern, NoUnusedVarsOptions};
use oxc_ast::AstKind;
use oxc_macros::declare_oxc_lint;
use oxc_semantic::{AstNode, ScopeFlags, SymbolFlags, SymbolId};
Expand Down Expand Up @@ -238,7 +238,7 @@ impl NoUnusedVars {
match (is_used, is_ignored) {
(true, true) => {
if self.report_used_ignore_pattern {
ctx.diagnostic(diagnostic::used_ignored(symbol));
ctx.diagnostic(diagnostic::used_ignored(symbol, &self.vars_ignore_pattern));
}
return;
},
Expand Down Expand Up @@ -283,9 +283,9 @@ impl NoUnusedVars {
if let Some(last_write) = symbol.references().rev().find(|r| r.is_write()) {
// ahg
let span = ctx.nodes().get_node(last_write.node_id()).kind().span();
diagnostic::assign(symbol, span)
diagnostic::assign(symbol, span, &self.vars_ignore_pattern)
} else {
diagnostic::declared(symbol)
diagnostic::declared(symbol, &self.vars_ignore_pattern)
};

ctx.diagnostic_with_suggestion(report, |fixer| {
Expand All @@ -298,39 +298,42 @@ impl NoUnusedVars {
if self.is_allowed_argument(ctx.semantic().as_ref(), symbol, param) {
return;
}
ctx.diagnostic(diagnostic::param(symbol));
ctx.diagnostic(diagnostic::param(symbol, &self.args_ignore_pattern));
}
AstKind::BindingRestElement(_) => {
if NoUnusedVars::is_allowed_binding_rest_element(symbol) {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
}
AstKind::Class(_) | AstKind::Function(_) => {
if self.is_allowed_class_or_function(symbol) {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
}
AstKind::TSModuleDeclaration(namespace) => {
if self.is_allowed_ts_namespace(symbol, namespace) {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
}
AstKind::TSInterfaceDeclaration(_) => {
if symbol.is_in_declared_module() {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None));
}
AstKind::TSTypeParameter(_) => {
if self.is_allowed_type_parameter(symbol, declaration.id()) {
return;
}
ctx.diagnostic(diagnostic::declared(symbol));
ctx.diagnostic(diagnostic::declared(symbol, &self.vars_ignore_pattern));
}
_ => ctx.diagnostic(diagnostic::declared(symbol)),
AstKind::CatchParameter(_) => {
ctx.diagnostic(diagnostic::declared(symbol, &self.caught_errors_ignore_pattern));
}
_ => ctx.diagnostic(diagnostic::declared(symbol, &IgnorePattern::<&str>::None)),
};
}

Expand Down
16 changes: 16 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_unused_vars/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,22 @@ impl<R> IgnorePattern<R> {
}
}
}
impl<R> IgnorePattern<R>
where
R: std::fmt::Display,
{
pub(super) fn diagnostic_help(&self, symbol_kind_plural: &str) -> Cow<'static, str> {
match self {
Self::None => Cow::Borrowed(""),
Self::Default => {
Cow::Owned(format!(" Unused {symbol_kind_plural} should start with a '_'."))
}
Self::Some(reg) => {
Cow::Owned(format!(" Unused {symbol_kind_plural} should match /{reg}/."))
}
}
}
}

impl From<Option<Regex>> for IgnorePattern<Regex> {
#[inline]
Expand Down
Loading

0 comments on commit 01a35bb

Please sign in to comment.