From 74c731c415c169d787fdaeefb7285ed59cbcdeff Mon Sep 17 00:00:00 2001 From: yangchenye Date: Tue, 14 Mar 2023 21:44:11 -0500 Subject: [PATCH] feat(linter): Implement eslint(no-compare-neg-zero) (#185) --- Cargo.lock | 1 + crates/oxc_linter/Cargo.toml | 4 +- crates/oxc_linter/src/rules.rs | 1 + .../src/rules/no_compare_neg_zero.rs | 126 ++++++++++++++++++ .../src/snapshots/no_compare_neg_zero.snap | 97 ++++++++++++++ tasks/coverage/babel | 2 +- tasks/coverage/test262 | 2 +- tasks/coverage/typescript | 2 +- 8 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 crates/oxc_linter/src/rules/no_compare_neg_zero.rs create mode 100644 crates/oxc_linter/src/snapshots/no_compare_neg_zero.snap diff --git a/Cargo.lock b/Cargo.lock index f1ff36c96..52e4c6a15 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -928,6 +928,7 @@ dependencies = [ "insta", "lazy_static", "miette", + "num-traits", "oxc_allocator", "oxc_ast", "oxc_diagnostics", diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml index 9d084909a..e167c7f67 100644 --- a/crates/oxc_linter/Cargo.toml +++ b/crates/oxc_linter/Cargo.toml @@ -20,10 +20,10 @@ serde_json = { workspace = true } indextree = { workspace = true } phf = { version = "0.11", features = ["macros"] } rustc-hash = { workspace = true } - +num-traits = "0.2.15" [dev_dependencies] oxc_allocator = { path = "../oxc_allocator" } -oxc_parser = { path = "../oxc_parser" } +oxc_parser = { path = "../oxc_parser" } insta = { version = "1.14.0", features = ["glob"] } miette = { workspace = true, features = ["fancy-no-backtrace"] } diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index fed8f2f6e..253cd460e 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -16,6 +16,7 @@ oxc_macros::declare_all_lint_rules! { no_self_compare, no_mixed_operators, no_constant_binary_expression, + no_compare_neg_zero, deepscan::uninvoked_array_callback, use_isnan, } diff --git a/crates/oxc_linter/src/rules/no_compare_neg_zero.rs b/crates/oxc_linter/src/rules/no_compare_neg_zero.rs new file mode 100644 index 000000000..7a9b74eaf --- /dev/null +++ b/crates/oxc_linter/src/rules/no_compare_neg_zero.rs @@ -0,0 +1,126 @@ +use oxc_ast::{ + ast::{BinaryOperator, Expression, UnaryOperator}, + AstKind, Span, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::{self, Error}, +}; +use oxc_macros::declare_oxc_lint; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(no-compare-neg-zero): Do not use the {0} operator to compare against -0.")] +#[diagnostic( + severity(warning), + help("Use Object.is(x, -0) to test equality with -0 and use 0 for other cases") +)] +struct NoCompareNegZeroDiagnostic(&'static str, #[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoCompareNegZero; + +declare_oxc_lint!( + /// ### What it does + /// Disallow comparing against -0 + /// + /// ### Why is this bad? + /// The rule should warn against code that tries to compare against -0, + /// since that will not work as intended. That is, code like x === -0 will + /// pass for both +0 and -0. The author probably intended Object.is(x, -0). + /// + /// ### Example + /// ```javascript + /// if (x === -0) {} + /// ``` + NoCompareNegZero, + correctness +); + +impl Rule for NoCompareNegZero { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::BinaryExpression(expr) = node.get().kind() else { return; }; + if Self::should_check(expr.operator) { + let op = expr.operator.as_str(); + if is_neg_zero(&expr.left) || is_neg_zero(&expr.right) { + ctx.diagnostic(NoCompareNegZeroDiagnostic(op, expr.span)); + } + } + } +} + +impl NoCompareNegZero { + fn should_check(operator: BinaryOperator) -> bool { + operator.is_compare() || operator.is_equality() + } +} + +fn is_neg_zero(expr: &Expression) -> bool { + use num_traits::Zero; + let Expression::UnaryExpression(unary) = expr.get_inner_expression() else { + return false; + }; + if unary.operator != UnaryOperator::UnaryNegation { + return false; + } + match &unary.argument { + Expression::NumberLiteral(number) => number.value == 0.0, + Expression::BigintLiteral(bigint) => bigint.value.is_zero(), + _ => false, + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("x === 0", None), + ("0 === x", None), + ("x == 0", None), + ("0 == x", None), + ("x === '0'", None), + ("'0' === x", None), + ("x == '0'", None), + ("'0' == x", None), + ("x === '-0'", None), + ("'-0' === x", None), + ("x == '-0'", None), + ("'-0' == x", None), + ("x === -1", None), + ("-1 === x", None), + ("x < 0", None), + ("0 < x", None), + ("x <= 0", None), + ("0 <= x", None), + ("x > 0", None), + ("0 > x", None), + ("x >= 0", None), + ("0 >= x", None), + ("x != 0", None), + ("0 != x", None), + ("x !== 0", None), + ("0 !== x", None), + ("Object.is(x, -0)", None), + ]; + + let fail = vec![ + ("x === -0", None), + ("-0 === x", None), + ("x == -0", None), + ("-0 == x", None), + ("x > -0", None), + ("-0 > x", None), + ("x >= -0", None), + ("-0 >= x", None), + ("x < -0", None), + ("-0 < x", None), + ("x <= -0", None), + ("-0 <= x", None), + // BigInt Literal + ("-0n <= x", None), + ]; + + Tester::new(NoCompareNegZero::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_compare_neg_zero.snap b/crates/oxc_linter/src/snapshots/no_compare_neg_zero.snap new file mode 100644 index 000000000..59c7ac446 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_compare_neg_zero.snap @@ -0,0 +1,97 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 53 +expression: no_compare_neg_zero +--- + + ⚠ eslint(no-compare-neg-zero): Do not use the === operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x === -0 + · ──────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the === operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 === x + · ──────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the == operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x == -0 + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the == operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 == x + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the > operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x > -0 + · ────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the > operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 > x + · ────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the >= operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x >= -0 + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the >= operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 >= x + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the < operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x < -0 + · ────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the < operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 < x + · ────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the <= operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ x <= -0 + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the <= operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0 <= x + · ─────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + + ⚠ eslint(no-compare-neg-zero): Do not use the <= operator to compare against -0. + ╭─[no_compare_neg_zero.tsx:1:1] + 1 │ -0n <= x + · ──────── + ╰──── + help: Use Object.is(x, -0) to test equality with -0 and use 0 for other cases + diff --git a/tasks/coverage/babel b/tasks/coverage/babel index 15bd88c43..c38bf12f0 160000 --- a/tasks/coverage/babel +++ b/tasks/coverage/babel @@ -1 +1 @@ -Subproject commit 15bd88c430c36d2a9b67c60eb717bdfcc823efd4 +Subproject commit c38bf12f010520ea7abe8a286f62922b2d1e1f1b diff --git a/tasks/coverage/test262 b/tasks/coverage/test262 index 9704d7f22..d216cc197 160000 --- a/tasks/coverage/test262 +++ b/tasks/coverage/test262 @@ -1 +1 @@ -Subproject commit 9704d7f22f6342d6c4753ab9a8d62d6725de8c4e +Subproject commit d216cc197269fc41eb6eca14710529c3d6650535 diff --git a/tasks/coverage/typescript b/tasks/coverage/typescript index 46d70d79c..8f40d5633 160000 --- a/tasks/coverage/typescript +++ b/tasks/coverage/typescript @@ -1 +1 @@ -Subproject commit 46d70d79cd0dd00d19e4c617d6ebb25e9f3fc7de +Subproject commit 8f40d5633fc36df04b4fd4392e3877558149987f