feat(linter) eslint-unicorn catch error name (#984)

This commit is contained in:
Cameron 2023-10-12 11:06:46 +01:00 committed by GitHub
parent ce79bc12ab
commit 3af35b8048
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 364 additions and 4 deletions

View file

@ -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,

View file

@ -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<Atom>,
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::<Vec<Atom>>();
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<CatchErrorNameDiagnostic> {
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<SymbolId>, 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();
}

View file

@ -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) => { })
· ───
╰────

View file

@ -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<Item = &'a Reference> + '_ {
pub fn symbol_references(&self, symbol_id: SymbolId) -> impl Iterator<Item = &Reference> + '_ {
self.symbols.get_resolved_references(symbol_id)
}