From ff9a2c3ab06ecf22f1661ce0fa014bca31e63cd1 Mon Sep 17 00:00:00 2001 From: Boshen Date: Tue, 21 Mar 2023 20:27:42 -0700 Subject: [PATCH] feat(linter): eslint disable comments (#196) feat(linter): eslint disable comments closes #100 closes #170 --- Cargo.lock | 10 + crates/oxc_ast/src/trivia.rs | 24 +- crates/oxc_linter/Cargo.toml | 3 +- crates/oxc_linter/src/context.rs | 30 +- crates/oxc_linter/src/disable_directives.rs | 284 ++++++++++++++++++ crates/oxc_linter/src/fixer.rs | 35 ++- crates/oxc_linter/src/lib.rs | 4 +- crates/oxc_linter/src/tester.rs | 16 +- crates/oxc_parser/src/lexer/trivia_builder.rs | 3 +- tasks/coverage/babel | 2 +- tasks/coverage/typescript | 2 +- 11 files changed, 389 insertions(+), 24 deletions(-) create mode 100644 crates/oxc_linter/src/disable_directives.rs diff --git a/Cargo.lock b/Cargo.lock index 5137f4c07..79bf0cf02 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -931,6 +931,7 @@ dependencies = [ "oxc_printer", "oxc_semantic", "phf", + "rust-lapper", "rustc-hash", "serde_json", ] @@ -1202,6 +1203,15 @@ dependencies = [ "ureq", ] +[[package]] +name = "rust-lapper" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee43d8e721ac803031dbab6a944b957b49a3b11eadbc099880c8aaaebf23ed27" +dependencies = [ + "num-traits", +] + [[package]] name = "rustc-hash" version = "1.1.0" diff --git a/crates/oxc_ast/src/trivia.rs b/crates/oxc_ast/src/trivia.rs index cbac0516e..07ce158fb 100644 --- a/crates/oxc_ast/src/trivia.rs +++ b/crates/oxc_ast/src/trivia.rs @@ -1,5 +1,5 @@ //! Trivia (called that because it's trivial) represent the parts of the source text that are largely insignificant for normal understanding of the code. -//! For example; whitespace, comments, and even conflict markers. +//! For example: whitespace, comments, and even conflict markers. use std::collections::BTreeMap; @@ -18,7 +18,7 @@ pub struct Comment { end: u32, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum CommentKind { SingleLine, MultiLine, @@ -29,9 +29,29 @@ impl Comment { pub fn new(end: u32, kind: CommentKind) -> Self { Self { kind, end } } + + #[must_use] + pub fn end(&self) -> u32 { + self.end + } + + #[must_use] + pub fn is_single_line(&self) -> bool { + self.kind == CommentKind::SingleLine + } + + #[must_use] + pub fn is_multi_line(&self) -> bool { + self.kind == CommentKind::MultiLine + } } impl Trivias { + #[must_use] + pub fn comments(&self) -> &BTreeMap { + &self.comments + } + #[must_use] pub fn has_comments_between(&self, span: Span) -> bool { self.comments.range(span.start..span.end).count() > 0 diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml index 3acc2084f..677063546 100644 --- a/crates/oxc_linter/Cargo.toml +++ b/crates/oxc_linter/Cargo.toml @@ -19,9 +19,10 @@ oxc_printer = { path = "../oxc_printer" } lazy_static = { workspace = true } serde_json = { workspace = true } indextree = { workspace = true } -phf = { version = "0.11", features = ["macros"] } rustc-hash = { workspace = true } +phf = { version = "0.11", features = ["macros"] } num-traits = "0.2.15" +rust-lapper = "1.1.0" [dev_dependencies] oxc_allocator = { path = "../oxc_allocator" } diff --git a/crates/oxc_linter/src/context.rs b/crates/oxc_linter/src/context.rs index 98790d447..4e372490a 100644 --- a/crates/oxc_linter/src/context.rs +++ b/crates/oxc_linter/src/context.rs @@ -7,6 +7,7 @@ use oxc_printer::{Printer, PrinterOptions}; use oxc_semantic::{AstNodes, Scope, ScopeTree, Semantic, SemanticNode}; use crate::{ + disable_directives::{DisableDirectives, DisableDirectivesBuilder}, fixer::{Fix, Message}, AstNode, }; @@ -18,13 +19,26 @@ pub struct LintContext<'a> { diagnostics: RefCell>>, + disable_directives: DisableDirectives<'a>, + /// Whether or not to apply code fixes during linting. fix: bool, + + current_rule_name: &'static str, } impl<'a> LintContext<'a> { pub fn new(source_text: &'a str, semantic: &Rc>, fix: bool) -> Self { - Self { source_text, semantic: Rc::clone(semantic), diagnostics: RefCell::new(vec![]), fix } + let disable_directives = + DisableDirectivesBuilder::new(source_text, semantic.trivias()).build(); + Self { + source_text, + semantic: Rc::clone(semantic), + diagnostics: RefCell::new(vec![]), + disable_directives, + fix, + current_rule_name: "", + } } #[must_use] @@ -42,14 +56,24 @@ impl<'a> LintContext<'a> { self.semantic().source_type() } + pub fn with_rule_name(&mut self, name: &'static str) { + self.current_rule_name = name; + } + /* Diagnostics */ pub fn into_message(self) -> Vec> { self.diagnostics.into_inner() } + fn add_diagnostic(&self, message: Message<'a>) { + if !self.disable_directives.contains(self.current_rule_name, message.start()) { + self.diagnostics.borrow_mut().push(message); + } + } + pub fn diagnostic>(&self, diagnostic: T) { - self.diagnostics.borrow_mut().push(Message::new(diagnostic.into(), None)); + self.add_diagnostic(Message::new(diagnostic.into(), None)); } pub fn diagnostic_with_fix(&self, diagnostic: T, fix: F) @@ -58,7 +82,7 @@ impl<'a> LintContext<'a> { F: FnOnce() -> Fix<'a>, { if self.fix { - self.diagnostics.borrow_mut().push(Message::new(diagnostic.into(), Some(fix()))); + self.add_diagnostic(Message::new(diagnostic.into(), Some(fix()))); } else { self.diagnostic(diagnostic); } diff --git a/crates/oxc_linter/src/disable_directives.rs b/crates/oxc_linter/src/disable_directives.rs new file mode 100644 index 000000000..8a18b46eb --- /dev/null +++ b/crates/oxc_linter/src/disable_directives.rs @@ -0,0 +1,284 @@ +use oxc_ast::{Span, Trivias}; +use rust_lapper::{Interval, Lapper}; +use rustc_hash::FxHashMap; + +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +enum DisabledRule<'a> { + All, + Single(&'a str), +} + +#[derive(Debug)] +pub struct DisableDirectives<'a> { + /// All the disabled rules with their corresponding covering spans + intervals: Lapper>, +} + +impl<'a> DisableDirectives<'a> { + pub fn contains(&self, rule_name: &'static str, start: u32) -> bool { + self.intervals.find(start, start + 1).any(|interval| { + interval.val == DisabledRule::All || interval.val == DisabledRule::Single(rule_name) + }) + } +} + +pub struct DisableDirectivesBuilder<'a, 'b> { + source_text: &'a str, + trivias: &'b Trivias, + /// All the disabled rules with their corresponding covering spans + intervals: Lapper>, + /// Start of `eslint-disable` + disable_all_start: Option, + /// Start of `eslint-disable rule_name` + disable_start_map: FxHashMap<&'a str, u32>, +} + +impl<'a, 'b> DisableDirectivesBuilder<'a, 'b> { + pub fn new(source_text: &'a str, trivias: &'b Trivias) -> Self { + Self { + source_text, + trivias, + intervals: Lapper::new(vec![]), + disable_all_start: None, + disable_start_map: FxHashMap::default(), + } + } + + pub fn build(mut self) -> DisableDirectives<'a> { + self.build_impl(); + DisableDirectives { intervals: self.intervals } + } + + fn add_interval(&mut self, start: u32, stop: u32, val: DisabledRule<'a>) { + self.intervals.insert(Interval { start, stop, val }); + } + + #[allow(clippy::cast_possible_truncation)] // for `as u32` + fn build_impl(&mut self) { + let source_len = self.source_text.len() as u32; + // This algorithm iterates through the comments and builds all intervals + // for matching disable and enable pairs. + // Wrongly ordered matching pairs are not taken into consideration. + for (start, comment) in self.trivias.comments() { + let span = Span::new(*start, comment.end()); + let text = span.source_text(self.source_text); + let text = text.trim_start(); + + if let Some(text) = text.strip_prefix("eslint-disable") { + // `eslint-disable` + if text.trim().is_empty() { + self.disable_all_start = Some(span.end); + continue; + } + + // `eslint-disable-next-line` + if let Some(text) = text.strip_prefix("-next-line") { + // Get the span up to the next new line + let stop = self.source_text[span.end as usize..] + .lines() + .take(if comment.is_single_line() { 1 } else { 2 }) + .map(|line| span.end + line.len() as u32) + .sum(); + if text.trim().is_empty() { + self.add_interval(span.end, stop, DisabledRule::All); + } else { + // `eslint-disable-next-line rule_name1, rule_name2` + Self::get_rule_names(text, |rule_name| { + self.add_interval(span.end, stop, DisabledRule::Single(rule_name)); + }); + } + continue; + } + + // `eslint-disable-line` + if let Some(text) = text.strip_prefix("-line") { + // Get the span between the preceding newline to this comment + let start = self.source_text[..=span.start as usize] + .lines() + .rev() + .next() + .map_or(0, |line| span.start - line.len() as u32); + let stop = span.start; + + // `eslint-disable-line` + if text.trim().is_empty() { + self.add_interval(start, stop, DisabledRule::All); + } else { + // `eslint-disable-line rule-name1, rule-name2` + Self::get_rule_names(text, |rule_name| { + self.add_interval(start, stop, DisabledRule::Single(rule_name)); + }); + } + continue; + } + + // `eslint-disable rule-name1, rule-name2` + Self::get_rule_names(text, |rule_name| { + self.disable_start_map.insert(rule_name, span.end); + }); + + continue; + } + + if let Some(text) = text.strip_prefix("eslint-enable") { + // `eslint-enable` + if text.trim().is_empty() { + if let Some(start) = self.disable_all_start.take() { + self.add_interval(start, span.start, DisabledRule::All); + } + } else { + // `eslint-enable rule-name1, rule-name2` + Self::get_rule_names(text, |rule_name| { + if let Some(start) = self.disable_start_map.remove(rule_name) { + self.add_interval(start, span.start, DisabledRule::Single(rule_name)); + } + }); + } + continue; + } + } + + // Lone `eslint-disable` + if let Some(start) = self.disable_all_start { + self.add_interval(start, source_len, DisabledRule::All); + } + + // Lone `eslint-disable rule_name` + for (rule_name, start) in self.disable_start_map.drain().collect::>() { + self.add_interval(start, source_len, DisabledRule::Single(rule_name)); + } + } + + fn get_rule_names(text: &'a str, cb: F) { + if let Some(text) = text.split_terminator("--").next() { + text.split(',').map(str::trim).for_each(cb); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + // [Disabling Rules](https://eslint.org/docs/latest/use/configure/rules#disabling-rules) + // Using configuration comments + let pass = vec![ + // To disable rule warnings in a part of a file, use block comments in the following format: + " + /* eslint-disable */ + debugger; + /* eslint-enable */ + ", + // You can also disable or enable warnings for specific rules: + " + /* eslint-disable no-debugger, no-console */ + debugger; + /* eslint-enable no-debugger, no-console */ + ", + // To disable rule warnings in an entire file, put a /* eslint-disable */ block comment at the top of the file: + " + /* eslint-disable */ + debugger; + ", + // You can also disable or enable specific rules for an entire file: + " + /* eslint-disable no-debugger */ + debugger; + ", + // To ensure that a rule is never applied (regardless of any future enable/disable lines): + // This is not supported. + // " + // /* eslint no-debugger: \"off\" */ + // debugger; + // ", + // To disable all rules on a specific line, use a line or block comment in one of the following formats: + " + debugger; // eslint-disable-line + + // eslint-disable-next-line + debugger; + + /* eslint-disable-next-line */ + debugger; + + debugger; /* eslint-disable-line */ + ", + // To disable a specific rule on a specific line: + " + debugger; // eslint-disable-line no-debugger + + // eslint-disable-next-line no-debugger + debugger; + + debugger; /* eslint-disable-line no-debugger */ + + /* eslint-disable-next-line no-debugger */ + debugger; + ", + // To disable multiple rules on a specific line: + " + debugger; // eslint-disable-line no-debugger, quotes, semi + + // eslint-disable-next-line no-debugger, quotes, semi + debugger; + + debugger; /* eslint-disable-line no-debugger, quotes, semi */ + + /* eslint-disable-next-line no-debugger, quotes, semi */ + debugger; + + /* eslint-disable-next-line + no-debugger, + quotes, + semi + */ + debugger; + ", + // Comment descriptions + " + // eslint-disable-next-line no-debugger -- Here's a description about why this configuration is necessary. + debugger; + + /* eslint-disable-next-line no-debugger -- + * Here's a very long description about why this configuration is necessary + * along with some additional information + **/ + debugger; + " + ]; + + let fail = vec![ + "debugger", + " + debugger; // eslint-disable-line no-alert + + // eslint-disable-next-line no-alert + debugger; + + debugger; /* eslint-disable-line no-alert */ + + /* eslint-disable-next-line no-alert */ + debugger; + ", + " + debugger; // eslint-disable-line no-alert, quotes, semi + + // eslint-disable-next-line no-alert, quotes, semi + debugger; + + debugger; /* eslint-disable-line no-alert, quotes, semi */ + + /* eslint-disable-next-line no-alert, quotes, semi */ + debugger; + + /* eslint-disable-next-line + no-alert, + quotes, + semi + */ + debugger; + ", + ]; + + Tester::new_without_config("no-debugger", pass, fail).test(); +} diff --git a/crates/oxc_linter/src/fixer.rs b/crates/oxc_linter/src/fixer.rs index 3ca6868f3..4bd1d9bbd 100644 --- a/crates/oxc_linter/src/fixer.rs +++ b/crates/oxc_linter/src/fixer.rs @@ -1,7 +1,7 @@ use std::borrow::Cow; use oxc_ast::Span; -use oxc_diagnostics::{miette::LabeledSpan, Error}; +use oxc_diagnostics::Error; #[derive(Debug, Default)] pub struct Fix<'a> { @@ -29,14 +29,34 @@ pub struct FixResult<'a> { #[derive(Debug)] pub struct Message<'a> { pub error: Error, + start: u32, + end: u32, fix: Option>, fixed: bool, } impl<'a> Message<'a> { + #[allow(clippy::cast_possible_truncation)] // for `as u32` #[must_use] pub fn new(error: Error, fix: Option>) -> Self { - Self { error, fix, fixed: false } + let labels = error.labels().map_or(vec![], Iterator::collect); + let start = + labels.iter().min_by_key(|span| span.offset()).map_or(0, |span| span.offset() as u32); + let end = labels + .iter() + .max_by_key(|span| span.offset() + span.len()) + .map_or(0, |span| (span.offset() + span.len()) as u32); + Self { error, start, end, fix, fixed: false } + } + + #[must_use] + pub fn start(&self) -> u32 { + self.start + } + + #[must_use] + pub fn end(&self) -> u32 { + self.end } } @@ -92,16 +112,7 @@ impl<'a> Fixer<'a> { output.push_str(&source_text[offset..]); let mut messages = self.messages.into_iter().filter(|m| !m.fixed).collect::>(); - messages.sort_by_cached_key(|m| { - let span = m - .error - .labels() - .expect("should specify a span for a rule") - .min_by_key(LabeledSpan::offset) - .expect("should contain at least one span"); - (span.offset(), span.len()) - }); - + messages.sort_by_key(|m| (m.start, m.end)); return FixResult { fixed, fixed_code: Cow::Owned(output), messages }; } } diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 99ebd3b41..cc68370fc 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -6,6 +6,7 @@ mod tester; mod ast_util; mod context; +mod disable_directives; mod fixer; mod globals; pub mod rule; @@ -85,11 +86,12 @@ impl Linter { #[must_use] pub fn run<'a>(&self, semantic: &Rc>, source_text: &'a str) -> Vec> { - let ctx = LintContext::new(source_text, semantic, self.fix); + let mut ctx = LintContext::new(source_text, semantic, self.fix); for node in semantic.nodes().iter() { self.early_error_javascript.run(node, &ctx); for rule in &self.rules { + ctx.with_rule_name(rule.name()); rule.run(node, &ctx); } } diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index 6a966b608..597512859 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -27,9 +27,23 @@ impl Tester { Self { rule_name, expect_pass, expect_fail, snapshot: String::new() } } - pub fn test_and_snapshot(&mut self) { + pub fn new_without_config>( + rule_name: &'static str, + expect_pass: Vec, + expect_fail: Vec, + ) -> Self { + let expect_pass = expect_pass.into_iter().map(|s| (s.into(), None)).collect::>(); + let expect_fail = expect_fail.into_iter().map(|s| (s.into(), None)).collect::>(); + Self { rule_name, expect_pass, expect_fail, snapshot: String::new() } + } + + pub fn test(&mut self) { self.test_pass(); self.test_fail(); + } + + pub fn test_and_snapshot(&mut self) { + self.test(); self.snapshot(); } diff --git a/crates/oxc_parser/src/lexer/trivia_builder.rs b/crates/oxc_parser/src/lexer/trivia_builder.rs index 49f3b4e86..72c261db2 100644 --- a/crates/oxc_parser/src/lexer/trivia_builder.rs +++ b/crates/oxc_parser/src/lexer/trivia_builder.rs @@ -6,7 +6,6 @@ pub struct TriviaBuilder { } impl TriviaBuilder { - #[allow(clippy::missing_const_for_fn)] pub fn build(self) -> Trivias { self.trivias } @@ -17,7 +16,7 @@ impl TriviaBuilder { } pub fn add_multi_line_comment(&mut self, start: u32, end: u32) { - // skip leading `/*` and trailing */ + // skip leading `/*` and trailing `*/` self.trivias.add_comment(Span::new(start + 2, end - 2), CommentKind::MultiLine); } } diff --git a/tasks/coverage/babel b/tasks/coverage/babel index 6925ac555..032203ea1 160000 --- a/tasks/coverage/babel +++ b/tasks/coverage/babel @@ -1 +1 @@ -Subproject commit 6925ac555c62c67886b86e38c0c6a7dca60b3cb7 +Subproject commit 032203ea18288b9ae51f0c18dfece03a59555113 diff --git a/tasks/coverage/typescript b/tasks/coverage/typescript index 7f292bf2a..b168b246b 160000 --- a/tasks/coverage/typescript +++ b/tasks/coverage/typescript @@ -1 +1 @@ -Subproject commit 7f292bf2a19aa14ed69a55e646111af9533d8f1c +Subproject commit b168b246b79dc7891bd35d973454a8c862fd3c82