feat(linter): implement eslint(no-unsafe-negation) (#186)

Cargo.lock

@@ -934,6 +934,7 @@ dependencies = [
  "oxc_diagnostics",
  "oxc_macros",
  "oxc_parser",
+ "oxc_printer",
  "oxc_semantic",
  "phf",
  "rustc-hash",

crates/oxc_linter/Cargo.toml

@@ -14,6 +14,7 @@ oxc_ast = { path = "../oxc_ast" }
 oxc_diagnostics = { path = "../oxc_diagnostics" }
 oxc_macros = { path = "../oxc_macros" }
 oxc_semantic = { path = "../oxc_semantic" }
+oxc_printer = { path = "../oxc_printer" }
 
 lazy_static = { workspace = true }
 serde_json = { workspace = true }

crates/oxc_linter/src/context.rs

@@ -3,6 +3,7 @@ use std::{cell::RefCell, rc::Rc};
 use indextree::{Ancestors, NodeId};
 use oxc_ast::{ast::IdentifierReference, AstKind, SourceType};
 use oxc_diagnostics::Error;
+use oxc_printer::{Printer, PrinterOptions};
 use oxc_semantic::{AstNodes, Scope, ScopeTree, Semantic, SemanticNode};
 
 use crate::{
@@ -120,4 +121,9 @@ impl<'a> LintContext<'a> {
     pub fn is_reference_to_global_variable(&self, _ident: &IdentifierReference) -> bool {
         true
     }
+
+    #[allow(clippy::unused_self)]
+    pub fn printer(&self) -> Printer {
+        Printer::new(0, PrinterOptions::default())
+    }
 }

crates/oxc_linter/src/rules.rs

@@ -17,6 +17,7 @@ oxc_macros::declare_all_lint_rules! {
     no_mixed_operators,
     no_constant_binary_expression,
     no_compare_neg_zero,
+    no_unsafe_negation,
     deepscan::uninvoked_array_callback,
     use_isnan,
 }

crates/oxc_linter/src/rules/no_unsafe_negation.rs

@@ -0,0 +1,143 @@
+use oxc_ast::{
+    ast::{BinaryExpression, BinaryOperator, Expression, UnaryOperator},
+    AstKind, GetSpan, Span,
+};
+use oxc_diagnostics::{
+    miette::{self, Diagnostic},
+    thiserror::{self, Error},
+};
+use oxc_macros::declare_oxc_lint;
+use oxc_printer::Gen;
+
+use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
+
+#[derive(Debug, Error, Diagnostic)]
+#[error("Unexpected logical not in the left hand side of '{0}' operator")]
+#[diagnostic(
+    severity(warning),
+    help(
+        "use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '{0}'"
+    )
+)]
+struct NoUnsafeNegationDiagnostic(&'static str, #[label] pub Span);
+
+#[derive(Debug, Default, Clone)]
+pub struct NoUnsafeNegation {
+    /// true: disallow negation of the left-hand side of ordering relational operators
+    /// false: allow negation of the left-hand side of ordering relational operators (<, >, <=, >=)
+    enforce_for_ordering_relations: bool,
+}
+
+declare_oxc_lint!(
+    /// ### What it does
+    /// Disallow negating the left operand of relational operators
+    ///
+    /// ### Why is this bad?
+    /// Just as developers might type -a + b when they mean -(a + b) for the negative of a sum,
+    /// they might type !key in object by mistake when they almost certainly mean !(key in object)
+    /// to test that a key is not in an object. !obj instanceof Ctor is similar.
+    ///
+    /// ### Example
+    /// ```javascript
+    /// if (!key in object) {
+    ///   //operator precedence makes it equivalent to (!key) in object
+    ///   //and type conversion makes it equivalent to (key ? "false" : "true") in object
+    /// }
+    /// ```
+    NoUnsafeNegation,
+    correctness
+);
+
+impl Rule for NoUnsafeNegation {
+    fn from_configuration(value: serde_json::Value) -> Self {
+        let enforce_for_ordering_relations = value
+            .get(0)
+            .and_then(|config| config.get("enforceForOrderingRelations"))
+            .and_then(serde_json::Value::as_bool)
+            .unwrap_or_default();
+        Self { enforce_for_ordering_relations }
+    }
+
+    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 Expression::UnaryExpression(left) = &expr.left else { return; };
+            if left.operator == UnaryOperator::LogicalNot {
+                Self::report_with_fix(expr, ctx);
+            }
+        }
+    }
+}
+
+impl NoUnsafeNegation {
+    fn should_check(&self, op: BinaryOperator) -> bool {
+        op.is_relational() || (self.enforce_for_ordering_relations && op.is_compare())
+    }
+
+    /// Precondition:
+    /// expr.left is `UnaryExpression` whose operator is '!'
+    fn report_with_fix(expr: &BinaryExpression, ctx: &LintContext<'_>) {
+        // Diagnostic points at the unexpected negation
+        let diagnostic = NoUnsafeNegationDiagnostic(expr.operator.as_str(), expr.left.span());
+
+        let fix_producer = || {
+            // modify `!a instance of B` to `!(a instanceof B)`
+            let modified_code = {
+                let mut printer = ctx.printer();
+                printer.print(b'!');
+                let Expression::UnaryExpression(left) = &expr.left else { unreachable!() };
+                printer.print(b'(');
+                left.argument.gen(&mut printer);
+                expr.operator.gen(&mut printer);
+                expr.right.gen(&mut printer);
+                printer.print(b')');
+                printer.into_code()
+            };
+            Fix::new(modified_code, expr.span)
+        };
+
+        ctx.diagnostic_with_fix(diagnostic, fix_producer);
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        ("a in b", None),
+        ("a in b === false", None),
+        ("!(a in b)", None),
+        ("(!a) in b", None),
+        ("a instanceof b", None),
+        ("a instanceof b === false", None),
+        ("!(a instanceof b)", None),
+        ("(!a) instanceof b", None),
+        ("if (! a < b) {}", None),
+        ("while (! a > b) {}", None),
+        ("foo = ! a <= b;", None),
+        ("foo = ! a >= b;", None),
+        ("! a <= b", Some(serde_json::json!([{}]))),
+        ("foo = ! a >= b;", Some(serde_json::json!([{ "enforceForOrderingRelations": false }]))),
+        ("foo = (!a) >= b;", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("a <= b", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("!(a < b)", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("foo = a > b;", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+    ];
+
+    let fail = vec![
+        ("!a in b", None),
+        ("(!a in b)", None),
+        ("!(a) in b", None),
+        ("!a instanceof b", None),
+        ("(!a instanceof b)", None),
+        ("!(a) instanceof b", None),
+        ("if (! a < b) {}", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("while (! a > b) {}", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("foo = ! a <= b;", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("foo = ! a >= b;", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+        ("! a <= b", Some(serde_json::json!([{ "enforceForOrderingRelations": true }]))),
+    ];
+
+    Tester::new(NoUnsafeNegation::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/no_unsafe_negation.snap

@@ -0,0 +1,82 @@
+---
+source: crates/oxc_linter/src/tester.rs
+expression: no_unsafe_negation
+---
+
+  ⚠ Unexpected logical not in the left hand side of 'in' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ !a in b
+   · ──
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'in'
+
+  ⚠ Unexpected logical not in the left hand side of 'in' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ (!a in b)
+   ·  ──
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'in'
+
+  ⚠ Unexpected logical not in the left hand side of 'in' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ !(a) in b
+   · ────
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'in'
+
+  ⚠ Unexpected logical not in the left hand side of 'instanceof' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ !a instanceof b
+   · ──
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'instanceof'
+
+  ⚠ Unexpected logical not in the left hand side of 'instanceof' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ (!a instanceof b)
+   ·  ──
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'instanceof'
+
+  ⚠ Unexpected logical not in the left hand side of 'instanceof' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ !(a) instanceof b
+   · ────
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than 'instanceof'
+
+  ⚠ Unexpected logical not in the left hand side of '<' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ if (! a < b) {}
+   ·     ───
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '<'
+
+  ⚠ Unexpected logical not in the left hand side of '>' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ while (! a > b) {}
+   ·        ───
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '>'
+
+  ⚠ Unexpected logical not in the left hand side of '<=' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ foo = ! a <= b;
+   ·       ───
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '<='
+
+  ⚠ Unexpected logical not in the left hand side of '>=' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ foo = ! a >= b;
+   ·       ───
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '>='
+
+  ⚠ Unexpected logical not in the left hand side of '<=' operator
+   ╭─[no_unsafe_negation.tsx:1:1]
+ 1 │ ! a <= b
+   · ───
+   ╰────
+  help: use parenthesis to express the negation of the whole boolean expression, as '!' binds more closely than '<='
+

crates/oxc_printer/src/gen.rs

@@ -1141,18 +1141,25 @@ impl<'a> Gen for UnaryExpression<'a> {
 impl<'a> Gen for BinaryExpression<'a> {
     fn gen(&self, p: &mut Printer) {
         self.left.gen(p);
-        let operator = self.operator.as_str().as_bytes();
-        if self.operator.is_keyword() {
+        self.operator.gen(p);
+        self.right.gen(p);
+    }
+}
+
+impl Gen for BinaryOperator {
+    fn gen(&self, p: &mut Printer) {
+        let operator = self.as_str().as_bytes();
+        if self.is_keyword() {
             p.print(b' ');
             p.print_str(operator);
             p.print(b' ');
         } else {
-            p.print_space_before_operator(self.operator.into());
+            let op: Operator = (*self).into();
+            p.print_space_before_operator(op);
             p.print_str(operator);
-            p.prev_op = Some(self.operator.into());
+            p.prev_op = Some(op);
             p.prev_op_end = p.code().len();
         }
-        self.right.gen(p);
     }
 }
 

crates/oxc_printer/src/lib.rs

@@ -8,7 +8,7 @@ mod gen;
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::ast::*;
 
-use crate::gen::Gen;
+pub use crate::gen::Gen;
 
 #[derive(Debug, Clone, Copy)]
 pub struct PrinterOptions {
@@ -65,6 +65,12 @@ impl Printer {
     #[must_use]
     pub fn build(mut self, program: &Program<'_>) -> String {
         program.gen(&mut self);
+        self.into_code()
+    }
+
+    #[must_use]
+    #[inline]
+    pub fn into_code(self) -> String {
         unsafe { String::from_utf8_unchecked(self.code) }
     }
 

tasks/coverage/printer.snap

@@ -1,3 +1,3 @@
 Printer Summary:
-AST Parsed     : 44519/44519 (100.00%)
-Positive Passed: 44519/44519 (100.00%)
+AST Parsed     : 44469/44469 (100.00%)
+Positive Passed: 44469/44469 (100.00%)