Skip to content

Commit

Permalink
feat(linter): for-direction rule add check for condition in reverse o… (
Browse files Browse the repository at this point in the history 
#1418)

Align eslint **for-direction** rule with the new default behaviour
eslint/eslint#17755
  • Loading branch information
@ild0tt0re
ild0tt0re committed Nov 18, 2023
1 parent 1dacb64 commit b72f83b
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 13 deletions.
53 changes: 40 additions & 13 deletions crates/oxc_linter/src/rules/eslint/for_direction.rs
View file
Expand Up @@ -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));
}
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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();
Expand Down
45 changes: 45 additions & 0 deletions crates/oxc_linter/src/snapshots/for_direction.snap
View file
Expand Up @@ -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){}
Expand Down Expand Up @@ -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


0 comments on commit b72f83b

Please sign in to comment.