Skip to content

Commit

Permalink
feat(linter): report identical logical expressions in const-compariso…
Browse files Browse the repository at this point in the history
…ns (#7630)
  • Loading branch information
camc314 committed Dec 4, 2024
1 parent 052ff9b commit fe05fff
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 2 deletions.
47 changes: 46 additions & 1 deletion 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, NumericLiteral},
ast::{Expression, LogicalExpression, NumericLiteral},
AstKind,
};
use oxc_diagnostics::OxcDiagnostic;
Expand Down Expand Up @@ -43,6 +43,16 @@ fn constant_comparison_diagnostic(span: Span, evaluates_to: bool, help: String)
.with_label(span)
}

fn identical_expressions_logical_operator(left_span: Span, right_span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Both sides of the logical operator are the same")
.with_help("This logical expression will always evaluate to the same value as the expression itself.")
.with_labels([
left_span.label("If this expression evaluates to true"),
right_span
.label("This expression will always 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 @@ -98,6 +108,16 @@ impl ConstComparisons {
return;
};

Self::check_logical_expression_const_literal_comparison(logical_expr, ctx);
Self::check_redundant_logical_expression(logical_expr, ctx);
}
}
impl ConstComparisons {
// checks for `x < 42 && x < 42` and `x < 42 && x > 42`
fn check_logical_expression_const_literal_comparison<'a>(
logical_expr: &LogicalExpression<'a>,
ctx: &LintContext<'a>,
) {
if logical_expr.operator != LogicalOperator::And {
return;
}
Expand Down Expand Up @@ -177,6 +197,27 @@ impl ConstComparisons {
}
}

// checks for `a === b && a === b` and `a === b && a !== b`
fn check_redundant_logical_expression<'a>(
logical_expr: &LogicalExpression<'a>,
ctx: &LintContext<'a>,
) {
if !matches!(logical_expr.operator, LogicalOperator::And | LogicalOperator::Or) {
return;
}

if is_same_reference(
logical_expr.left.get_inner_expression(),
logical_expr.right.get_inner_expression(),
ctx,
) {
ctx.diagnostic(identical_expressions_logical_operator(
logical_expr.left.span(),
logical_expr.right.span(),
));
}
}

fn check_binary_expression<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::BinaryExpression(bin_expr) = node.kind() else {
return;
Expand Down Expand Up @@ -459,6 +500,10 @@ fn test() {
"a <= a",
"a > a",
"a >= a",
"a == b && a == b",
"a == b || a == b",
"!foo && !foo",
"!foo || !foo",
];

Tester::new(ConstComparisons::NAME, ConstComparisons::CATEGORY, pass, fail).test_and_snapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ fn test() {
"foo.slice(foo.length - 1 / 1)",
"[1, 2, 3].slice([1, 2, 3].length - 1)",
"foo[bar++].slice(foo[bar++].length - 1)",
"foo[a + b].slice(foo[a + b].length - 1)",
"foo[`${bar}`].slice(foo[`${bar}`].length - 1)",
"function foo() {return [].slice.apply(arguments);}",
"String.prototype.toSpliced.call(foo, foo.length - 1)",
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 @@ -262,3 +262,39 @@ source: crates/oxc_linter/src/tester.rs
· ──────
╰────
help: Because `a` will always be equal to itself

oxc(const-comparisons): Both sides of the logical operator are the same
╭─[const_comparisons.tsx:1:1]
1a == b && a == b
· ───┬── ───┬──
· │ ╰── This expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
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 the same
╭─[const_comparisons.tsx:1:1]
1a == b || a == b
· ───┬── ───┬──
· │ ╰── This expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
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 the same
╭─[const_comparisons.tsx:1:1]
1!foo && !foo
· ──┬─ ──┬─
· │ ╰── This expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
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 the same
╭─[const_comparisons.tsx:1:1]
1!foo || !foo
· ──┬─ ──┬─
· │ ╰── This expression will always evaluate to true
· ╰── If this expression evaluates to true
╰────
help: This logical expression will always evaluate to the same value as the expression itself.
29 changes: 29 additions & 0 deletions crates/oxc_linter/src/utils/unicorn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,35 @@ pub fn is_same_reference(left: &Expression, right: &Expression, ctx: &LintContex
return left_bool.value == right_bool.value;
}

(
Expression::BinaryExpression(left_bin_expr),
Expression::BinaryExpression(right_bin_expr),
) => {
return left_bin_expr.operator == right_bin_expr.operator
&& is_same_reference(
left_bin_expr.left.get_inner_expression(),
right_bin_expr.left.get_inner_expression(),
ctx,
)
&& is_same_reference(
left_bin_expr.right.get_inner_expression(),
right_bin_expr.right.get_inner_expression(),
ctx,
);
}

(
Expression::UnaryExpression(left_unary_expr),
Expression::UnaryExpression(right_unary_expr),
) => {
return left_unary_expr.operator == right_unary_expr.operator
&& is_same_reference(
left_unary_expr.argument.get_inner_expression(),
right_unary_expr.argument.get_inner_expression(),
ctx,
);
}

(
Expression::ChainExpression(left_chain_expr),
Expression::ChainExpression(right_chain_expr),
Expand Down

0 comments on commit fe05fff

Please sign in to comment.