feat: add no-self-compare rule (#1230)

docs/rules/no_self_compare.md

@@ -0,0 +1,31 @@
+Disallows comparisons where both sides are exactly the same.
+
+Comparing a variable or value against itself is usually an error, either a typo
+or refactoring error. It is confusing to the reader and may potentially
+introduce a runtime error.
+
+### Invalid:
+
+```typescript
+if (x === x) {
+}
+if ("x" === "x") {
+}
+if (a.b === a.b) {
+}
+if (a["b"] === a["b"]) {
+}
+```
+
+### Valid:
+
+```typescript
+if (x === y) {
+}
+if ("x" === "y") {
+}
+if (a.b === a.c) {
+}
+if (a["b"] === a["c"]) {
+}
+```

src/rules.rs

@@ -76,6 +76,7 @@ pub mod no_prototype_builtins;
 pub mod no_redeclare;
 pub mod no_regex_spaces;
 pub mod no_self_assign;
+pub mod no_self_compare;
 pub mod no_setter_return;
 pub mod no_shadow_restricted_names;
 pub mod no_sparse_arrays;
@@ -298,6 +299,7 @@ fn get_all_rules_raw() -> Vec<&'static dyn LintRule> {
     &no_redeclare::NoRedeclare,
     &no_regex_spaces::NoRegexSpaces,
     &no_self_assign::NoSelfAssign,
+    &no_self_compare::NoSelfCompare,
     &no_setter_return::NoSetterReturn,
     &no_shadow_restricted_names::NoShadowRestrictedNames,
     &no_sparse_arrays::NoSparseArrays,

src/rules/no_self_compare.rs

@@ -0,0 +1,197 @@
+use super::{Context, LintRule};
+use crate::handler::{Handler, Traverse};
+use if_chain::if_chain;
+
+use deno_ast::{
+  view::{BinaryOp, NodeTrait},
+  SourceRanged,
+};
+use derive_more::Display;
+#[derive(Debug)]
+pub struct NoSelfCompare;
+
+const CODE: &str = "no-self-compare";
+const HINT: &str =
+  "Comparing a value to itself may be redundant and could potentially indicate a mistake in your code. Please double-check your logic";
+
+#[derive(Display)]
+enum NoSelfCompareMessage {
+  #[display(fmt = "`{}` is compared to itself", _0)]
+  Invalid(String),
+}
+
+impl LintRule for NoSelfCompare {
+  fn code(&self) -> &'static str {
+    CODE
+  }
+
+  fn lint_program_with_ast_view<'view>(
+    &self,
+    context: &mut Context<'view>,
+    program: deno_ast::view::Program<'view>,
+  ) {
+    NoSelfCompareHandler.traverse(program, context);
+  }
+
+  #[cfg(feature = "docs")]
+  fn docs(&self) -> &'static str {
+    include_str!("../../docs/rules/no_self_compare.md")
+  }
+}
+
+struct NoSelfCompareHandler;
+
+impl Handler for NoSelfCompareHandler {
+  fn bin_expr(
+    &mut self,
+    binary_expression: &deno_ast::view::BinExpr,
+    ctx: &mut Context,
+  ) {
+    if_chain! {
+      if matches!(
+        binary_expression.op(),
+        BinaryOp::EqEqEq
+          | BinaryOp::EqEq
+          | BinaryOp::NotEqEq
+          | BinaryOp::NotEq
+          | BinaryOp::Gt
+          | BinaryOp::Lt
+          | BinaryOp::GtEq
+          | BinaryOp::LtEq
+      );
+
+      if binary_expression.left.text() == binary_expression.right.text();
+
+      then {
+        ctx.add_diagnostic_with_hint(
+          binary_expression.range(),
+          CODE,
+          NoSelfCompareMessage::Invalid(binary_expression.left.text().to_string()),
+          HINT,
+        )
+      }
+
+    }
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+
+  #[test]
+  fn no_self_compare_valid() {
+    assert_lint_ok! {
+        NoSelfCompare,
+        "if (x === y) { }",
+        "if (1 === 2) { }",
+        "y=x*x",
+        "foo.bar.baz === foo.bar.qux",
+        "if ('x' === 'y') { }",
+    };
+  }
+
+  #[test]
+  fn no_self_compare_invalid() {
+    assert_lint_err! {
+        NoSelfCompare,
+        "if (x === x) { }": [
+            {
+                line: 1,
+                col: 4,
+                message: variant!(NoSelfCompareMessage, Invalid, "x"),
+                hint: HINT,
+            }
+        ],
+        "if (x == x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+      ],
+        "if (x !== x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+        "if (x > x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+        "if (x < x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+        "if (x >= x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+        "if (x <= x) { }": [
+          {
+              line: 1,
+              col: 4,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+        "foo.bar().baz.qux >= foo.bar().baz.qux": [
+          {
+              line: 1,
+              col: 0,
+              message: variant!(NoSelfCompareMessage, Invalid, "foo.bar().baz.qux"),
+              hint: HINT,
+          }
+        ],
+        "foo.bar().baz['qux'] >= foo.bar().baz['qux']": [
+          {
+              line: 1,
+              col: 0,
+              message: variant!(NoSelfCompareMessage, Invalid, "foo.bar().baz['qux']"),
+              hint: HINT,
+          }
+        ],
+        "if ('x' > 'x') { }": [
+          {
+            line: 1,
+            col: 4,
+            message: variant!(NoSelfCompareMessage, Invalid, "'x'"),
+            hint: HINT,
+          }
+        ],
+        "do {} while (x === x)": [
+          {
+            line: 1,
+            col: 13,
+            message: variant!(NoSelfCompareMessage, Invalid, "x"),
+            hint: HINT,
+          }
+        ],
+        "x === x ? console.log('foo') : console.log('bar');": [
+          {
+              line: 1,
+              col: 0,
+              message: variant!(NoSelfCompareMessage, Invalid, "x"),
+              hint: HINT,
+          }
+        ],
+
+    };
+  }
+}

www/static/docs.json

@@ -466,6 +466,11 @@
       "recommended"
     ]
   },
+  {
+    "code": "no-self-compare",
+    "docs": "Disallows comparisons where both sides are exactly the same.\n\nComparing a variable or value against itself is usually an error, either a typo\nor refactoring error. It is confusing to the reader and may potentially\nintroduce a runtime error.\n\n### Invalid:\n\n```typescript\nif (x === x) {\n}\nif (\"x\" === \"x\") {\n}\nif (a.b === a.b) {\n}\nif (a[\"b\"] === a[\"b\"]) {\n}\n```\n\n### Valid:\n\n```typescript\nif (x === y) {\n}\nif (\"x\" === \"y\") {\n}\nif (a.b === a.c) {\n}\nif (a[\"b\"] === a[\"c\"]) {\n}\n```\n",
+    "tags": []
+  },
   {
     "code": "no-setter-return",
     "docs": "Disallows returning values from setters.\n\nSetters are supposed to be used for setting some value to the property, which\nmeans that returning a value from a setter makes no sense. In fact, returned\nvalues are ignored and cannot ever be used at all although returning a value\nfrom a setter produces no error. This is why static check for this mistake by\nthe linter is quite beneficial.\n\nNote that returning without a value is allowed; this is a useful technique to do\nearly-return from a function.\n\n### Invalid:\n\n```typescript\nconst a = {\n  set foo(x: number) {\n    return \"something\";\n  },\n};\n\nclass B {\n  private set foo(x: number) {\n    return \"something\";\n  }\n}\n\nconst c = {\n  set foo(x: boolean) {\n    if (x) {\n      return 42;\n    }\n  },\n};\n```\n\n### Valid:\n\n```typescript\n// return without a value is allowed since it is used to do early-return\nconst a = {\n  set foo(x: number) {\n    if (x % 2 == 0) {\n      return;\n    }\n  },\n};\n\n// not a setter, but a getter\nclass B {\n  get foo() {\n    return 42;\n  }\n}\n\n// not a setter\nconst c = {\n  set(x: number) {\n    return \"something\";\n  },\n};\n```\n",