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
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]
1 │ a == 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 │ a == 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