feat(linter): eslint-plugin-unicorn/no-abusive-eslint-disable (#1125)

This PR implements the
[eslint-plugin-unicorn/no-abusive-eslint-disable](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-abusive-eslint-disable.md)
rule.

Tests taken from
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/test/no-abusive-eslint-disable.mjs.

Related issue: #684
This commit is contained in:
Hao Cheng 2023-11-12 04:32:14 +01:00 committed by GitHub
parent 071311a898
commit 82c1769836
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 257 additions and 1 deletions

View file

@ -50,6 +50,10 @@ impl<'a> LintContext<'a> {
&self.semantic
}
pub fn disable_directives(&self) -> &DisableDirectives<'a> {
&self.disable_directives
}
pub fn source_text(&self) -> &'a str {
self.semantic().source_text()
}

View file

@ -9,9 +9,21 @@ enum DisabledRule<'a> {
Single(&'a str),
}
/// A comment which disables one or more specific rules
pub struct DisableRuleComment<'a> {
/// Span of the comment
pub span: Span,
/// Rules disabled by the comment
pub rules: Vec<&'a str>,
}
pub struct DisableDirectives<'a> {
/// All the disabled rules with their corresponding covering spans
intervals: Lapper<u32, DisabledRule<'a>>,
/// Spans of comments that disable all rules
disable_all_comments: Vec<Span>,
/// All comments that disable one or more specific rules
disable_rule_comments: Vec<DisableRuleComment<'a>>,
}
impl<'a> DisableDirectives<'a> {
@ -24,6 +36,14 @@ impl<'a> DisableDirectives<'a> {
|| matches!(interval.val, DisabledRule::Single(name) if name.contains(rule_name))
})
}
pub fn disable_all_comments(&self) -> &Vec<Span> {
&self.disable_all_comments
}
pub fn disable_rule_comments(&self) -> &Vec<DisableRuleComment<'a>> {
&self.disable_rule_comments
}
}
pub struct DisableDirectivesBuilder<'a, 'b> {
@ -35,6 +55,10 @@ pub struct DisableDirectivesBuilder<'a, 'b> {
disable_all_start: Option<u32>,
/// Start of `eslint-disable rule_name`
disable_start_map: FxHashMap<&'a str, u32>,
/// Spans of comments that disable all rules
disable_all_comments: Vec<Span>,
/// All comments that disable one or more specific rules
disable_rule_comments: Vec<DisableRuleComment<'a>>,
}
impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
@ -45,12 +69,18 @@ impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
intervals: Lapper::new(vec![]),
disable_all_start: None,
disable_start_map: FxHashMap::default(),
disable_all_comments: vec![],
disable_rule_comments: vec![],
}
}
pub fn build(mut self) -> DisableDirectives<'a> {
self.build_impl();
DisableDirectives { intervals: self.intervals }
DisableDirectives {
intervals: self.intervals,
disable_all_comments: self.disable_all_comments,
disable_rule_comments: self.disable_rule_comments,
}
}
fn add_interval(&mut self, start: u32, stop: u32, val: DisabledRule<'a>) {
@ -74,6 +104,7 @@ impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
if self.disable_all_start.is_none() {
self.disable_all_start = Some(span.end);
}
self.disable_all_comments.push(span);
continue;
}
@ -86,11 +117,15 @@ impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
.fold(span.end, |acc, line| acc + line.len() as u32);
if text.trim().is_empty() {
self.add_interval(span.end, stop, DisabledRule::All);
self.disable_all_comments.push(span);
} else {
// `eslint-disable-next-line rule_name1, rule_name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(span.end, stop, DisabledRule::Single(rule_name));
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
}
continue;
}
@ -107,19 +142,26 @@ impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> {
// `eslint-disable-line`
if text.trim().is_empty() {
self.add_interval(start, stop, DisabledRule::All);
self.disable_all_comments.push(span);
} else {
// `eslint-disable-line rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.add_interval(start, stop, DisabledRule::Single(rule_name));
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
}
continue;
}
// `eslint-disable rule-name1, rule-name2`
let mut rules = vec![];
Self::get_rule_names(text, |rule_name| {
self.disable_start_map.entry(rule_name).or_insert(span.end);
rules.push(rule_name);
});
self.disable_rule_comments.push(DisableRuleComment { span, rules });
continue;
}

View file

@ -146,6 +146,7 @@ mod unicorn {
pub mod error_message;
pub mod filename_case;
pub mod new_for_builtins;
pub mod no_abusive_eslint_disable;
pub mod no_console_spaces;
pub mod no_empty_file;
pub mod no_instanceof_array;
@ -286,6 +287,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::error_message,
unicorn::filename_case,
unicorn::new_for_builtins,
unicorn::no_abusive_eslint_disable,
unicorn::no_console_spaces,
unicorn::no_empty_file,
unicorn::no_instanceof_array,

View file

@ -0,0 +1,141 @@
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use crate::{context::LintContext, disable_directives::DisableRuleComment, rule::Rule};
#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.")]
#[diagnostic(severity(warning), help("Specify the rules you want to disable."))]
struct NoAbusiveEslintDisableDiagnostic(#[label] pub Span);
#[derive(Debug, Default, Clone)]
pub struct NoAbusiveEslintDisable;
declare_oxc_lint!(
/// ### What it does
/// This rule disallows `eslint-disable` comments that do not specify any rules to disable.
///
/// ### Why is this bad?
/// When only one rule should be disabled but the `eslint-disable` comment does not specify any rules, other useful errors will also be silently ignored.
///
/// ### Example
/// ```javascript
/// // Fail
/// /* eslint-disable */
/// console.log(message);
///
/// console.log(message); // eslint-disable-line
///
/// // eslint-disable-next-line
/// console.log(message);
///
/// // Pass
/// /* eslint-disable no-console */
/// console.log(message);
///
/// console.log(message); // eslint-disable-line no-console
///
/// // eslint-disable-next-line no-console
/// console.log(message);
/// ```
NoAbusiveEslintDisable,
restriction
);
impl Rule for NoAbusiveEslintDisable {
fn run_once(&self, ctx: &LintContext) {
for span in ctx.disable_directives().disable_all_comments() {
ctx.diagnostic(NoAbusiveEslintDisableDiagnostic(*span));
}
for DisableRuleComment { span, rules } in ctx.disable_directives().disable_rule_comments() {
if rules.is_empty() || !is_valid_rule_name(rules[0]) {
ctx.diagnostic(NoAbusiveEslintDisableDiagnostic(*span));
}
}
}
}
fn is_valid_rule_name(rule_name: &str) -> bool {
let segment_count = rule_name.split('/').count();
if rule_name.starts_with('@') {
segment_count == 2 || segment_count == 3
} else {
segment_count <= 2
}
}
#[test]
fn test() {
use crate::tester::Tester;
let pass = vec![
"eval();",
"eval(); // eslint-disable-line no-eval",
"eval(); // eslint-disable-line no-eval, no-console",
"eval(); //eslint-disable-line no-eval",
"eval(); // eslint-disable-line no-eval",
"eval(); // eslint-disable-line no-eval",
"eval(); /* eslint-disable-line no-eval */",
"eval(); // eslint-disable-line plugin/rule",
"eval(); // eslint-disable-line @scope/plugin/rule-name",
"eval(); // eslint-disable-line no-eval, @scope/plugin/rule-name",
"eval(); // eslint-disable-line @scope/rule-name",
"eval(); // eslint-disable-line no-eval, @scope/rule-name",
"eval(); // eslint-line-disable",
"eval(); // some comment",
"/* eslint-disable no-eval */",
r#"
/* eslint-disable no-abusive-eslint-disable */
eval(); // eslint-disable-line
"#,
r#"
foo();
// eslint-disable-line no-eval
eval();
"#,
r#"
foo();
/* eslint-disable no-eval */
eval();
"#,
r#"
foo();
/* eslint-disable-next-line no-eval */
eval();
"#,
];
let fail = vec![
r#"
// eslint-disable-next-line @scopewithoutplugin
eval();
"#,
"eval(); // eslint-disable-line",
r#"
foo();
eval(); // eslint-disable-line
"#,
"/* eslint-disable */",
r#"
foo();
/* eslint-disable */
eval();
"#,
r#"
foo();
/* eslint-disable-next-line */
eval();
"#,
r#"
// eslint-disable-next-line
eval();
"#,
];
Tester::new_without_config(NoAbusiveEslintDisable::NAME, pass, fail).test_and_snapshot();
}

View file

@ -0,0 +1,67 @@
---
source: crates/oxc_linter/src/tester.rs
assertion_line: 105
expression: no_abusive_eslint_disable
---
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:1:1]
1 │
2 │ // eslint-disable-next-line @scopewithoutplugin
· ──────────────────────────────────────────────
3 │ eval();
4 │
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:1:1]
1 │ eval(); // eslint-disable-line
· ────────────────────
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:2:1]
2 │ foo();
3 │ eval(); // eslint-disable-line
· ─────────────────────
4 │
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:1:1]
1 │ /* eslint-disable */
· ────────────────
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:2:1]
2 │ foo();
3 │ /* eslint-disable */
· ────────────────
4 │ eval();
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:2:1]
2 │ foo();
3 │ /* eslint-disable-next-line */
· ──────────────────────────
4 │ eval();
╰────
help: Specify the rules you want to disable.
⚠ eslint-plugin-unicorn(no-abusive-eslint-disable): Unexpected `eslint-disable` comment that does not specify any rules to disable.
╭─[no_abusive_eslint_disable.tsx:1:1]
1 │
2 │ // eslint-disable-next-line
· ──────────────────────────
3 │ eval();
4 │
╰────
help: Specify the rules you want to disable.