Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 101 additions & 6 deletions crates/oxc_linter/src/rules/oxc/const_comparisons.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,35 @@ fn identical_expressions_logical_operator(left_span: Span, right_span: Span) ->
])
}

fn identical_expressions_logical_operator_negated(
fn equivalent_expressions_logical_operator(left_span: Span, right_span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Both sides of the logical operator are equivalent")
.with_help("This logical expression will always evaluate to the same value as either side.")
.with_labels([
left_span.label("If this expression evaluates to true"),
right_span.label("This equivalent expression will always evaluate to true"),
])
}

fn complementary_expressions_logical_operator(
always_truthy: bool,
left_span: Span,
right_span: Span,
) -> OxcDiagnostic {
let (left_label, right_label) = if always_truthy {
("If this expression evaluates to false", "This expression must evaluate to true")
} else {
("If this expression evaluates to true", "This expression cannot also evaluate to true")
};

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"),
])
.with_labels([left_span.label(left_label), right_span.label(right_label)])
}

#[derive(Clone, Copy)]
enum EqualityRelation {
Equivalent,
Inverse,
}

/// <https://rust-lang.github.io/rust-clippy/master/index.html#/impossible>
Expand Down Expand Up @@ -246,6 +264,30 @@ impl ConstComparisons {
logical_expr.left.span(),
logical_expr.right.span(),
));
return;
}

if let Some(relation) = equality_relation(
logical_expr.left.get_inner_expression(),
logical_expr.right.get_inner_expression(),
ctx,
) {
match relation {
EqualityRelation::Equivalent => {
ctx.diagnostic(equivalent_expressions_logical_operator(
logical_expr.left.span(),
logical_expr.right.span(),
));
}
EqualityRelation::Inverse => {
ctx.diagnostic(complementary_expressions_logical_operator(
matches!(logical_expr.operator, LogicalOperator::Or),
logical_expr.left.span(),
logical_expr.right.span(),
));
}
}
return;
}

// if either are `!foo`, check whether it looks like `foo && !foo` or `foo || !foo`
Expand All @@ -256,7 +298,7 @@ impl ConstComparisons {
if negated_expr.operator == UnaryOperator::LogicalNot
&& is_same_expression(&negated_expr.argument, other_expr, ctx)
{
ctx.diagnostic(identical_expressions_logical_operator_negated(
ctx.diagnostic(complementary_expressions_logical_operator(
matches!(logical_expr.operator, LogicalOperator::Or),
logical_expr.left.span(),
logical_expr.right.span(),
Expand Down Expand Up @@ -361,6 +403,51 @@ fn comparison_to_const<'a, 'b>(
None
}

fn equality_relation(
left: &Expression,
right: &Expression,
ctx: &LintContext,
) -> Option<EqualityRelation> {
if let Expression::BinaryExpression(left_bin_expr) = left
&& let Expression::BinaryExpression(right_bin_expr) = right
&& left_bin_expr.operator.is_equality()
&& right_bin_expr.operator.is_equality()
{
let same_order = is_same_expression(
left_bin_expr.left.get_inner_expression(),
right_bin_expr.left.get_inner_expression(),
ctx,
) && is_same_expression(
left_bin_expr.right.get_inner_expression(),
right_bin_expr.right.get_inner_expression(),
ctx,
);
let swapped_order = is_same_expression(
left_bin_expr.left.get_inner_expression(),
right_bin_expr.right.get_inner_expression(),
ctx,
) && is_same_expression(
left_bin_expr.right.get_inner_expression(),
right_bin_expr.left.get_inner_expression(),
ctx,
);

if !same_order && !swapped_order {
return None;
}

if left_bin_expr.operator == right_bin_expr.operator {
return Some(EqualityRelation::Equivalent);
} else if left_bin_expr.operator.equality_inverse_operator()
== Some(right_bin_expr.operator)
{
return Some(EqualityRelation::Inverse);
}
}

None
}

fn all_and_comparison_to_const<'a, 'b>(
expr: &'b Expression<'a>,
) -> Box<dyn Iterator<Item = (CmpOp, &'b Expression<'a>, &'b NumericLiteral<'a>, Span)> + 'b> {
Expand Down Expand Up @@ -590,13 +677,21 @@ fn test() {
"a.b.c >= a.b.c",
"a == b && a == b",
"a == b || a == b",
"a == b && b == a",
"a == (b) && (b) == a",
"a != b || b != a",
"!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",
"a == b && a != b",
"a != b && b == a",
"a !== a && a === a",
"a === b || a !== b",
"a !== a || a === a",
"let numOrUndefined: number | undefined = 6; if (numOrUndefined || 0 > 0) {}",
"let numOrUndefined: number | undefined = 6; if (numOrUndefined ?? 0 > 0) {}",
];
Expand Down
84 changes: 78 additions & 6 deletions crates/oxc_linter/src/snapshots/oxc_const_comparisons.snap
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,33 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: This logical expression will always evaluate to the same value as the expression itself.

⚠ oxc(const-comparisons): Both sides of the logical operator are equivalent
╭─[const_comparisons.tsx:1:1]
1 │ a == b && b == a
· ───┬── ───┬──
· │ ╰── This equivalent expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to the same value as either side.

⚠ oxc(const-comparisons): Both sides of the logical operator are equivalent
╭─[const_comparisons.tsx:1:1]
1 │ a == (b) && (b) == a
· ────┬─── ────┬───
· │ ╰── This equivalent expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to the same value as either side.

⚠ oxc(const-comparisons): Both sides of the logical operator are equivalent
╭─[const_comparisons.tsx:1:1]
1 │ a != b || b != a
· ───┬── ───┬──
· │ ╰── This equivalent expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to the same value as either side.

⚠ oxc(const-comparisons): Both sides of the logical operator are the same
╭─[const_comparisons.tsx:1:1]
1 │ !foo && !foo
Expand Down Expand Up @@ -318,7 +345,7 @@ source: crates/oxc_linter/src/tester.rs
╭─[const_comparisons.tsx:1:1]
1 │ !foo && foo
· ──┬─ ─┬─
· │ ╰── This expression will never evaluate to true
· │ ╰── This expression cannot also evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to false
Expand All @@ -327,7 +354,7 @@ source: crates/oxc_linter/src/tester.rs
╭─[const_comparisons.tsx:1:1]
1 │ foo && !foo
· ─┬─ ──┬─
· │ ╰── This expression will never evaluate to true
· │ ╰── This expression cannot also evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to false
Expand All @@ -336,17 +363,62 @@ source: crates/oxc_linter/src/tester.rs
╭─[const_comparisons.tsx:1:1]
1 │ !foo || foo
· ──┬─ ─┬─
· │ ╰── This expression will never evaluate to true
· ╰── If this expression evaluates to true
· │ ╰── This expression must evaluate to true
· ╰── If this expression evaluates to false
╰────
help: This logical expression will always evaluate to true

⚠ 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
· │ ╰── This expression must evaluate to true
· ╰── If this expression evaluates to false
╰────
help: This logical expression will always evaluate to true

⚠ oxc(const-comparisons): Unexpected constant comparison
╭─[const_comparisons.tsx:1:1]
1 │ a == b && a != b
· ───┬── ───┬──
· │ ╰── This expression cannot also 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 │ a != b && b == a
· ───┬── ───┬──
· │ ╰── This expression cannot also 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 │ a !== a && a === a
· ───┬─── ───┬───
· │ ╰── This expression cannot also 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 │ a === b || a !== b
· ───┬─── ───┬───
· │ ╰── This expression must evaluate to true
· ╰── If this expression evaluates to false
╰────
help: This logical expression will always evaluate to true

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

Expand Down
Loading