diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index a58d924ad6694..73b0b8663d348 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -90,6 +90,7 @@ mod eslint { pub mod no_new_wrappers; pub mod no_nonoctal_decimal_escape; pub mod no_obj_calls; + pub mod no_plusplus; pub mod no_proto; pub mod no_prototype_builtins; pub mod no_redeclare; @@ -548,6 +549,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_new_wrappers, eslint::no_nonoctal_decimal_escape, eslint::no_obj_calls, + eslint::no_plusplus, eslint::no_proto, eslint::no_prototype_builtins, eslint::no_redeclare, diff --git a/crates/oxc_linter/src/rules/eslint/no_plusplus.rs b/crates/oxc_linter/src/rules/eslint/no_plusplus.rs new file mode 100644 index 0000000000000..aaf0c91f68597 --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_plusplus.rs @@ -0,0 +1,186 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +fn no_plusplus_diagnostic(span: Span, operator: &str) -> OxcDiagnostic { + let diagnostic = + OxcDiagnostic::warn(format!("Unary operator '{operator}' used.")).with_label(span); + + if operator == "++" { + return diagnostic.with_help("Use the assignment operator `+=` instead."); + } else if operator == "--" { + return diagnostic.with_help("Use the assignment operator `-=` instead."); + } + + diagnostic +} + +#[derive(Debug, Default, Clone)] +pub struct NoPlusplus { + /// Whether to allow `++` and `--` in for loop afterthoughts. + allow_for_loop_afterthoughts: bool, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow the unary operators `++`` and `--`. + /// + /// ### Why is this bad? + /// + /// Because the unary `++` and `--` operators are subject to automatic semicolon insertion, differences in whitespace + /// can change the semantics of source code. For example, these two code blocks are not equivalent: + /// + /// ```js + /// var i = 10; + /// var j = 20; + /// + /// i ++ + /// j + /// // => i = 11, j = 20 + /// ``` + /// + /// ```js + /// var i = 10; + /// var j = 20; + /// + /// i + /// ++ + /// j + /// // => i = 10, j = 21 + /// ``` + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// var x = 0; x++; + /// var y = 0; y--; + /// for (let i = 0; i < l; i++) { + /// doSomething(i); + /// } + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// var x = 0; x += 1; + /// var y = 0; y -= 1; + /// for (let i = 0; i < l; i += 1) { + /// doSomething(i); + /// } + /// ``` + NoPlusplus, + restriction, + pending +); + +impl Rule for NoPlusplus { + fn from_configuration(value: serde_json::Value) -> Self { + let obj = value.get(0); + Self { + allow_for_loop_afterthoughts: obj + .and_then(|v| v.get("allowForLoopAfterthoughts")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + } + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::UpdateExpression(expr) = node.kind() else { + return; + }; + + if self.allow_for_loop_afterthoughts && is_for_loop_afterthought(node, ctx).unwrap_or(false) + { + return; + } + + ctx.diagnostic(no_plusplus_diagnostic(expr.span, expr.operator.as_str())); + } +} + +/// Determines whether the given node is considered to be a for loop "afterthought" by the logic of this rule. +/// In particular, it returns `true` if the given node is either: +/// - The update node of a `ForStatement`: for (;; i++) {} +/// - An operand of a sequence expression that is the update node: for (;; foo(), i++) {} +/// - An operand of a sequence expression that is child of another sequence expression, etc., +/// up to the sequence expression that is the update node: for (;; foo(), (bar(), (baz(), i++))) {} +fn is_for_loop_afterthought(node: &AstNode, ctx: &LintContext) -> Option { + let mut cur = ctx.nodes().parent_node(node.id())?; + + while let AstKind::SequenceExpression(_) | AstKind::ParenthesizedExpression(_) = cur.kind() { + cur = ctx.nodes().parent_node(cur.id())?; + } + + Some(matches!(cur.kind(), AstKind::ForStatement(stmt) if stmt.update.is_some())) +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("var foo = 0; foo=+1;", None), + ("var foo = 0; foo+=1;", None), + ("var foo = 0; foo-=1;", None), + ("var foo = 0; foo=+1;", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ( + "for (i = 0; i < l; i++) { console.log(i); }", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ( + "for (var i = 0, j = i + 1; j < example.length; i++, j++) {}", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ("for (;; i--, foo());", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ("for (;; foo(), --i);", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ( + "for (;; foo(), ++i, bar);", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ( + "for (;; i++, (++j, k--));", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ( + "for (;; foo(), (bar(), i++), baz());", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ( + "for (;; (--i, j += 2), bar = j + 1);", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ( + "for (;; a, (i--, (b, ++j, c)), d);", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ]; + + let fail = vec![ + ("var foo = 0; foo++;", None), + ("var foo = 0; foo--;", None), + ("var foo = 0; --foo;", None), + ("var foo = 0; ++foo;", None), + ("for (i = 0; i < l; i++) { console.log(i); }", None), + ("for (i = 0; i < l; foo, i++) { console.log(i); }", None), + ("var foo = 0; foo++;", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ( + "for (i = 0; i < l; i++) { v++; }", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ("for (i++;;);", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ("for (;--i;);", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ("for (;;) ++i;", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ("for (;; i = j++);", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ("for (;; i++, f(--j));", Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }]))), + ( + "for (;; foo + (i++, bar));", + Some(serde_json::json!([{ "allowForLoopAfterthoughts": true }])), + ), + ]; + + Tester::new(NoPlusplus::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_plusplus.snap b/crates/oxc_linter/src/snapshots/no_plusplus.snap new file mode 100644 index 0000000000000..3c4b6fa40c243 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_plusplus.snap @@ -0,0 +1,100 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:14] + 1 │ var foo = 0; foo++; + · ───── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '--' used. + ╭─[no_plusplus.tsx:1:14] + 1 │ var foo = 0; foo--; + · ───── + ╰──── + help: Use the assignment operator `-=` instead. + + ⚠ eslint(no-plusplus): Unary operator '--' used. + ╭─[no_plusplus.tsx:1:14] + 1 │ var foo = 0; --foo; + · ───── + ╰──── + help: Use the assignment operator `-=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:14] + 1 │ var foo = 0; ++foo; + · ───── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:20] + 1 │ for (i = 0; i < l; i++) { console.log(i); } + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:25] + 1 │ for (i = 0; i < l; foo, i++) { console.log(i); } + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:14] + 1 │ var foo = 0; foo++; + · ───── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:27] + 1 │ for (i = 0; i < l; i++) { v++; } + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:6] + 1 │ for (i++;;); + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '--' used. + ╭─[no_plusplus.tsx:1:7] + 1 │ for (;--i;); + · ─── + ╰──── + help: Use the assignment operator `-=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:10] + 1 │ for (;;) ++i; + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:13] + 1 │ for (;; i = j++); + · ─── + ╰──── + help: Use the assignment operator `+=` instead. + + ⚠ eslint(no-plusplus): Unary operator '--' used. + ╭─[no_plusplus.tsx:1:16] + 1 │ for (;; i++, f(--j)); + · ─── + ╰──── + help: Use the assignment operator `-=` instead. + + ⚠ eslint(no-plusplus): Unary operator '++' used. + ╭─[no_plusplus.tsx:1:16] + 1 │ for (;; foo + (i++, bar)); + · ─── + ╰──── + help: Use the assignment operator `+=` instead.