Skip to content

Commit

Permalink
refactor(linter): add rule fixer (#3589)
Browse files Browse the repository at this point in the history
Adds a new `RuleFixer` struct that gets passed to
`ctx.diagnostic_with_fix`.
  • Loading branch information
DonIsaac authored Jun 10, 2024
1 parent 8212090 commit f98f777
Show file tree
Hide file tree
Showing 62 changed files with 587 additions and 619 deletions.
8 changes: 1 addition & 7 deletions crates/oxc_ast/src/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,6 @@ impl<'a> GetSpan for Expression<'a> {
}
}

impl<'a> GetSpan for Directive<'a> {
fn span(&self) -> Span {
self.span
}
}

impl<'a> GetSpan for BindingPatternKind<'a> {
fn span(&self) -> Span {
match self {
Expand All @@ -122,7 +116,7 @@ impl<'a> GetSpan for BindingPattern<'a> {
}
}

impl<'a> GetSpan for BindingProperty<'a> {
impl GetSpan for BindingProperty<'_> {
fn span(&self) -> Span {
self.span
}
Expand Down
19 changes: 18 additions & 1 deletion crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mod gen_ts;
mod operator;
mod sourcemap_builder;

use std::ops::Range;
use std::{borrow::Cow, ops::Range};

#[allow(clippy::wildcard_imports)]
use oxc_ast::ast::*;
Expand Down Expand Up @@ -524,3 +524,20 @@ fn choose_quote(s: &str) -> char {
'\''
}
}

impl From<CodegenReturn> for String {
fn from(val: CodegenReturn) -> Self {
val.source_text
}
}

impl<'a, const MINIFY: bool> From<Codegen<'a, MINIFY>> for String {
fn from(mut val: Codegen<'a, MINIFY>) -> Self {
val.into_source_text()
}
}
impl<'a, const MINIFY: bool> From<Codegen<'a, MINIFY>> for Cow<'a, str> {
fn from(mut val: Codegen<'a, MINIFY>) -> Self {
Cow::Owned(val.into_source_text())
}
}
17 changes: 8 additions & 9 deletions crates/oxc_linter/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};

use oxc_codegen::{Codegen, CodegenOptions};
use oxc_diagnostics::{OxcDiagnostic, Severity};
use oxc_semantic::{AstNodes, JSDocFinder, ScopeTree, Semantic, SymbolTable};
use oxc_span::{SourceType, Span};

use crate::{
disable_directives::{DisableDirectives, DisableDirectivesBuilder},
fixer::{Fix, Message},
fixer::{Fix, Message, RuleFixer},
javascript_globals::GLOBALS,
AllowWarnDeny, OxlintConfig, OxlintEnv, OxlintGlobals, OxlintSettings,
};
Expand Down Expand Up @@ -147,9 +146,14 @@ impl<'a> LintContext<'a> {
}

/// Report a lint rule violation and provide an automatic fix.
pub fn diagnostic_with_fix<F: FnOnce() -> Fix<'a>>(&self, diagnostic: OxcDiagnostic, fix: F) {
pub fn diagnostic_with_fix<F: FnOnce(RuleFixer<'_, 'a>) -> Fix<'a>>(
&self,
diagnostic: OxcDiagnostic,
fix: F,
) {
if self.fix {
self.add_diagnostic(Message::new(diagnostic, Some(fix())));
let fixer = RuleFixer::new(self);
self.add_diagnostic(Message::new(diagnostic, Some(fix(fixer))));
} else {
self.diagnostic(diagnostic);
}
Expand All @@ -167,11 +171,6 @@ impl<'a> LintContext<'a> {
self.semantic().symbols()
}

#[allow(clippy::unused_self)]
pub fn codegen(&self) -> Codegen<false> {
Codegen::<false>::new("", "", CodegenOptions::default(), None)
}

/* JSDoc */
pub fn jsdoc(&self) -> &JSDocFinder<'a> {
self.semantic().jsdoc()
Expand Down
51 changes: 50 additions & 1 deletion crates/oxc_linter/src/fixer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::borrow::Cow;

use oxc_codegen::{Codegen, CodegenOptions};
use oxc_diagnostics::OxcDiagnostic;
use oxc_span::Span;
use oxc_span::{GetSpan, Span};

use crate::LintContext;

#[derive(Debug, Clone, Default)]
pub struct Fix<'a> {
Expand All @@ -19,6 +22,52 @@ impl<'a> Fix<'a> {
}
}

/// Inspired by ESLint's [`RuleFixer`].
///
/// [`RuleFixer`]: https://github.com/eslint/eslint/blob/main/lib/linter/rule-fixer.js
#[derive(Clone, Copy)]
pub struct RuleFixer<'c, 'a: 'c> {
ctx: &'c LintContext<'a>,
}

impl<'c, 'a: 'c> RuleFixer<'c, 'a> {
pub fn new(ctx: &'c LintContext<'a>) -> Self {
Self { ctx }
}

pub fn source_range(self, span: Span) -> &'a str {
self.ctx.source_range(span)
}

/// Create a [`Fix`] that deletes the text covered by the given [`Span`] or
/// AST node.
pub fn delete<S: GetSpan>(self, spanned: &S) -> Fix<'a> {
self.delete_range(spanned.span())
}

#[allow(clippy::unused_self)]
pub fn delete_range(self, span: Span) -> Fix<'a> {
Fix::delete(span)
}

/// Replace a `target` AST node with the source code of a `replacement` node..
pub fn replace_with<T: GetSpan, S: GetSpan>(self, target: &T, replacement: &S) -> Fix<'a> {
let replacement_text = self.ctx.source_range(replacement.span());
Fix::new(replacement_text, target.span())
}

/// Replace a `target` AST node with a `replacement` string.
#[allow(clippy::unused_self)]
pub fn replace<S: Into<Cow<'a, str>>>(self, target: Span, replacement: S) -> Fix<'a> {
Fix::new(replacement, target)
}

#[allow(clippy::unused_self)]
pub fn codegen(self) -> Codegen<'a, false> {
Codegen::<false>::new("", "", CodegenOptions::default(), None)
}
}

pub struct FixResult<'a> {
pub fixed: bool,
pub fixed_code: Cow<'a, str>,
Expand Down
7 changes: 4 additions & 3 deletions crates/oxc_linter/src/rules/eslint/eqeqeq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{BinaryOperator, UnaryOperator};

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn eqeqeq_diagnostic(x0: &str, x1: &str, span2: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("eslint(eqeqeq): Expected {x1} and instead saw {x0}"))
Expand Down Expand Up @@ -120,10 +120,11 @@ impl Rule for Eqeqeq {
if is_type_of_binary_bool || are_literals_and_same_type_bool {
ctx.diagnostic_with_fix(
eqeqeq_diagnostic(operator, preferred_operator, operator_span),
|| {
|fixer| {
let start = binary_expr.left.span().end;
let end = binary_expr.right.span().start;
Fix::new(preferred_operator_with_padding, Span::new(start, end))
let span = Span::new(start, end);
fixer.replace(span, preferred_operator_with_padding)
},
);
} else {
Expand Down
6 changes: 4 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_debugger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_debugger_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint(no-debugger): `debugger` statement is not allowed")
Expand Down Expand Up @@ -35,7 +35,9 @@ declare_oxc_lint!(
impl Rule for NoDebugger {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::DebuggerStatement(stmt) = node.kind() {
ctx.diagnostic_with_fix(no_debugger_diagnostic(stmt.span), || Fix::delete(stmt.span));
ctx.diagnostic_with_fix(no_debugger_diagnostic(stmt.span), |fixer| {
fixer.delete(&stmt.span)
});
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_div_regex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_div_regex_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(
Expand Down Expand Up @@ -38,8 +38,9 @@ impl Rule for NoDivRegex {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
if let AstKind::RegExpLiteral(lit) = node.kind() {
if lit.regex.pattern.starts_with('=') {
ctx.diagnostic_with_fix(no_div_regex_diagnostic(lit.span), || {
Fix::new("[=]", Span::new(lit.span.start + 1, lit.span.start + 2))
ctx.diagnostic_with_fix(no_div_regex_diagnostic(lit.span), |fixer| {
let span = Span::sized(lit.span.start + 1, 1);
fixer.replace(span, "[=]")
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_linter/src/rules/eslint/no_unsafe_negation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::{BinaryOperator, UnaryOperator};

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, fixer::RuleFixer, rule::Rule, AstNode};

fn no_unsafe_negation_diagnostic(x0: &str, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Unexpected logical not in the left hand side of '{x0}' operator"))
Expand Down Expand Up @@ -75,15 +75,15 @@ impl NoUnsafeNegation {

/// Precondition:
/// expr.left is `UnaryExpression` whose operator is '!'
fn report_with_fix(expr: &BinaryExpression, ctx: &LintContext<'_>) {
fn report_with_fix<'a>(expr: &BinaryExpression, ctx: &LintContext<'a>) {
use oxc_codegen::{Context, Gen};
// Diagnostic points at the unexpected negation
let diagnostic = no_unsafe_negation_diagnostic(expr.operator.as_str(), expr.left.span());

let fix_producer = || {
let fix_producer = |fixer: RuleFixer<'_, 'a>| {
// modify `!a instance of B` to `!(a instanceof B)`
let modified_code = {
let mut codegen = ctx.codegen();
let mut codegen = fixer.codegen();
codegen.print(b'!');
let Expression::UnaryExpression(left) = &expr.left else { unreachable!() };
codegen.print(b'(');
Expand All @@ -93,7 +93,7 @@ impl NoUnsafeNegation {
codegen.print(b')');
codegen.into_source_text()
};
Fix::new(modified_code, expr.span)
fixer.replace(expr.span, modified_code)
};

ctx.diagnostic_with_fix(diagnostic, fix_producer);
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/no_unused_labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{context::LintContext, fixer::Fix, rule::Rule};
use crate::{context::LintContext, rule::Rule};

fn no_unused_labels_diagnostic(x0: &str, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint(no-unused-labels): Disallow unused labels")
Expand Down Expand Up @@ -51,7 +51,7 @@ impl Rule for NoUnusedLabels {
// e.g. A: /* Comment */ function foo(){}
ctx.diagnostic_with_fix(
no_unused_labels_diagnostic(stmt.label.name.as_str(), stmt.label.span),
|| Fix::delete(stmt.label.span),
|fixer| fixer.delete_range(stmt.label.span),
);
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/rules/eslint/no_useless_escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_macros::declare_oxc_lint;
use oxc_semantic::AstNodeId;
use oxc_span::Span;

use crate::{context::LintContext, rule::Rule, AstNode, Fix};
use crate::{context::LintContext, rule::Rule, AstNode};

fn no_useless_escape_diagnostic(x0: char, span1: Span) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("eslint(no-useless-escape): Unnecessary escape character {x0:?}"))
Expand Down Expand Up @@ -84,8 +84,8 @@ fn check(ctx: &LintContext<'_>, node_id: AstNodeId, start: u32, offsets: &[usize

if !is_within_jsx_attribute_item(node_id, ctx) {
let span = Span::new(offset - 1, offset + len);
ctx.diagnostic_with_fix(no_useless_escape_diagnostic(c, span), || {
Fix::new(c.to_string(), span)
ctx.diagnostic_with_fix(no_useless_escape_diagnostic(c, span), |fixer| {
fixer.replace(span, c.to_string())
});
}
}
Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_linter/src/rules/eslint/unicode_bom.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use oxc_span::{Span, SPAN};

use crate::{context::LintContext, rule::Rule, Fix};
use crate::{context::LintContext, rule::Rule};

fn unexpected_unicode_bom_diagnostic(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint(unicode-bom): Unexpected Unicode BOM (Byte Order Mark)")
Expand Down Expand Up @@ -59,14 +59,14 @@ impl Rule for UnicodeBom {
let has_bomb = source.starts_with('');

if has_bomb && matches!(self.bom_option, BomOptionType::Never) {
ctx.diagnostic_with_fix(unexpected_unicode_bom_diagnostic(Span::new(0, 0)), || {
return Fix::delete(Span::new(0, 3));
ctx.diagnostic_with_fix(unexpected_unicode_bom_diagnostic(SPAN), |fixer| {
return fixer.delete_range(Span::new(0, 3));
});
}

if !has_bomb && matches!(self.bom_option, BomOptionType::Always) {
ctx.diagnostic_with_fix(expected_unicode_bom_diagnostic(Span::new(0, 0)), || {
return Fix::new("", Span::new(0, 0));
ctx.diagnostic_with_fix(expected_unicode_bom_diagnostic(SPAN), |fixer| {
return fixer.replace(SPAN, "");
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_linter/src/rules/eslint/use_isnan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use oxc_macros::declare_oxc_lint;
use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::BinaryOperator;

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn comparison_with_na_n(span0: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("eslint(use-isnan): Requires calls to isNaN() when checking for NaN")
Expand Down Expand Up @@ -90,13 +90,13 @@ impl Rule for UseIsnan {
}
AstKind::BinaryExpression(expr) if expr.operator.is_equality() => {
if is_nan_identifier(&expr.left) {
ctx.diagnostic_with_fix(comparison_with_na_n(expr.left.span()), || {
Fix::new(make_equality_fix(true, expr, ctx), expr.span)
ctx.diagnostic_with_fix(comparison_with_na_n(expr.left.span()), |fixer| {
fixer.replace(expr.span, make_equality_fix(true, expr, ctx))
});
}
if is_nan_identifier(&expr.right) {
ctx.diagnostic_with_fix(comparison_with_na_n(expr.right.span()), || {
Fix::new(make_equality_fix(false, expr, ctx), expr.span)
ctx.diagnostic_with_fix(comparison_with_na_n(expr.right.span()), |fixer| {
fixer.replace(expr.span, make_equality_fix(false, expr, ctx))
});
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/eslint/valid_typeof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use oxc_span::{GetSpan, Span};
use oxc_syntax::operator::UnaryOperator;
use phf::{phf_set, Set};

use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode};
use crate::{context::LintContext, rule::Rule, AstNode};

fn not_string(x0: Option<&'static str>, span1: Span) -> OxcDiagnostic {
let mut d = OxcDiagnostic::warn(
Expand Down Expand Up @@ -110,7 +110,7 @@ impl Rule for ValidTypeof {
sibling.span(),
)
},
|| Fix::new("\"undefined\"", sibling.span()),
|fixer| fixer.replace(sibling.span(), "\"undefined\""),
);
return;
}
Expand Down
Loading

0 comments on commit f98f777

Please sign in to comment.