Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

up(linter): add rule fixer #3589

Merged
merged 4 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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