feat(rome_js_analyze): `noSelfCompare` rule
Closes
MODIFY
crates/rome_diagnostics_categories/src/categories.rs
crates/rome_diagnostics_categories/src/categories.rs
@@ -46,6 +46,7 @@ define_dategories! {
"lint/nursery/noAssignInExpressions": "https://docs.rome.tools/lint/rules/noAssignInExpressions",
"lint/nursery/noWith": "https://docs.rome.tools/lint/rules/noWith",
"lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes",
+ "lint/nursery/noCommaOperator": "https://docs.rome.tools/lint/rules/noCommaOperator",
"lint/nursery/noConstEnum": "https://docs.rome.tools/lint/rules/noConstEnum",
"lint/nursery/noConstructorReturn": "https://docs.rome.tools/lint/rules/noConstructorReturn",
"lint/nursery/noDistractingElements": "https://docs.rome.tools/lint/rules/noDistractingElements",
@@ -59,7 +60,7 @@ define_dategories! {
"lint/nursery/noRedundantAlt": "https://docs.rome.tools/lint/rules/noRedundantAlt",
"lint/nursery/noRedundantUseStrict": "https://docs.rome.tools/lint/rules/noRedundantUseStrict",
"lint/nursery/noRestrictedGlobals": "https://docs.rome.tools/lint/rules/noRestrictedGlobals",
- "lint/nursery/noCommaOperator": "https://docs.rome.tools/lint/rules/noCommaOperator",
+ "lint/nursery/noSelfCompare": "https://docs.rome.tools/lint/rules/noSelfCompare",
"lint/nursery/noSetterReturn": "https://docs.rome.tools/lint/rules/noSetterReturn",
"lint/nursery/noStringCaseMismatch": "https://docs.rome.tools/lint/rules/noStringCaseMismatch",
"lint/nursery/noUnsafeFinally": "https://docs.rome.tools/lint/rules/noUnsafeFinally",

MODIFY
crates/rome_js_analyze/src/analyzers/nursery.rs
crates/rome_js_analyze/src/analyzers/nursery.rs
@@ -17,6 +17,7 @@ mod no_non_null_assertion;
mod no_precision_loss;
mod no_redundant_alt;
mod no_redundant_use_strict;
+mod no_self_compare;
mod no_setter_return;
mod no_string_case_mismatch;
mod no_unsafe_finally;
@@ -28,4 +29,4 @@ mod use_default_switch_clause_last;
mod use_enum_initializers;
mod use_exponentiation_operator;
mod use_numeric_literals;
-declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } }
+declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_assign_in_expressions :: NoAssignInExpressions , self :: no_banned_types :: NoBannedTypes , self :: no_comma_operator :: NoCommaOperator , self :: no_const_enum :: NoConstEnum , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_duplicate_object_keys :: NoDuplicateObjectKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_non_null_assertion :: NoNonNullAssertion , self :: no_precision_loss :: NoPrecisionLoss , self :: no_redundant_alt :: NoRedundantAlt , self :: no_redundant_use_strict :: NoRedundantUseStrict , self :: no_self_compare :: NoSelfCompare , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_useless_switch_case :: NoUselessSwitchCase , self :: no_void_type_return :: NoVoidTypeReturn , self :: no_with :: NoWith , self :: use_default_parameter_last :: UseDefaultParameterLast , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_enum_initializers :: UseEnumInitializers , self :: use_exponentiation_operator :: UseExponentiationOperator , self :: use_numeric_literals :: UseNumericLiterals ,] } }

ADD
crates/rome_js_analyze/src/analyzers/nursery/no_self_compare.rs
+use rome_analyze::context::RuleContext;
+use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_js_syntax::{
+ JsBinaryExpression, JsBinaryOperator, JsSyntaxNode, JsSyntaxToken, WalkEvent,
+};
+use rome_rowan::{AstNode, Direction};
+use std::iter;
+
+declare_rule! {
+ /// Disallow comparisons where both sides are exactly the same.
+ ///
+ /// > Comparing a variable 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.
+ ///
+ /// > The only time you would compare a variable against itself is when you are testing for `NaN`. However, it is far more appropriate to use `typeof x === 'number' && isNaN(x)` or the [Number.isNaN ES2015 function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN) for that use case rather than leaving the reader of the code to determine the intent of self comparison.
+ ///
+ /// Source: [no-self-compare](https://eslint.org/docs/latest/rules/no-self-compare).
+ ///
+ /// ## Examples
+ ///
+ /// ### Invalid
+ ///
+ /// ```js,expect_diagnostic
+ /// if (x === x) {}
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// if (a.b.c() !== a.b .c()) {}
+ /// ```
+ ///
+ pub(crate) NoSelfCompare {
+ version: "12.0.0",
+ name: "noSelfCompare",
+ recommended: true,
+ }
+}
+
+impl Rule for NoSelfCompare {
+ type Query = Ast<JsBinaryExpression>;
+ type State = ();
+ type Signals = Option<Self::State>;
+ type Options = ();
+
+ fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+ let node = ctx.query();
+
+ if !matches!(
+ &node.operator(),
+ Ok(JsBinaryOperator::Equality
+ | JsBinaryOperator::Inequality
+ | JsBinaryOperator::GreaterThan
+ | JsBinaryOperator::GreaterThanOrEqual
+ | JsBinaryOperator::LessThan
+ | JsBinaryOperator::LessThanOrEqual
+ | JsBinaryOperator::StrictEquality
+ | JsBinaryOperator::StrictInequality)
+ ) {
+ return None;
+ }
+
+ let left = node.left().ok()?;
+ let right = node.right().ok()?;
+
+ if is_node_equal(&left.into_syntax(), &right.into_syntax()) {
+ return Some(());
+ }
+
+ None
+ }
+
+ fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
+ Some(RuleDiagnostic::new(
+ rule_category!(),
+ ctx.query().range(),
+ "Comparing to itself is potentially pointless.",
+ ))
+ }
+}
+
+/// Verifies that both nodes are equal by checking their descendants (nodes included) kinds
+/// and tokens (same kind and inner token text).
+fn is_node_equal(a_node: &JsSyntaxNode, b_node: &JsSyntaxNode) -> bool {
+ let a_tree = a_node.preorder_with_tokens(Direction::Next);
+ let b_tree = b_node.preorder_with_tokens(Direction::Next);
+
+ for (a_child, b_child) in iter::zip(a_tree, b_tree) {
+ let a_event = match a_child {
+ WalkEvent::Enter(event) => event,
+ WalkEvent::Leave(event) => event,
+ };
+
+ let b_event = match b_child {
+ WalkEvent::Enter(event) => event,
+ WalkEvent::Leave(event) => event,
+ };
+
+ if a_event.kind() != b_event.kind() {
+ return false;
+ }
+
+ let a_token = a_event.as_token();
+ let b_token = b_event.as_token();
+
+ match (a_token, b_token) {
+ // both are nodes
+ (None, None) => continue,
+ // one of them is a node
+ (None, Some(_)) | (Some(_), None) => return false,
+ // both are tokens
+ (Some(a), Some(b)) => {
+ if !is_token_text_equal(a, b) {
+ return false;
+ }
+ continue;
+ }
+ }
+ }
+
+ true
+}
+
+/// Verify that tokens' inner text are equal
+fn is_token_text_equal(a: &JsSyntaxToken, b: &JsSyntaxToken) -> bool {
+ static QUOTES: [char; 2] = ['"', '\''];
+
+ a.token_text_trimmed()
+ .trim_start_matches(QUOTES)
+ .trim_end_matches(QUOTES)
+ == b.token_text_trimmed()
+ .trim_start_matches(QUOTES)
+ .trim_end_matches(QUOTES)
+}

ADD
crates/rome_js_analyze/tests/specs/nursery/noSelfCompare.js
+// valid
+if (a || b) { }
+if (1 ^ 2) { }
+if (x === y) { }
+if (1 === 2) { }
+y=x*x
+foo.bar.baz === foo.bar.qux
+class C { #field; foo() { this.#field === this['#field']; } }
+class C { #field; foo() { this['#field'] === this.#field; } }
+
+// invalid
+if (x === x) { }
+if (x !== x) { }
+if (x > x) { }
+if ('x' > 'x') { }
+if ('x' > "x") { }
+do {} while (x === x)
+x === x
+x !== x
+x == x
+x != x
+x > x
+x < x
+x >= x
+x <= x
+foo.bar().baz.qux >= foo.bar ().baz .qux
+class C { #field; foo() { this.#field === this.#field; } }

MODIFY
editors/vscode/configuration_schema.json
editors/vscode/configuration_schema.json
}
]
},
+ "noSelfCompare": {
+ "description": "Disallow comparisons where both sides are exactly the same.",
+ "anyOf": [
+ {
+ "$ref": "#/definitions/RuleConfiguration"
+ },
+ {
+ "type": "null"
+ }
+ ]
+ },
"noSetterReturn": {
"description": "Disallow returning a value from a setter",
"anyOf": [

MODIFY
npm/rome/configuration_schema.json
npm/rome/configuration_schema.json
}
]
},
+ "noSelfCompare": {
+ "description": "Disallow comparisons where both sides are exactly the same.",
+ "anyOf": [
+ {
+ "$ref": "#/definitions/RuleConfiguration"
+ },
+ {
+ "type": "null"
+ }
+ ]
+ },
"noSetterReturn": {
"description": "Disallow returning a value from a setter",
"anyOf": [

feat(rome_js_analyze): `noSelfCompare` rule (#4031)

Closes https://github.com/rome/tools/issues/3982