feat(linter): let fixer functions return a None fix (#4210)

Part of #4187.

Adds `CompositeFix::None`, which enables fixer functions to decide not to fix some code.

While I was in the area, I took the liberty of adding some doc comments.
This commit is contained in:
DonIsaac 2024-07-12 01:12:42 +00:00
parent dd07a5475b
commit 3016f03578
4 changed files with 104 additions and 18 deletions

View file

@ -1,3 +1,4 @@
#![allow(rustdoc::private_intra_doc_links)] // useful for intellisense
use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc}; use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};
use oxc_cfg::ControlFlowGraph; use oxc_cfg::ControlFlowGraph;
@ -18,11 +19,16 @@ use crate::{
pub struct LintContext<'a> { pub struct LintContext<'a> {
semantic: Rc<Semantic<'a>>, semantic: Rc<Semantic<'a>>,
/// Diagnostics reported by the linter.
///
/// Contains diagnostics for all rules across all files.
diagnostics: RefCell<Vec<Message<'a>>>, diagnostics: RefCell<Vec<Message<'a>>>,
disable_directives: Rc<DisableDirectives<'a>>, disable_directives: Rc<DisableDirectives<'a>>,
/// Whether or not to apply code fixes during linting. /// Whether or not to apply code fixes during linting. Defaults to `false`.
///
/// Set via the `--fix` CLI flag.
fix: bool, fix: bool,
file_path: Rc<Path>, file_path: Rc<Path>,
@ -32,6 +38,15 @@ pub struct LintContext<'a> {
// states // states
current_rule_name: &'static str, current_rule_name: &'static str,
/// Current rule severity. Allows for user severity overrides, e.g.
/// ```json
/// // .oxlintrc.json
/// {
/// "rules": {
/// "no-debugger": "error"
/// }
/// }
/// ```
severity: Severity, severity: Severity,
} }
@ -39,6 +54,8 @@ impl<'a> LintContext<'a> {
/// # Panics /// # Panics
/// If `semantic.cfg()` is `None`. /// If `semantic.cfg()` is `None`.
pub fn new(file_path: Box<Path>, semantic: Rc<Semantic<'a>>) -> Self { pub fn new(file_path: Box<Path>, semantic: Rc<Semantic<'a>>) -> Self {
const DIAGNOSTICS_INITIAL_CAPACITY: usize = 128;
// We should always check for `semantic.cfg()` being `Some` since we depend on it and it is // We should always check for `semantic.cfg()` being `Some` since we depend on it and it is
// unwrapped without any runtime checks after construction. // unwrapped without any runtime checks after construction.
assert!( assert!(
@ -50,7 +67,7 @@ impl<'a> LintContext<'a> {
.build(); .build();
Self { Self {
semantic, semantic,
diagnostics: RefCell::new(vec![]), diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
disable_directives: Rc::new(disable_directives), disable_directives: Rc::new(disable_directives),
fix: false, fix: false,
file_path: file_path.into(), file_path: file_path.into(),
@ -60,6 +77,7 @@ impl<'a> LintContext<'a> {
} }
} }
/// Enable/disable automatic code fixes.
#[must_use] #[must_use]
pub fn with_fix(mut self, fix: bool) -> Self { pub fn with_fix(mut self, fix: bool) -> Self {
self.fix = fix; self.fix = fix;
@ -112,14 +130,17 @@ impl<'a> LintContext<'a> {
span.source_text(self.semantic().source_text()) span.source_text(self.semantic().source_text())
} }
/// [`SourceType`] of the file currently being linted.
pub fn source_type(&self) -> &SourceType { pub fn source_type(&self) -> &SourceType {
self.semantic().source_type() self.semantic().source_type()
} }
/// Path to the file currently being linted.
pub fn file_path(&self) -> &Path { pub fn file_path(&self) -> &Path {
&self.file_path &self.file_path
} }
/// Plugin settings
pub fn settings(&self) -> &OxlintSettings { pub fn settings(&self) -> &OxlintSettings {
&self.eslint_config.settings &self.eslint_config.settings
} }
@ -128,6 +149,9 @@ impl<'a> LintContext<'a> {
&self.eslint_config.globals &self.eslint_config.globals
} }
/// Runtime environments turned on/off by the user.
///
/// Examples of environments are `builtin`, `browser`, `node`, etc.
pub fn env(&self) -> &OxlintEnv { pub fn env(&self) -> &OxlintEnv {
&self.eslint_config.env &self.eslint_config.env
} }
@ -174,6 +198,11 @@ impl<'a> LintContext<'a> {
} }
/// Report a lint rule violation and provide an automatic fix. /// Report a lint rule violation and provide an automatic fix.
///
/// The second argument is a [closure] that takes a [`RuleFixer`] and
/// returns something that can turn into a [`CompositeFix`].
///
/// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F) pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
where where
C: Into<CompositeFix<'a>>, C: Into<CompositeFix<'a>>,
@ -189,23 +218,37 @@ impl<'a> LintContext<'a> {
} }
} }
/// AST nodes
///
/// Shorthand for `self.semantic().nodes()`.
pub fn nodes(&self) -> &AstNodes<'a> { pub fn nodes(&self) -> &AstNodes<'a> {
self.semantic().nodes() self.semantic().nodes()
} }
/// Scope tree
///
/// Shorthand for `ctx.semantic().scopes()`.
pub fn scopes(&self) -> &ScopeTree { pub fn scopes(&self) -> &ScopeTree {
self.semantic().scopes() self.semantic().scopes()
} }
/// Symbol table
///
/// Shorthand for `ctx.semantic().symbols()`.
pub fn symbols(&self) -> &SymbolTable { pub fn symbols(&self) -> &SymbolTable {
self.semantic().symbols() self.semantic().symbols()
} }
/// Imported modules and exported symbols
///
/// Shorthand for `ctx.semantic().module_record()`.
pub fn module_record(&self) -> &ModuleRecord { pub fn module_record(&self) -> &ModuleRecord {
self.semantic().module_record() self.semantic().module_record()
} }
/* JSDoc */ /// JSDoc comments
///
/// Shorthand for `ctx.semantic().jsdoc()`.
pub fn jsdoc(&self) -> &JSDocFinder<'a> { pub fn jsdoc(&self) -> &JSDocFinder<'a> {
self.semantic().jsdoc() self.semantic().jsdoc()
} }

View file

@ -2,16 +2,22 @@ use std::borrow::Cow;
use oxc_codegen::Codegen; use oxc_codegen::Codegen;
use oxc_diagnostics::OxcDiagnostic; use oxc_diagnostics::OxcDiagnostic;
use oxc_span::{GetSpan, Span}; use oxc_span::{GetSpan, Span, SPAN};
use crate::LintContext; use crate::LintContext;
#[derive(Debug, Clone, Default)] #[derive(Debug, Clone)]
pub struct Fix<'a> { pub struct Fix<'a> {
pub content: Cow<'a, str>, pub content: Cow<'a, str>,
pub span: Span, pub span: Span,
} }
impl Default for Fix<'_> {
fn default() -> Self {
Self::empty()
}
}
impl<'a> Fix<'a> { impl<'a> Fix<'a> {
pub const fn delete(span: Span) -> Self { pub const fn delete(span: Span) -> Self {
Self { content: Cow::Borrowed(""), span } Self { content: Cow::Borrowed(""), span }
@ -20,10 +26,21 @@ impl<'a> Fix<'a> {
pub fn new<T: Into<Cow<'a, str>>>(content: T, span: Span) -> Self { pub fn new<T: Into<Cow<'a, str>>>(content: T, span: Span) -> Self {
Self { content: content.into(), span } Self { content: content.into(), span }
} }
/// Creates a [`Fix`] that doesn't change the source code.
#[inline]
pub const fn empty() -> Self {
Self { content: Cow::Borrowed(""), span: SPAN }
}
} }
#[derive(Debug, Default)]
pub enum CompositeFix<'a> { pub enum CompositeFix<'a> {
/// No fixes
#[default]
None,
Single(Fix<'a>), Single(Fix<'a>),
/// Several fixes that will be merged into one, in order.
Multiple(Vec<Fix<'a>>), Multiple(Vec<Fix<'a>>),
} }
@ -33,11 +50,24 @@ impl<'a> From<Fix<'a>> for CompositeFix<'a> {
} }
} }
impl<'a> From<Option<Fix<'a>>> for CompositeFix<'a> {
fn from(fix: Option<Fix<'a>>) -> Self {
match fix {
Some(fix) => CompositeFix::Single(fix),
None => CompositeFix::None,
}
}
}
impl<'a> From<Vec<Fix<'a>>> for CompositeFix<'a> { impl<'a> From<Vec<Fix<'a>>> for CompositeFix<'a> {
fn from(fixes: Vec<Fix<'a>>) -> Self { fn from(fixes: Vec<Fix<'a>>) -> Self {
if fixes.is_empty() {
CompositeFix::None
} else {
CompositeFix::Multiple(fixes) CompositeFix::Multiple(fixes)
} }
} }
}
impl<'a> CompositeFix<'a> { impl<'a> CompositeFix<'a> {
/// Gets one fix from the fixes. If we retrieve multiple fixes, this merges those into one. /// Gets one fix from the fixes. If we retrieve multiple fixes, this merges those into one.
@ -46,19 +76,21 @@ impl<'a> CompositeFix<'a> {
match self { match self {
CompositeFix::Single(fix) => fix, CompositeFix::Single(fix) => fix,
CompositeFix::Multiple(fixes) => Self::merge_fixes(fixes, source_text), CompositeFix::Multiple(fixes) => Self::merge_fixes(fixes, source_text),
CompositeFix::None => Fix::empty(),
} }
} }
// Merges multiple fixes to one, returns an `Fix::default`(which will not fix anything) if: /// Merges multiple fixes to one, returns an `Fix::default`(which will not fix anything) if:
// 1. `fixes` is empty ///
// 2. contains overlapped ranges /// 1. `fixes` is empty
// 3. contains negative ranges (span.start > span.end) /// 2. contains overlapped ranges
// <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L147-L179> /// 3. contains negative ranges (span.start > span.end)
///
/// <https://github.com/eslint/eslint/blob/main/lib/linter/report-translator.js#L147-L179>
fn merge_fixes(fixes: Vec<Fix<'a>>, source_text: &str) -> Fix<'a> { fn merge_fixes(fixes: Vec<Fix<'a>>, source_text: &str) -> Fix<'a> {
let mut fixes = fixes; let mut fixes = fixes;
let empty_fix = Fix::default();
if fixes.is_empty() { if fixes.is_empty() {
// Do nothing // Do nothing
return empty_fix; return Fix::empty();
} }
if fixes.len() == 1 { if fixes.len() == 1 {
return fixes.pop().unwrap(); return fixes.pop().unwrap();
@ -77,7 +109,7 @@ impl<'a> CompositeFix<'a> {
// negative range or overlapping ranges is invalid // negative range or overlapping ranges is invalid
if span.start > span.end { if span.start > span.end {
debug_assert!(false, "Negative range is invalid: {span:?}"); debug_assert!(false, "Negative range is invalid: {span:?}");
return empty_fix; return Fix::empty();
} }
if last_pos > span.start { if last_pos > span.start {
debug_assert!( debug_assert!(
@ -85,14 +117,15 @@ impl<'a> CompositeFix<'a> {
"Fix must not be overlapped, last_pos: {}, span.start: {}", "Fix must not be overlapped, last_pos: {}, span.start: {}",
last_pos, span.start last_pos, span.start
); );
return empty_fix; return Fix::empty();
} }
let Some(before) = source_text.get((last_pos) as usize..span.start as usize) else { let Some(before) = source_text.get((last_pos) as usize..span.start as usize) else {
debug_assert!(false, "Invalid range: {}, {}", last_pos, span.start); debug_assert!(false, "Invalid range: {}, {}", last_pos, span.start);
return empty_fix; return Fix::empty();
}; };
output.reserve(before.len() + content.len());
output.push_str(before); output.push_str(before);
output.push_str(content); output.push_str(content);
last_pos = span.end; last_pos = span.end;
@ -100,10 +133,11 @@ impl<'a> CompositeFix<'a> {
let Some(after) = source_text.get(last_pos as usize..end as usize) else { let Some(after) = source_text.get(last_pos as usize..end as usize) else {
debug_assert!(false, "Invalid range: {:?}", last_pos as usize..end as usize); debug_assert!(false, "Invalid range: {:?}", last_pos as usize..end as usize);
return empty_fix; return Fix::empty();
}; };
output.push_str(after); output.push_str(after);
output.shrink_to_fit();
Fix::new(output, Span::new(start, end)) Fix::new(output, Span::new(start, end))
} }
} }
@ -121,6 +155,8 @@ impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
Self { ctx } Self { ctx }
} }
/// Get a snippet of source text covered by the given [`Span`]. For details,
/// see [`Span::source_text`].
pub fn source_range(self, span: Span) -> &'a str { pub fn source_range(self, span: Span) -> &'a str {
self.ctx.source_range(span) self.ctx.source_range(span)
} }
@ -186,6 +222,11 @@ impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
pub fn codegen(self) -> Codegen<'a, false> { pub fn codegen(self) -> Codegen<'a, false> {
Codegen::<false>::new() Codegen::<false>::new()
} }
#[allow(clippy::unused_self)]
pub fn noop(self) -> Fix<'a> {
Fix::empty()
}
} }
pub struct FixResult<'a> { pub struct FixResult<'a> {

View file

@ -49,6 +49,7 @@ fn size_asserts() {
assert_eq_size!(RuleEnum, [u8; 16]); assert_eq_size!(RuleEnum, [u8; 16]);
} }
#[derive(Debug)]
pub struct Linter { pub struct Linter {
rules: Vec<RuleWithSeverity>, rules: Vec<RuleWithSeverity>,
options: LintOptions, options: LintOptions,
@ -83,6 +84,7 @@ impl Linter {
self self
} }
/// Enable/disable automatic code fixes.
#[must_use] #[must_use]
pub fn with_fix(mut self, yes: bool) -> Self { pub fn with_fix(mut self, yes: bool) -> Self {
self.options.fix = yes; self.options.fix = yes;

View file

@ -100,7 +100,7 @@ impl fmt::Display for RuleCategory {
} }
} }
#[derive(Clone)] #[derive(Debug, Clone)]
pub struct RuleWithSeverity { pub struct RuleWithSeverity {
pub rule: RuleEnum, pub rule: RuleEnum,
pub severity: AllowWarnDeny, pub severity: AllowWarnDeny,