Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(linter): for-direction rule add check for condition in reverse o… #1418

Merged
merged 1 commit into from
Nov 18, 2023
Merged

feat(linter): for-direction rule add check for condition in reverse o… #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 40 additions & 13 deletions crates/oxc_linter/src/rules/eslint/for_direction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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