diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 8333ff44d4df1..07852800f94f8 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -124,6 +124,7 @@ mod jest { } mod unicorn { + pub mod catch_error_name; pub mod filename_case; pub mod no_instanceof_array; pub mod no_thenable; @@ -229,6 +230,7 @@ oxc_macros::declare_all_lint_rules! { jest::no_standalone_expect, jest::no_identical_title, jest::valid_title, + unicorn::catch_error_name, unicorn::no_instanceof_array, unicorn::no_unnecessary_await, unicorn::no_thenable, diff --git a/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs b/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs new file mode 100644 index 0000000000000..4eacd9b6a3910 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/catch_error_name.rs @@ -0,0 +1,272 @@ +use oxc_ast::{ + ast::{Argument, BindingPatternKind, Expression}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::SymbolId; +use oxc_span::{Atom, Span}; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-unicorn(catch-error-name): The catch parameter {0:?} should be named {1:?}")] +struct CatchErrorNameDiagnostic(Atom, Atom, #[label] pub Span); + +#[derive(Debug, Clone)] +pub struct CatchErrorName { + ignore: Vec, + name: Atom, +} + +impl Default for CatchErrorName { + fn default() -> Self { + Self { ignore: vec![], name: Atom::new_inline("error") } + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule enforces naming conventions for catch statements. + /// + /// ### Example + /// ```javascript + /// + /// // fail + /// try { } catch (foo) { } + /// + /// // pass + /// try { } catch (error) { } + /// + /// ``` + CatchErrorName, + style +); + +impl Rule for CatchErrorName { + fn from_configuration(value: serde_json::Value) -> Self { + let ignored_names = value + .get(0) + .and_then(|v| v.get("ignored")) + .and_then(serde_json::Value::as_array) + .unwrap_or(&vec![]) + .iter() + .map(serde_json::Value::as_str) + .filter(std::option::Option::is_some) + .map(|x| Atom::from(x.unwrap().to_string())) + .collect::>(); + + let allowed_name = Atom::from( + value + .get(0) + .and_then(|v| v.get("name")) + .and_then(serde_json::Value::as_str) + .unwrap_or("error"), + ); + + Self { ignore: ignored_names, name: allowed_name } + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + if let AstKind::CatchClause(catch_node) = node.kind() { + if let Some(catch_param) = &catch_node.param { + if let oxc_ast::ast::BindingPatternKind::BindingIdentifier(binding_ident) = + &catch_param.kind + { + if self.is_name_allowed(&binding_ident.name) { + return; + } + + if binding_ident.name.starts_with('_') { + if symbol_has_references(binding_ident.symbol_id.get(), ctx) { + ctx.diagnostic(CatchErrorNameDiagnostic( + binding_ident.name.clone(), + self.name.clone(), + binding_ident.span, + )); + } + return; + } + + ctx.diagnostic(CatchErrorNameDiagnostic( + binding_ident.name.clone(), + self.name.clone(), + binding_ident.span, + )); + } + } + } + + if let AstKind::CallExpression(call_expr) = node.kind() { + if let Expression::MemberExpression(member_expr) = &call_expr.callee { + if member_expr.static_property_name() == Some("catch") { + if let Some(arg0) = call_expr.arguments.get(0) { + if let Some(diagnostic) = self.check_function_arguments(arg0, ctx) { + ctx.diagnostic(diagnostic); + } + } + } + + if member_expr.static_property_name() == Some("then") { + if let Some(arg0) = call_expr.arguments.get(1) { + if let Some(diagnostic) = self.check_function_arguments(arg0, ctx) { + ctx.diagnostic(diagnostic); + } + } + } + } + } + } +} + +impl CatchErrorName { + fn is_name_allowed(&self, name: &Atom) -> bool { + self.name == name || self.ignore.contains(name) + } + fn check_function_arguments( + &self, + arg0: &Argument, + ctx: &LintContext, + ) -> Option { + if let Argument::Expression(expr) = arg0 { + if let Expression::ArrowExpression(arrow_expr) = expr { + if let Some(arg0) = arrow_expr.params.items.get(0) { + if let BindingPatternKind::BindingIdentifier(v) = &arg0.pattern.kind { + if self.is_name_allowed(&v.name) { + return None; + } + + if v.name.starts_with('_') { + if symbol_has_references(v.symbol_id.get(), ctx) { + ctx.diagnostic(CatchErrorNameDiagnostic( + v.name.clone(), + self.name.clone(), + v.span, + )); + } + + return None; + } + + return Some(CatchErrorNameDiagnostic( + v.name.clone(), + self.name.clone(), + v.span, + )); + } + } + } + + if let Expression::FunctionExpression(fn_expr) = expr { + if let Some(arg0) = fn_expr.params.items.get(0) { + if let BindingPatternKind::BindingIdentifier(binding_ident) = &arg0.pattern.kind + { + if self.is_name_allowed(&binding_ident.name) { + return None; + } + + if binding_ident.name.starts_with('_') { + if symbol_has_references(binding_ident.symbol_id.get(), ctx) { + ctx.diagnostic(CatchErrorNameDiagnostic( + binding_ident.name.clone(), + self.name.clone(), + binding_ident.span, + )); + } + + return None; + } + + return Some(CatchErrorNameDiagnostic( + binding_ident.name.clone(), + self.name.clone(), + binding_ident.span, + )); + } + } + } + } + None + } +} + +fn symbol_has_references(symbol_id: Option, ctx: &LintContext) -> bool { + if let Some(symbol_id) = symbol_id { + return ctx.semantic().symbol_references(symbol_id).next().is_some(); + } + false +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("try { } catch (error) { }", None), + ("try { } catch (err) { }", Some(serde_json::json!([{"name": "err"}]))), + ("obj.catch(error => { })", None), + ("obj.then(undefined, error => { })", None), + ("obj.then(result => { }, error => { })", None), + ("obj.catch(() => { })", None), + ("obj.catch(err => { })", Some(serde_json::json!([{"name": "err"}]))), + ("obj.then(undefined, err => { })", Some(serde_json::json!([{"name": "err"}]))), + ("obj.catch(function (error) { })", None), + ("obj.then(undefined, function (error) { })", None), + ("obj.catch(function onReject(error) { })", None), + ("obj.then(undefined, function onReject(error) { })", None), + ("obj.then(function onFulfilled(result) { }, function onReject(error) { })", None), + ("obj.catch(function () { })", None), + ("obj.catch(function (err) { })", Some(serde_json::json!([{"name": "err"}]))), + ("obj.then(undefined, function (err) { })", Some(serde_json::json!([{"name": "err"}]))), + ("obj.catch()", None), + ("foo(function (error) { })", None), + ("foo().then(function (error) { })", None), + ("foo().catch(function (error) { })", None), + ("try { } catch ({message}) { }", None), + ("obj.catch(function ({message}) { })", None), + ("obj.catch(({message}) => { })", None), + ("obj.then(undefined, ({message}) => { })", None), + ("obj.catch(error => { }, anotherArgument)", None), + ("obj.then(undefined, error => { }, anotherArgument)", None), + ("obj.catch(_ => { })", None), + ("obj.catch((_) => { })", None), + ("obj.catch((_) => { console.log(foo); })", None), + ("try { } catch (_) { }", None), + ("try { } catch (_) { console.log(foo); }", None), + ( + " + try { + } catch (_) { + console.log(_); + } + ", + Some(serde_json::json!([{"ignored": ["_"]}])), + ), + ("try { } catch (error) { }", None), + ("promise.catch(unicorn => { })", Some(serde_json::json!([{"ignored": ["unicorn"]}]))), + ("try { } catch (exception) { }", Some(serde_json::json!([{"name": "exception"}]))), + ]; + + let fail = vec![ + ("try { } catch (descriptiveError) { }", Some(serde_json::json!([{"name": "exception"}]))), + ("try { } catch (e) { }", Some(serde_json::json!([{"name": "has_space_after "}]))), + ("try { } catch (e) { }", Some(serde_json::json!([{"name": "1_start_with_a_number"}]))), + ("try { } catch (e) { }", Some(serde_json::json!([{"name": "_){ } evilCode; if(false"}]))), + ("try { } catch (notMatching) { }", Some(serde_json::json!([{"ignore": []}]))), + ("try { } catch (notMatching) { }", Some(serde_json::json!([{"ignore": ["unicorn"]}]))), + ("try { } catch (notMatching) { }", Some(serde_json::json!([{"ignore": ["unicorn"]}]))), + ("try { } catch (_) { console.log(_) }", None), + ("promise.catch(notMatching => { })", Some(serde_json::json!([{"ignore": ["unicorn"]}]))), + ("promise.catch((foo) => { })", None), + ("promise.catch(function (foo) { })", None), + ("promise.then(function (foo) { }).catch((foo) => { })", None), + ("promise.then(undefined, function (foo) { })", None), + ("promise.then(undefined, (foo) => { })", None), + ]; + + Tester::new(CatchErrorName::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/catch_error_name.snap b/crates/oxc_linter/src/snapshots/catch_error_name.snap new file mode 100644 index 0000000000000..b2277a2e92c62 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/catch_error_name.snap @@ -0,0 +1,89 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: catch_error_name +--- + × eslint-plugin-unicorn(catch-error-name): The catch parameter "descriptiveError" should be named "exception" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (descriptiveError) { } + · ──────────────── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "e" should be named "has_space_after " + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (e) { } + · ─ + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "e" should be named "1_start_with_a_number" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (e) { } + · ─ + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "e" should be named "_){ } evilCode; if(false" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (e) { } + · ─ + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "notMatching" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (notMatching) { } + · ─────────── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "notMatching" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (notMatching) { } + · ─────────── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "notMatching" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (notMatching) { } + · ─────────── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "_" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ try { } catch (_) { console.log(_) } + · ─ + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "notMatching" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.catch(notMatching => { }) + · ─────────── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "foo" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.catch((foo) => { }) + · ─── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "foo" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.catch(function (foo) { }) + · ─── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "foo" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.then(function (foo) { }).catch((foo) => { }) + · ─── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "foo" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.then(undefined, function (foo) { }) + · ─── + ╰──── + + × eslint-plugin-unicorn(catch-error-name): The catch parameter "foo" should be named "error" + ╭─[catch_error_name.tsx:1:1] + 1 │ promise.then(undefined, (foo) => { }) + · ─── + ╰──── + + diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index aa958ee0002e3..3f556c0581e8c 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -106,10 +106,7 @@ impl<'a> Semantic<'a> { } /// Get all resolved references for a symbol - pub fn symbol_references( - &'a self, - symbol_id: SymbolId, - ) -> impl Iterator + '_ { + pub fn symbol_references(&self, symbol_id: SymbolId) -> impl Iterator + '_ { self.symbols.get_resolved_references(symbol_id) }