Skip to content

Commit

Permalink
feat(linter): implement no-plusplus rule (#5570)
Browse files Browse the repository at this point in the history
- part of #479

This PR implements the `no-plusplus` rule and is more-or-less a direct
port of the ESLint version of the rule.

---------

Co-authored-by: Don Isaac <donald.isaac@gmail.com>
  • Loading branch information
camchenry and DonIsaac authored Sep 12, 2024
1 parent 2b8369f commit 4b04f65
Show file tree
Hide file tree
Showing 3 changed files with 288 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
186 changes: 186 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_plusplus.rs
Original file line number Diff line number Diff line change
@@ -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<bool> {
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();
}
100 changes: 100 additions & 0 deletions crates/oxc_linter/src/snapshots/no_plusplus.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
---
source: crates/oxc_linter/src/tester.rs
---
eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:14]
1var foo = 0; foo++;
· ─────
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '--' used.
╭─[no_plusplus.tsx:1:14]
1var foo = 0; foo--;
· ─────
╰────
help: Use the assignment operator `-=` instead.

eslint(no-plusplus): Unary operator '--' used.
╭─[no_plusplus.tsx:1:14]
1var foo = 0; --foo;
· ─────
╰────
help: Use the assignment operator `-=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:14]
1var foo = 0; ++foo;
· ─────
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:20]
1for (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]
1for (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]
1var foo = 0; foo++;
· ─────
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:27]
1for (i = 0; i < l; i++) { v++; }
· ───
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:6]
1for (i++;;);
· ───
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '--' used.
╭─[no_plusplus.tsx:1:7]
1for (;--i;);
· ───
╰────
help: Use the assignment operator `-=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:10]
1for (;;) ++i;
· ───
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:13]
1for (;; i = j++);
· ───
╰────
help: Use the assignment operator `+=` instead.

eslint(no-plusplus): Unary operator '--' used.
╭─[no_plusplus.tsx:1:16]
1for (;; i++, f(--j));
· ───
╰────
help: Use the assignment operator `-=` instead.

eslint(no-plusplus): Unary operator '++' used.
╭─[no_plusplus.tsx:1:16]
1for (;; foo + (i++, bar));
· ───
╰────
help: Use the assignment operator `+=` instead.

0 comments on commit 4b04f65

Please sign in to comment.