From 4ee7c615eac3afab91755066eafad1cacbdeeec3 Mon Sep 17 00:00:00 2001 From: Angelo Annunziata Date: Sun, 19 Nov 2023 00:16:00 +0100 Subject: [PATCH] feat(linter): for-direction rule add check for condition in reverse order --- .../src/rules/eslint/for_direction.rs | 53 ++++++++++++++----- .../src/snapshots/for_direction.snap | 45 ++++++++++++++++ 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/for_direction.rs b/crates/oxc_linter/src/rules/eslint/for_direction.rs index 3f82589d3635..383fcdd4472f 100644 --- a/crates/oxc_linter/src/rules/eslint/for_direction.rs +++ b/crates/oxc_linter/src/rules/eslint/for_direction.rs @@ -49,19 +49,28 @@ impl Rule for ForDirection { fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { if let AstKind::ForStatement(for_loop) = node.kind() { if let Some(Expression::BinaryExpression(test)) = &for_loop.test { - if let Expression::Identifier(counter) = &test.left { - let test_operator = &test.operator; - let wrong_direction = match test_operator { - BinaryOperator::LessEqualThan | BinaryOperator::LessThan => BACKWARD, - BinaryOperator::GreaterEqualThan | BinaryOperator::GreaterThan => FORWARD, - _ => return, - }; - if let Some(update) = &for_loop.update { - let update_direction = get_update_direction(update, counter); - if update_direction == wrong_direction { - let update_span = get_update_span(update); - ctx.diagnostic(ForDirectionDiagnostic(test.span, update_span)); - } + let (counter, counter_position) = match (&test.left, &test.right) { + (Expression::Identifier(counter), _) => (counter, LEFT), + (_, Expression::Identifier(counter)) => (counter, RIGHT), + _ => return, + }; + let test_operator = &test.operator; + let wrong_direction = match (test_operator, counter_position) { + (BinaryOperator::LessEqualThan | BinaryOperator::LessThan, LEFT) => BACKWARD, + (BinaryOperator::LessEqualThan | BinaryOperator::LessThan, RIGHT) => FORWARD, + (BinaryOperator::GreaterEqualThan | BinaryOperator::GreaterThan, LEFT) => { + FORWARD + } + (BinaryOperator::GreaterEqualThan | BinaryOperator::GreaterThan, RIGHT) => { + BACKWARD + } + _ => return, + }; + if let Some(update) = &for_loop.update { + let update_direction = get_update_direction(update, counter); + if update_direction == wrong_direction { + let update_span = get_update_span(update); + ctx.diagnostic(ForDirectionDiagnostic(test.span, update_span)); } } } @@ -74,6 +83,10 @@ const FORWARD: UpdateDirection = 1; const BACKWARD: UpdateDirection = -1; const UNKNOWN: UpdateDirection = 0; +type CounterPosition<'a> = &'a str; +const LEFT: CounterPosition = "left"; +const RIGHT: CounterPosition = "right"; + fn get_update_direction(update: &Expression, counter: &IdentifierReference) -> UpdateDirection { match update { // match increment or decrement @@ -148,6 +161,11 @@ fn test() { ("for(var i = 0; i <= 10; i++){}", None), ("for(var i = 10; i > 0; i--){}", None), ("for(var i = 10; i >= 0; i--){}", None), + // test if '++', '--' with counter 'i' on the right side of test condition + ("for(var i = 0; 10 > i; i++){}", None), + ("for(var i = 0; 10 >= i; i++){}", None), + ("for(var i = 10; 0 < i; i--){}", None), + ("for(var i = 10; 0 <= i; i--){}", None), // test if '+=', '-=', ("for(var i = 0; i < 10; i+=1){}", None), ("for(var i = 0; i <= 10; i+=1){}", None), @@ -157,6 +175,8 @@ fn test() { ("for(var i = 10; i >= 0; i-=1){}", None), ("for(var i = 10; i > 0; i+=-1){}", None), ("for(var i = 10; i >= 0; i+=-1){}", None), + // test if '+=', '-=' with counter 'i' on the right side of test condition + ("for(var i = 0; 10 > i; i+=1){}", None), // test if no update. ("for(var i = 10; i > 0;){}", None), ("for(var i = 10; i >= 0;){}", None), @@ -186,6 +206,11 @@ fn test() { ("for (var i = 0; i <= 10; i--){}", None), ("for(var i = 10; i > 10; i++){}", None), ("for(var i = 10; i >= 0; i++){}", None), + // test if '++', '--' with counter 'i' on the right side of test condition + ("for(var i = 0; 10 > i; i--){}", None), + ("for(var i = 0; 10 >= i; i--){}", None), + ("for(var i = 10; 10 < i; i++){}", None), + ("for(var i = 10; 0 <= i; i++){}", None), // test if '+=', '-=' ("for(var i = 0; i < 10; i-=1){}", None), ("for(var i = 0; i <= 10; i-=1){}", None), @@ -195,6 +220,8 @@ fn test() { ("for(var i = 0; i <= 10; i+=-1){}", None), ("for(var i = 10; i > 10; i-=-1){}", None), ("for(var i = 10; i >= 0; i-=-1){}", None), + // test if '+=', '-=' with counter 'i' on the right side of test condition + ("for(var i = 0; 10 > i; i-=1){}", None), ]; Tester::new(ForDirection::NAME, pass, fail).test_and_snapshot(); diff --git a/crates/oxc_linter/src/snapshots/for_direction.snap b/crates/oxc_linter/src/snapshots/for_direction.snap index bba543c198b4..2e20ebe76e8c 100644 --- a/crates/oxc_linter/src/snapshots/for_direction.snap +++ b/crates/oxc_linter/src/snapshots/for_direction.snap @@ -38,6 +38,42 @@ expression: for_direction ╰──── help: Use while loop for intended infinite loop + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction + ╭─[for_direction.tsx:1:1] + 1 │ for(var i = 0; 10 > i; i--){} + · ───┬── ─┬─ + · │ ╰── with this update + · ╰── This test moves in the wrong direction + ╰──── + help: Use while loop for intended infinite loop + + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction + ╭─[for_direction.tsx:1:1] + 1 │ for(var i = 0; 10 >= i; i--){} + · ───┬─── ─┬─ + · │ ╰── with this update + · ╰── This test moves in the wrong direction + ╰──── + help: Use while loop for intended infinite loop + + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction + ╭─[for_direction.tsx:1:1] + 1 │ for(var i = 10; 10 < i; i++){} + · ───┬── ─┬─ + · │ ╰── with this update + · ╰── This test moves in the wrong direction + ╰──── + help: Use while loop for intended infinite loop + + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction + ╭─[for_direction.tsx:1:1] + 1 │ for(var i = 10; 0 <= i; i++){} + · ───┬── ─┬─ + · │ ╰── with this update + · ╰── This test moves in the wrong direction + ╰──── + help: Use while loop for intended infinite loop + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction ╭─[for_direction.tsx:1:1] 1 │ for(var i = 0; i < 10; i-=1){} @@ -110,4 +146,13 @@ expression: for_direction ╰──── help: Use while loop for intended infinite loop + ⚠ eslint(for-direction): The update clause in this loop moves the variable in the wrong direction + ╭─[for_direction.tsx:1:1] + 1 │ for(var i = 0; 10 > i; i-=1){} + · ───┬── ──┬─ + · │ ╰── with this update + · ╰── This test moves in the wrong direction + ╰──── + help: Use while loop for intended infinite loop +