Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyze): noCommaOperator (#4019)
Browse files Browse the repository at this point in the history
Co-authored-by: Micha Reiser <micha@rome.tools>
Closes #3932
  • Loading branch information
Conaclos authored Dec 15, 2022
1 parent 1aee087 commit ed01c0e
Show file tree
Hide file tree
Showing 15 changed files with 702 additions and 35 deletions.
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ define_dategories! {
"lint/nursery/noRedundantAlt": "https://docs.rome.tools/lint/rules/noRedundantAlt",
"lint/nursery/noRedundantUseStrict": "https://docs.rome.tools/lint/rules/noRedundantUseStrict",
"lint/nursery/noRestrictedGlobals": "https://docs.rome.tools/lint/rules/noRestrictedGlobals",
"lint/nursery/noCommaOperator": "https://docs.rome.tools/lint/rules/noCommaOperator",
"lint/nursery/noSetterReturn": "https://docs.rome.tools/lint/rules/noSetterReturn",
"lint/nursery/noStringCaseMismatch": "https://docs.rome.tools/lint/rules/noStringCaseMismatch",
"lint/nursery/noUnsafeFinally": "https://docs.rome.tools/lint/rules/noUnsafeFinally",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 76 additions & 0 deletions crates/rome_js_analyze/src/analyzers/nursery/no_comma_operator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic};
use rome_js_syntax::{JsForStatement, JsSequenceExpression};
use rome_rowan::AstNode;

declare_rule! {
/// Disallow comma operator.
///
/// The comma operator includes multiple expressions where only one is expected.
/// It evaluates every operand from left to right and returns the value of the last operand.
/// It frequently obscures side effects, and its use is often an accident.
///
/// The use of the comma operator in the initialization and update parts of a `for` is still allowed.
///
/// Source: https://eslint.org/docs/latest/rules/no-sequences
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// const foo = doSomething(), 0;
/// ```
///
/// ```js,expect_diagnostic
/// for (; doSomething(), !!test; ) {}
/// ```
///
/// ```js,expect_diagnostic
/// // Use a semicolon instead.
/// let a, b;
/// a = 1, b = 2;
/// ```
///
/// ### Valid
///
/// ```js
/// for(a = 0, b = 0; (a + b) < 10; a++, b += 2) {}
/// ```
///
pub(crate) NoCommaOperator {
version: "12.0.0",
name: "noCommaOperator",
recommended: true,
}
}

impl Rule for NoCommaOperator {
type Query = Ast<JsSequenceExpression>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let seq = ctx.query();
if let Some(for_stmt) = seq.parent::<JsForStatement>() {
// Allow comma operator in initializer and update parts of a `for`
if for_stmt.test().map(AstNode::into_syntax).as_ref() != Some(seq.syntax()) {
return None;
}
}
Some(())
}

fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
let seq = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
seq.comma_token().ok()?.text_trimmed_range(),
"The comma operator is disallowed.",
)
.note("Its use is often confusing and obscures side effects."),
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[
"a = 1, 2",
"do {} while (doSomething(), !!test);",
"for (; doSomething(), !!test; );",
"if (doSomething(), !!test);",
"switch (doSomething(), val) {}",
"while (doSomething(), !!test);",
"with (doSomething(), val) {}",
"a => (doSomething(), a)",
"(1), 2",
"((1)) , (2)",
"while((1) , 2);",

// Do not allow comma operator in parentheses
"var foo = (1, 2);",
"(0,eval)(\"foo()\");",
"foo(a, (b, c), d);",
"do {} while ((doSomething(), !!test));",
"for (; (doSomething(), !!test); );",
"if ((doSomething(), !!test));",
"switch ((doSomething(), val)) {}",
"while ((doSomething(), !!test));",
"with ((doSomething(), val)) {}",
"a => ((doSomething(), a))"
]
Loading

0 comments on commit ed01c0e

Please sign in to comment.