mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
feat(linter): implement eslint/no-extra-label (#8181)
Implements [eslint/no-extra-label](https://eslint.org/docs/latest/rules/no-extra-label). --------- Co-authored-by: Cameron Clark <cameron.clark@hey.com>
This commit is contained in:
Anson Heung
GitHub
parent
ef76e288ca
commit
47cea9a52d
3 changed files with 420 additions and 0 deletions
|
|
@ -80,6 +80,7 @@ mod eslint {
|
|||
pub mod no_ex_assign;
|
||||
pub mod no_extend_native;
|
||||
pub mod no_extra_boolean_cast;
|
||||
pub mod no_extra_label;
|
||||
pub mod no_fallthrough;
|
||||
pub mod no_func_assign;
|
||||
pub mod no_global_assign;
|
||||
|
|
@ -540,6 +541,7 @@ oxc_macros::declare_all_lint_rules! {
|
|||
eslint::max_classes_per_file,
|
||||
eslint::max_lines,
|
||||
eslint::max_params,
|
||||
eslint::no_extra_label,
|
||||
eslint::no_multi_assign,
|
||||
eslint::no_nested_ternary,
|
||||
eslint::no_labels,
|
||||
|
|
|
|||
278
crates/oxc_linter/src/rules/eslint/no_extra_label.rs
Normal file
278
crates/oxc_linter/src/rules/eslint/no_extra_label.rs
Normal file
|
|
@ -0,0 +1,278 @@
|
|||
use oxc_ast::{ast::LabelIdentifier, AstKind};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
use oxc_span::{GetSpan, Span};
|
||||
|
||||
use crate::{context::LintContext, rule::Rule, AstNode};
|
||||
|
||||
fn no_extra_label_diagnostic(label: &LabelIdentifier) -> OxcDiagnostic {
|
||||
let label_name = &label.name;
|
||||
OxcDiagnostic::warn(format!("This label '{label_name}' is unnecessary"))
|
||||
.with_help(format!("Remove this label. It will have the same result because the labeled statement '{label_name}' has no nested loops or switches",))
|
||||
.with_label(label.span)
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct NoExtraLabel;
|
||||
|
||||
declare_oxc_lint!(
|
||||
/// ### What it does
|
||||
///
|
||||
/// Disallow unnecessary labels.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
///
|
||||
/// If a loop contains no nested loops or switches, labeling the loop is unnecessary.
|
||||
/// ```js
|
||||
/// A: while (a) {
|
||||
/// break A;
|
||||
/// }
|
||||
/// ```
|
||||
/// You can achieve the same result by removing the label and using `break` or `continue` without a label.
|
||||
/// Probably those labels would confuse developers because they expect labels to jump to further.
|
||||
///
|
||||
/// ### Examples
|
||||
///
|
||||
/// Examples of **incorrect** code for this rule:
|
||||
/// ```js
|
||||
/// A: while (a) {
|
||||
/// break A;
|
||||
/// }
|
||||
///
|
||||
/// B: for (let i = 0; i < 10; ++i) {
|
||||
/// break B;
|
||||
/// }
|
||||
///
|
||||
/// C: switch (a) {
|
||||
/// case 0:
|
||||
/// break C;
|
||||
/// }
|
||||
/// ```
|
||||
///
|
||||
/// Examples of **correct** code for this rule:
|
||||
/// ```js
|
||||
/// while (a) {
|
||||
/// break;
|
||||
/// }
|
||||
///
|
||||
/// for (let i = 0; i < 10; ++i) {
|
||||
/// break;
|
||||
/// }
|
||||
///
|
||||
/// switch (a) {
|
||||
/// case 0:
|
||||
/// break;
|
||||
/// }
|
||||
///
|
||||
/// A: {
|
||||
/// break A;
|
||||
/// }
|
||||
///
|
||||
/// B: while (a) {
|
||||
/// while (b) {
|
||||
/// break B;
|
||||
/// }
|
||||
/// }
|
||||
///
|
||||
/// C: switch (a) {
|
||||
/// case 0:
|
||||
/// while (b) {
|
||||
/// break C;
|
||||
/// }
|
||||
/// break;
|
||||
/// }
|
||||
/// ```
|
||||
NoExtraLabel,
|
||||
style,
|
||||
fix
|
||||
);
|
||||
|
||||
impl Rule for NoExtraLabel {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
if let AstKind::BreakStatement(break_stmt) = node.kind() {
|
||||
if let Some(label) = &break_stmt.label {
|
||||
report_label_if_extra(label, node, ctx);
|
||||
}
|
||||
}
|
||||
if let AstKind::ContinueStatement(cont_stmt) = node.kind() {
|
||||
if let Some(label) = &cont_stmt.label {
|
||||
report_label_if_extra(label, node, ctx);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn report_label_if_extra(label: &LabelIdentifier, node: &AstNode, ctx: &LintContext) {
|
||||
let nodes = ctx.nodes();
|
||||
for ancestor_id in nodes.ancestor_ids(node.id()) {
|
||||
if !is_breakable_statement(nodes.kind(ancestor_id)) {
|
||||
continue;
|
||||
}
|
||||
let Some(AstKind::LabeledStatement(labeled_stmt)) = nodes.parent_kind(ancestor_id) else {
|
||||
return; // no need to check outer loops/switches
|
||||
};
|
||||
if labeled_stmt.label.name != label.name {
|
||||
return;
|
||||
}
|
||||
|
||||
let keyword_len: u32 = match node.kind() {
|
||||
AstKind::BreakStatement(_) => 5,
|
||||
AstKind::ContinueStatement(_) => 8,
|
||||
_ => unreachable!(),
|
||||
};
|
||||
|
||||
let keyword_end = node.span().start + keyword_len;
|
||||
let delete_span = Span::new(keyword_end, label.span.end);
|
||||
|
||||
let diagnostic = no_extra_label_diagnostic(label);
|
||||
if ctx.comments().iter().any(|comment| delete_span.contains_inclusive(comment.span)) {
|
||||
// No autofix to avoid deleting comments between keyword and label
|
||||
// e.g. `break /* comment */ label;`
|
||||
ctx.diagnostic(diagnostic);
|
||||
} else {
|
||||
// e.g. `break label;` -> `break;`
|
||||
ctx.diagnostic_with_fix(diagnostic, |fixer| fixer.delete_range(delete_span));
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
fn is_breakable_statement(kind: AstKind) -> bool {
|
||||
match kind {
|
||||
kind if kind.is_iteration_statement() => true,
|
||||
AstKind::SwitchStatement(_) => true,
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test() {
|
||||
use crate::tester::Tester;
|
||||
|
||||
let pass = vec![
|
||||
"A: break A;",
|
||||
"A: { if (a) break A; }",
|
||||
"A: { while (b) { break A; } }",
|
||||
"A: { switch (b) { case 0: break A; } }",
|
||||
"A: while (a) { while (b) { break; } break; }",
|
||||
"A: while (a) { while (b) { break A; } }",
|
||||
"A: while (a) { while (b) { continue A; } }",
|
||||
"A: while (a) { switch (b) { case 0: break A; } }",
|
||||
"A: while (a) { switch (b) { case 0: continue A; } }",
|
||||
"A: switch (a) { case 0: while (b) { break A; } }",
|
||||
"A: switch (a) { case 0: switch (b) { case 0: break A; } }",
|
||||
"A: for (;;) { while (b) { break A; } }",
|
||||
"A: do { switch (b) { case 0: break A; break; } } while (a);",
|
||||
"A: for (a in obj) { while (b) { break A; } }",
|
||||
"A: for (a of ary) { switch (b) { case 0: break A; } }", // { "ecmaVersion": 6 }
|
||||
];
|
||||
|
||||
let fail = vec![
|
||||
"A: while (a) break A;",
|
||||
"A: while (a) { B: { continue A; } }",
|
||||
"X: while (x) { A: while (a) { B: { break A; break B; continue X; } } }",
|
||||
"A: do { break A; } while (a);",
|
||||
"A: for (;;) { break A; }",
|
||||
"A: for (a in obj) { break A; }",
|
||||
"A: for (a of ary) { break A; }", // { "ecmaVersion": 6 },
|
||||
"A: switch (a) { case 0: break A; }",
|
||||
"X: while (x) { A: switch (a) { case 0: break A; } }",
|
||||
"X: switch (a) { case 0: A: while (b) break A; }",
|
||||
"
|
||||
A: while (true) {
|
||||
break A;
|
||||
while (true) {
|
||||
break A;
|
||||
}
|
||||
}
|
||||
",
|
||||
"A: while(true) { /*comment*/break A; }",
|
||||
"A: while(true) { break/**/ A; }",
|
||||
"A: while(true) { continue /**/ A; }",
|
||||
"A: while(true) { break /**/A; }",
|
||||
"A: while(true) { continue/**/A; }",
|
||||
"A: while(true) { continue A/*comment*/; }",
|
||||
"A: while(true) { break A//comment
|
||||
}",
|
||||
"A: while(true) { break A/*comment*/
|
||||
foo() }",
|
||||
];
|
||||
|
||||
let fix = vec![
|
||||
("A: while (a) break A;", "A: while (a) break;", None),
|
||||
("A: while (a) { B: { continue A; } }", "A: while (a) { B: { continue; } }", None),
|
||||
(
|
||||
"X: while (x) { A: while (a) { B: { break A; break B; continue X; } } }",
|
||||
"X: while (x) { A: while (a) { B: { break; break B; continue X; } } }",
|
||||
None,
|
||||
),
|
||||
("A: do { break A; } while (a);", "A: do { break; } while (a);", None),
|
||||
("A: for (;;) { break A; }", "A: for (;;) { break; }", None),
|
||||
("A: for (a in obj) { break A; }", "A: for (a in obj) { break; }", None),
|
||||
("A: for (a of ary) { break A; }", "A: for (a of ary) { break; }", None),
|
||||
("A: switch (a) { case 0: break A; }", "A: switch (a) { case 0: break; }", None),
|
||||
(
|
||||
"X: while (x) { A: switch (a) { case 0: break A; } }",
|
||||
"X: while (x) { A: switch (a) { case 0: break; } }",
|
||||
None,
|
||||
),
|
||||
(
|
||||
"X: switch (a) { case 0: A: while (b) break A; }",
|
||||
"X: switch (a) { case 0: A: while (b) break; }",
|
||||
None,
|
||||
),
|
||||
(
|
||||
"
|
||||
A: while (true) {
|
||||
break A;
|
||||
while (true) {
|
||||
break A;
|
||||
}
|
||||
}
|
||||
",
|
||||
"
|
||||
A: while (true) {
|
||||
break;
|
||||
while (true) {
|
||||
break A;
|
||||
}
|
||||
}
|
||||
",
|
||||
None,
|
||||
),
|
||||
("A: while(true) { /*comment*/break A; }", "A: while(true) { /*comment*/break; }", None),
|
||||
(
|
||||
"A: while(true) { continue A/*comment*/; }",
|
||||
"A: while(true) { continue/*comment*/; }",
|
||||
None,
|
||||
),
|
||||
(
|
||||
"A: while(true) { break A//comment
|
||||
}",
|
||||
"A: while(true) { break//comment
|
||||
}",
|
||||
None,
|
||||
),
|
||||
(
|
||||
"A: while(true) { break A/*comment*/
|
||||
foo() }",
|
||||
"A: while(true) { break/*comment*/
|
||||
foo() }",
|
||||
None,
|
||||
),
|
||||
// Do not fix if a comment sits between break/continue and label
|
||||
(
|
||||
r"A: while(true) { break /*comment*/ A; }",
|
||||
r"A: while(true) { break /*comment*/ A; }",
|
||||
None,
|
||||
),
|
||||
(
|
||||
r"A: while(true) { continue /*comment*/ A; }",
|
||||
r"A: while(true) { continue /*comment*/ A; }",
|
||||
None,
|
||||
),
|
||||
];
|
||||
Tester::new(NoExtraLabel::NAME, NoExtraLabel::CATEGORY, pass, fail)
|
||||
.expect_fix(fix)
|
||||
.test_and_snapshot();
|
||||
}
|
||||
140
crates/oxc_linter/src/snapshots/eslint_no_extra_label.snap
Normal file
140
crates/oxc_linter/src/snapshots/eslint_no_extra_label.snap
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
---
|
||||
source: crates/oxc_linter/src/tester.rs
|
||||
snapshot_kind: text
|
||||
---
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:20]
|
||||
1 │ A: while (a) break A;
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:30]
|
||||
1 │ A: while (a) { B: { continue A; } }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:42]
|
||||
1 │ X: while (x) { A: while (a) { B: { break A; break B; continue X; } } }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:15]
|
||||
1 │ A: do { break A; } while (a);
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:21]
|
||||
1 │ A: for (;;) { break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:27]
|
||||
1 │ A: for (a in obj) { break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:27]
|
||||
1 │ A: for (a of ary) { break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:31]
|
||||
1 │ A: switch (a) { case 0: break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:46]
|
||||
1 │ X: while (x) { A: switch (a) { case 0: break A; } }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:44]
|
||||
1 │ X: switch (a) { case 0: A: while (b) break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:3:19]
|
||||
2 │ A: while (true) {
|
||||
3 │ break A;
|
||||
· ─
|
||||
4 │ while (true) {
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:35]
|
||||
1 │ A: while(true) { /*comment*/break A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:28]
|
||||
1 │ A: while(true) { break/**/ A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:32]
|
||||
1 │ A: while(true) { continue /**/ A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:28]
|
||||
1 │ A: while(true) { break /**/A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:30]
|
||||
1 │ A: while(true) { continue/**/A; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:27]
|
||||
1 │ A: while(true) { continue A/*comment*/; }
|
||||
· ─
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:24]
|
||||
1 │ A: while(true) { break A//comment
|
||||
· ─
|
||||
2 │ }
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
|
||||
⚠ eslint(no-extra-label): This label 'A' is unnecessary
|
||||
╭─[no_extra_label.tsx:1:24]
|
||||
1 │ A: while(true) { break A/*comment*/
|
||||
· ─
|
||||
2 │ foo() }
|
||||
╰────
|
||||
help: Remove this label. It will have the same result because the labeled statement 'A' has no nested loops or switches
|
||||
Loading…
Reference in a new issue