diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 58180448585ba..de384d52c8c04 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -134,6 +134,7 @@ mod typescript { pub mod consistent_type_definitions; pub mod consistent_type_imports; pub mod explicit_function_return_type; + pub mod no_confusing_non_null_assertion; pub mod no_duplicate_enum_values; pub mod no_dynamic_delete; pub mod no_empty_interface; @@ -561,6 +562,7 @@ oxc_macros::declare_all_lint_rules! { typescript::explicit_function_return_type, typescript::no_non_null_assertion, typescript::no_non_null_asserted_nullish_coalescing, + typescript::no_confusing_non_null_assertion, typescript::no_dynamic_delete, jest::consistent_test_it, jest::expect_expect, diff --git a/crates/oxc_linter/src/rules/typescript/no_confusing_non_null_assertion.rs b/crates/oxc_linter/src/rules/typescript/no_confusing_non_null_assertion.rs new file mode 100644 index 0000000000000..209c11622d13a --- /dev/null +++ b/crates/oxc_linter/src/rules/typescript/no_confusing_non_null_assertion.rs @@ -0,0 +1,118 @@ +use oxc_ast::{ + ast::{Expression, SimpleAssignmentTarget}, + AstKind, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Default, Clone)] +pub struct NoConfusingNonNullAssertion; + +declare_oxc_lint!( + /// ### What it does + /// Disallow non-null assertion in locations that may be confusing. + /// + /// ### Why is this bad? + /// Using a non-null assertion (!) next to an assign or equals check (= or == or ===) creates code that is confusing as it looks similar to a not equals check (!= !==). + /// + /// ### Example + /// ```javascript + /// a! == b; // a non-null assertions(`!`) and an equals test(`==`) + /// a !== b; // not equals test(`!==`) + /// a! === b; // a non-null assertions(`!`) and an triple equals test(`===`) + /// ``` + NoConfusingNonNullAssertion, + suspicious, + // See for details +); + +fn not_need_no_confusing_non_null_assertion_diagnostic(op_str: &str, span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!( + "typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like \"a! {op_str} b\", which looks very similar to not equal \"a !{op_str} b\"." + )) + .with_help("Remove the \"!\", or prefix the \"=\" with it.") + .with_label(span) +} + +fn wrap_up_no_confusing_non_null_assertion_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn( + "typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like \"a! = b\", which looks very similar to not equal \"a != b\"." + ) + .with_help("Wrap left-hand side in parentheses to avoid putting non-null assertion \"!\" and \"=\" together.") + .with_label(span) +} + +fn get_depth_ends_in_bang(expr: &Expression<'_>) -> Option { + match expr { + Expression::TSNonNullExpression(_) => Some(0), + Expression::BinaryExpression(binary_expr) => { + get_depth_ends_in_bang(&binary_expr.right).map(|x| x + 1) + } + Expression::UnaryExpression(unary_expr) => { + get_depth_ends_in_bang(&unary_expr.argument).map(|x| x + 1) + } + Expression::AssignmentExpression(assignment_expr) => { + get_depth_ends_in_bang(&assignment_expr.right).map(|x| x + 1) + } + _ => None, + } +} + +impl Rule for NoConfusingNonNullAssertion { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::BinaryExpression(binary_expr) => { + let Some(bang_depth) = get_depth_ends_in_bang(&binary_expr.left) else { + return; + }; + if bang_depth == 0 { + ctx.diagnostic(not_need_no_confusing_non_null_assertion_diagnostic( + binary_expr.operator.as_str(), + binary_expr.span, + )); + } else { + ctx.diagnostic(wrap_up_no_confusing_non_null_assertion_diagnostic( + binary_expr.span, + )); + } + } + AstKind::AssignmentExpression(assignment_expr) => { + let Some(simple_target) = assignment_expr.left.as_simple_assignment_target() else { + return; + }; + let SimpleAssignmentTarget::TSNonNullExpression(_) = simple_target else { return }; + ctx.diagnostic(not_need_no_confusing_non_null_assertion_diagnostic( + assignment_expr.operator.as_str(), + assignment_expr.span, + )); + } + _ => {} + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec!["a == b!;", "a = b!;", "a !== b;", "a != b;", "(a + b!) == c;"]; // "(a + b!) = c;"]; that's a parse error?? + let fail = vec![ + "a! == b;", + "a! === b;", + "a + b! == c;", + "(obj = new new OuterObj().InnerObj).Name! == c;", + "(a==b)! ==c;", + "a! = b;", + "(obj = new new OuterObj().InnerObj).Name! = c;", + "(a=b)! =c;", + ]; + // let fix = vec![ + // // source, expected, rule_config? + // // ("f = 1 + d! == 2", "f = (1 + d!) == 2", None), TODO: Add suggest or the weird ;() fix + // // ("f = d! == 2", "f = d == 2", None), TODO: Add suggest remove bang + // ]; + Tester::new(NoConfusingNonNullAssertion::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_confusing_non_null_assertion.snap b/crates/oxc_linter/src/snapshots/no_confusing_non_null_assertion.snap new file mode 100644 index 0000000000000..926b13c51883e --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_confusing_non_null_assertion.snap @@ -0,0 +1,58 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ a! == b; + · ─────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! === b", which looks very similar to not equal "a !=== b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ a! === b; + · ──────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! = b", which looks very similar to not equal "a != b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ a + b! == c; + · ─────────── + ╰──── + help: Wrap left-hand side in parentheses to avoid putting non-null assertion "!" and "=" together. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ (obj = new new OuterObj().InnerObj).Name! == c; + · ────────────────────────────────────────────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! == b", which looks very similar to not equal "a !== b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ (a==b)! ==c; + · ─────────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! = b", which looks very similar to not equal "a != b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ a! = b; + · ────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! = b", which looks very similar to not equal "a != b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ (obj = new new OuterObj().InnerObj).Name! = c; + · ───────────────────────────────────────────── + ╰──── + help: Remove the "!", or prefix the "=" with it. + + ⚠ typescript-eslint(no-confusing-non-null-assertion): Confusing combinations of non-null assertion and equal test like "a! = b", which looks very similar to not equal "a != b". + ╭─[no_confusing_non_null_assertion.tsx:1:1] + 1 │ (a=b)! =c; + · ───────── + ╰──── + help: Remove the "!", or prefix the "=" with it.