Skip to content

Commit

Permalink
feat(linter): catch more cases in const-comparisons (#8215)
Browse files Browse the repository at this point in the history
  • Loading branch information
camc314 authored Jan 2, 2025
1 parent cd274ee commit 0e168b8
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 2 deletions.
49 changes: 47 additions & 2 deletions crates/oxc_linter/src/rules/oxc/const_comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use std::cmp::Ordering;

use oxc_ast::{
ast::{Expression, LogicalExpression, NumericLiteral},
ast::{Expression, LogicalExpression, NumericLiteral, UnaryOperator},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
Expand Down Expand Up @@ -53,6 +53,19 @@ fn identical_expressions_logical_operator(left_span: Span, right_span: Span) ->
])
}

fn identical_expressions_logical_operator_negated(
always_truthy: bool,
left_span: Span,
right_span: Span,
) -> OxcDiagnostic {
OxcDiagnostic::warn("Unexpected constant comparison")
.with_help(format!("This logical expression will always evaluate to {always_truthy}"))
.with_labels([
left_span.label("If this expression evaluates to true"),
right_span.label("This expression will never evaluate to true"),
])
}

/// <https://rust-lang.github.io/rust-clippy/master/index.html#/impossible>
/// <https://rust-lang.github.io/rust-clippy/master/index.html#/redundant_comparisons>
#[derive(Debug, Default, Clone)]
Expand Down Expand Up @@ -197,7 +210,13 @@ impl ConstComparisons {
}
}

// checks for `a === b && a === b` and `a === b && a !== b`
/// checks for:
/// ```ts
/// a === b && b === a
/// a === b && a !== b
/// !a && a
/// a && !a
/// ```
fn check_redundant_logical_expression<'a>(
logical_expr: &LogicalExpression<'a>,
ctx: &LintContext<'a>,
Expand All @@ -216,6 +235,24 @@ impl ConstComparisons {
logical_expr.right.span(),
));
}

// if either are `!foo`, check whether it looks like `foo && !foo` or `foo || !foo`
match (logical_expr.left.get_inner_expression(), logical_expr.right.get_inner_expression())
{
(Expression::UnaryExpression(negated_expr), other_expr)
| (other_expr, Expression::UnaryExpression(negated_expr)) => {
if negated_expr.operator == UnaryOperator::LogicalNot
&& is_same_expression(&negated_expr.argument, other_expr, ctx)
{
ctx.diagnostic(identical_expressions_logical_operator_negated(
matches!(logical_expr.operator, LogicalOperator::Or),
logical_expr.left.span(),
logical_expr.right.span(),
));
}
}
_ => {}
}
}

fn check_binary_expression<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) {
Expand Down Expand Up @@ -420,6 +457,10 @@ fn test() {
"a > b",
"a >= b",
"class Foo { #a; #b; constructor() { this.#a = 1; }; test() { return this.#a > this.#b } }",
"!foo && bar",
"!foo && !bar",
"foo || bar",
"!foo || bar",
];

let fail = vec![
Expand Down Expand Up @@ -508,6 +549,10 @@ fn test() {
"!foo && !foo",
"!foo || !foo",
"class Foo { #a; #b; constructor() { this.#a = 1; }; test() { return this.#a > this.#a } }",
"!foo && foo",
"foo && !foo",
"!foo || foo",
"foo || !foo",
];

Tester::new(ConstComparisons::NAME, ConstComparisons::CATEGORY, pass, fail).test_and_snapshot();
Expand Down
36 changes: 36 additions & 0 deletions crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,39 @@ source: crates/oxc_linter/src/tester.rs
· ─────────────────
╰────
help: Because `this.#a` will never be greater than itself

oxc(const-comparisons): Unexpected constant comparison
╭─[const_comparisons.tsx:1:1]
1!foo && foo
· ──┬─ ─┬─
· │ ╰── This expression will never evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to false

oxc(const-comparisons): Unexpected constant comparison
╭─[const_comparisons.tsx:1:1]
1foo && !foo
· ─┬─ ──┬─
· │ ╰── This expression will never evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to false

oxc(const-comparisons): Unexpected constant comparison
╭─[const_comparisons.tsx:1:1]
1!foo || foo
· ──┬─ ─┬─
· │ ╰── This expression will never evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to true

oxc(const-comparisons): Unexpected constant comparison
╭─[const_comparisons.tsx:1:1]
1foo || !foo
· ─┬─ ──┬─
· │ ╰── This expression will never evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to true

0 comments on commit 0e168b8

Please sign in to comment.