feat(rome_js_analyze): new rule useValidForDirection
MODIFY
crates/rome_diagnostics_categories/src/categories.rs
crates/rome_diagnostics_categories/src/categories.rs
@@ -63,6 +63,7 @@ define_dategories! {
"lint/nursery/noPositiveTabindex": "https://rome.tools/docs/lint/rules/noPositiveTabindex",
"lint/nursery/noConstAssign": "https://rome.tools/docs/lint/rules/noConstAssign",
"lint/nursery/noExplicitAny": "https://rome.tools/docs/lint/rules/noExplicitAny",
+ "lint/nursery/useValidForDirection": "https://rome.tools/docs/lint/rules/useValidForDirection",
"lint/style/noNegationElse": "https://rome.tools/docs/lint/rules/noNegationElse",
"lint/style/noShoutyConstants": "https://rome.tools/docs/lint/rules/noShoutyConstants",
"lint/style/useSelfClosingElements": "https://rome.tools/docs/lint/rules/useSelfClosingElements",

MODIFY
crates/rome_js_analyze/src/analyzers/nursery.rs
crates/rome_js_analyze/src/analyzers/nursery.rs
@@ -10,4 +10,5 @@ mod use_flat_map;
mod use_key_with_click_events;
mod use_optional_chain;
mod use_valid_anchor;
-declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_auto_focus :: NoAutoFocus , self :: no_new_symbol :: NoNewSymbol , self :: no_unreachable :: NoUnreachable , self :: use_anchor_content :: UseAnchorContent , self :: use_blank_target :: UseBlankTarget , self :: use_flat_map :: UseFlatMap , self :: use_key_with_click_events :: UseKeyWithClickEvents , self :: use_optional_chain :: UseOptionalChain , self :: use_valid_anchor :: UseValidAnchor ,] } }
+mod use_valid_for_direction;
+declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_auto_focus :: NoAutoFocus , self :: no_new_symbol :: NoNewSymbol , self :: no_unreachable :: NoUnreachable , self :: use_anchor_content :: UseAnchorContent , self :: use_blank_target :: UseBlankTarget , self :: use_flat_map :: UseFlatMap , self :: use_key_with_click_events :: UseKeyWithClickEvents , self :: use_optional_chain :: UseOptionalChain , self :: use_valid_anchor :: UseValidAnchor , self :: use_valid_for_direction :: UseValidForDirection ,] } }

ADD
crates/rome_js_analyze/src/analyzers/nursery/use_valid_for_direction.rs
+use rome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{
+ JsAnyExpression, JsAssignmentOperator, JsBinaryOperator, JsForStatement,
+ JsIdentifierAssignment, JsIdentifierExpression, JsPostUpdateOperator, JsUnaryOperator,
+ TextRange,
+};
+
+declare_rule! {
+ /// Enforce "for" loop update clause moving the counter in the right direction.
+ ///
+ /// A for loop with a stop condition that can never be reached,
+ /// such as one with a counter that moves in the wrong direction, will run infinitely.
+ /// While there are occasions when an infinite loop is intended, the convention is to construct such loops as while loops.
+ /// More typically, an infinite for loop is a bug.
+ ///
+ /// ## Examples
+ ///
+ /// ### Invalid
+ ///
+ /// ```js,expect_diagnostic
+ /// for (var i = 0; i < 10; i--) {
+ /// }
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// for (var i = 10; i >= 0; i++) {
+ /// }
+ /// ```
+ ///
+ /// ```js,expect_diagnostic
+ /// for (var i = 0; i > 10; i++) {
+ /// }
+ /// ```
+ ///
+ /// ### Valid
+ ///
+ /// ```js
+ /// for (var i = 0; i < 10; i++) {
+ /// }
+ /// ```
+ pub(crate) UseValidForDirection {
+ version: "0.10.0",
+ name: "useValidForDirection",
+ recommended: false,
+ }
+}
+
+pub struct RuleRangeState {
+ l_paren_range: TextRange,
+ r_paren_range: TextRange,
+}
+
+impl Rule for UseValidForDirection {
+ type Query = Ast<JsForStatement>;
+ type State = RuleRangeState;
+ type Signals = Option<Self::State>;
+ type Options = ();
+
+ fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+ let n = ctx.query();
+
+ let l_paren_range = n.l_paren_token().ok()?.text_trimmed_range();
+ let r_paren_range = n.r_paren_token().ok()?.text_trimmed_range();
+ let rule_state = RuleRangeState {
+ l_paren_range,
+ r_paren_range,
+ };
+
+ let test = n.test()?;
+ let binary_expr = test.as_js_binary_expression()?;
+ let operator = binary_expr.operator().ok()?;
+
+ let is_less_than = matches!(
+ operator,
+ JsBinaryOperator::LessThan | JsBinaryOperator::LessThanOrEqual
+ );
+ let is_greater_than = matches!(
+ operator,
+ JsBinaryOperator::GreaterThan | JsBinaryOperator::GreaterThanOrEqual
+ );
+
+ if !is_less_than && !is_greater_than {
+ return None;
+ }
+
+ match n.update()? {
+ JsAnyExpression::JsPostUpdateExpression(update_expr) => {
+ let binary_expr_left = binary_expr.left().ok()?;
+ let counter_ident = binary_expr_left.as_js_identifier_expression()?;
+ let update_expr_operand = update_expr.operand().ok()?;
+ let update_ident = update_expr_operand.as_js_identifier_assignment()?;
+
+ if !is_identifier_same(counter_ident, update_ident)? {
+ return None;
+ }
+
+ if update_expr.operator().ok()? == JsPostUpdateOperator::Increment
+ && is_greater_than
+ {
+ return Some(rule_state);
+ }
+
+ if update_expr.operator().ok()? == JsPostUpdateOperator::Decrement && is_less_than {
+ return Some(rule_state);
+ }
+ }
+ JsAnyExpression::JsAssignmentExpression(assignment_expr) => {
+ let binary_expr_left = binary_expr.left().ok()?;
+ let counter_ident = binary_expr_left.as_js_identifier_expression()?;
+ let assignment_expr_left = assignment_expr.left().ok()?;
+ let update_ident = assignment_expr_left
+ .as_js_any_assignment()?
+ .as_js_identifier_assignment()?;
+
+ if !is_identifier_same(counter_ident, update_ident)? {
+ return None;
+ }
+
+ if assignment_expr
+ .right()
+ .ok()?
+ .as_js_identifier_expression()
+ .is_some()
+ {
+ return None;
+ }
+
+ match assignment_expr.operator().ok()? {
+ JsAssignmentOperator::AddAssign => {
+ if is_greater_than {
+ return Some(rule_state);
+ }
+
+ let assignment_expr_right = assignment_expr.right().ok()?;
+ let unary_expr = assignment_expr_right.as_js_unary_expression()?;
+ if is_less_than && unary_expr.operator().ok()? == JsUnaryOperator::Minus {
+ return Some(rule_state);
+ }
+ }
+ JsAssignmentOperator::SubtractAssign => {
+ if is_less_than {
+ return Some(rule_state);
+ }
+
+ let assignment_expr_right = assignment_expr.right().ok()?;
+ let unary_expr = assignment_expr_right.as_js_unary_expression()?;
+ if is_greater_than && unary_expr.operator().ok()? == JsUnaryOperator::Minus
+ {
+ return Some(rule_state);
+ }
+ }
+ _ => return None,
+ }
+ }
+ _ => return None,
+ }
+
+ None
+ }
+
+ fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+ Some(RuleDiagnostic::new(
+ rule_category!(),
+ state.l_paren_range.cover(state.r_paren_range),
+ markup! {
+ "The update clause in this loop moves the variable in the wrong direction."
+ },
+ ))
+ }
+}
+
+fn is_identifier_same(
+ counter_ident: &JsIdentifierExpression,
+ update_ident: &JsIdentifierAssignment,
+) -> Option<bool> {
+ Some(
+ counter_ident
+ .name()
+ .ok()?
+ .value_token()
+ .ok()?
+ .text_trimmed()
+ == update_ident.name_token().ok()?.text_trimmed(),
+ )
+}

ADD
crates/rome_js_analyze/tests/specs/nursery/useValidForDirection.js
+// invalid
+for (var i = 0; i < 10; i--) {}
+for (var i = 0; i <= 10; i--) {}
+for (var i = 10; i > 10; i++) {}
+for (var i = 10; i >= 0; i++) {}
+for (var i = 0; i < 10; i -= 1) {}
+for (var i = 0; i <= 10; i -= 1) {}
+for (var i = 10; i > 10; i += 1) {}
+for (var i = 10; i >= 0; i += 1) {}
+for (var i = 0; i < 10; i += -1) {}
+for (var i = 0; i <= 10; i += -1) {}
+for (var i = 10; i > 10; i -= -1) {}
+for (var i = 10; i >= 0; i -= -1) {}
+
+// valid
+for (var i = 0; i < 10; i++) {}
+for (var i = 0; i <= 10; i++) {}
+for (var i = 10; i > 0; i--) {}
+for (var i = 10; i >= 0; i--) {}
+for (var i = 10; i > 0; ) {}
+for (var i = 10; i >= 0; ) {}
+for (var i = 10; i < 0; ) {}
+for (var i = 10; i <= 0; ) {}
+for (var i = 10; i <= 0; j++) {}
+for (var i = 10; i <= 0; j--) {}
+for (var i = 10; i >= 0; j++) {}
+for (var i = 10; i >= 0; j--) {}
+for (var i = 10; i >= 0; j += 2) {}
+for (var i = 10; i >= 0; j -= 2) {}
+for (var i = 10; i >= 0; i |= 2) {}
+for (var i = 10; i >= 0; i %= 2) {}
+for (var i = 0; i < MAX; i += STEP_SIZE);
+for (var i = 0; i < MAX; i -= STEP_SIZE);
+for (var i = 10; i > 0; i += STEP_SIZE);
+for (var i = 0; i !== 10; i += 1) {}
+for (var i = 0; i === 10; i += 1) {}
+for (var i = 0; i == 10; i += 1) {}
+for (var i = 0; i != 10; i += 1) {}

MODIFY
crates/rome_js_syntax/src/expr_ext.rs
crates/rome_js_syntax/src/expr_ext.rs
@@ -4,9 +4,9 @@ use crate::{
JsAnyExpression, JsAnyLiteralExpression, JsArrayExpression, JsArrayHole,
JsAssignmentExpression, JsBinaryExpression, JsCallExpression, JsComputedMemberExpression,
JsIdentifierExpression, JsLiteralMemberName, JsLogicalExpression, JsNumberLiteralExpression,
- JsObjectExpression, JsReferenceIdentifier, JsRegexLiteralExpression, JsStaticMemberExpression,
- JsStringLiteralExpression, JsSyntaxKind, JsSyntaxToken, JsTemplate, JsUnaryExpression,
- OperatorPrecedence, T,
+ JsObjectExpression, JsPostUpdateExpression, JsReferenceIdentifier, JsRegexLiteralExpression,
+ JsStaticMemberExpression, JsStringLiteralExpression, JsSyntaxKind, JsSyntaxToken, JsTemplate,
+ JsUnaryExpression, OperatorPrecedence, T,
};
use crate::{JsPreUpdateExpression, JsSyntaxKind::*};
use core::iter;
@@ -396,6 +396,32 @@ impl JsPreUpdateExpression {
}
}
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub enum JsPostUpdateOperator {
+ /// `++`
+ Increment,
+ /// `--`
+ Decrement,
+}
+
+impl JsPostUpdateOperator {
+ pub const fn precedence(&self) -> OperatorPrecedence {
+ OperatorPrecedence::Unary
+ }
+}
+
+impl JsPostUpdateExpression {
+ pub fn operator(&self) -> SyntaxResult<JsPostUpdateOperator> {
+ let operator = self.operator_token()?;
+
+ Ok(match operator.kind() {
+ T![++] => JsPostUpdateOperator::Increment,
+ T![--] => JsPostUpdateOperator::Decrement,
+ _ => unreachable!(),
+ })
+ }
+}
+
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum JsAssignmentOperator {
Assign,

MODIFY
crates/rome_service/src/configuration/linter/rules.rs
crates/rome_service/src/configuration/linter/rules.rs
@@ -412,10 +412,11 @@ struct NurserySchema {
use_key_with_mouse_events: Option<RuleConfiguration>,
use_optional_chain: Option<RuleConfiguration>,
use_valid_anchor: Option<RuleConfiguration>,
+ use_valid_for_direction: Option<RuleConfiguration>,
}
impl Nursery {
const CATEGORY_NAME: &'static str = "nursery";
- pub(crate) const CATEGORY_RULES: [&'static str; 26] = [
+ pub(crate) const CATEGORY_RULES: [&'static str; 27] = [
"noArrayIndexKey",
"noAutofocus",
"noChildrenProp",
@@ -442,6 +443,7 @@ impl Nursery {
"useKeyWithMouseEvents",
"useOptionalChain",
"useValidAnchor",
+ "useValidForDirection",
];
const RECOMMENDED_RULES: [&'static str; 0] = [];
const RECOMMENDED_RULES_AS_FILTERS: [RuleFilter<'static>; 0] = [];

MODIFY
editors/vscode/configuration_schema.json
editors/vscode/configuration_schema.json
"type": "null"
}
]
+ },
+ "useValidForDirection": {
+ "anyOf": [
+ {
+ "$ref": "#/definitions/RuleConfiguration"
+ },
+ {
+ "type": "null"
+ }
+ ]
}
}
},

MODIFY
npm/backend-jsonrpc/src/workspace.ts
npm/backend-jsonrpc/src/workspace.ts
@@ -190,6 +190,7 @@ export interface Nursery {
useKeyWithMouseEvents?: RuleConfiguration;
useOptionalChain?: RuleConfiguration;
useValidAnchor?: RuleConfiguration;
+ useValidForDirection?: RuleConfiguration;
}
/**
* A list of rules that belong to this group
@@ -333,6 +334,7 @@ export type Category =
| "lint/nursery/noPositiveTabindex"
| "lint/nursery/noConstAssign"
| "lint/nursery/noExplicitAny"
+ | "lint/nursery/useValidForDirection"
| "lint/style/noNegationElse"
| "lint/style/noShoutyConstants"
| "lint/style/useSelfClosingElements"

MODIFY
website/src/docs/lint/rules/index.md
website/src/docs/lint/rules/index.md
@@ -413,3 +413,9 @@ Enforce using concise optional chain instead of chained logical expressions.
</h3>
Enforce that all anchors are valid, and they are navigable elements.
</section>
+<section class="rule">
+<h3 data-toc-exclude id="useValidForDirection">
+ <a href="/docs/lint/rules/useValidForDirection">useValidForDirection</a>
+</h3>
+Enforce &quot;for&quot; loop update clause moving the counter in the right direction.
+</section>

feat(rome_js_analyze): new rule useValidForDirection (#3475)