Skip to content

Commit

Permalink
feat(linter): support suggestions and dangerous fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Jul 17, 2024
1 parent 697c0ef commit 8d98d2c
Show file tree
Hide file tree
Showing 17 changed files with 477 additions and 100 deletions.
27 changes: 26 additions & 1 deletion apps/oxlint/src/command/lint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{path::PathBuf, str::FromStr};

use bpaf::Bpaf;
use oxc_linter::AllowWarnDeny;
use oxc_linter::{AllowWarnDeny, FixKind};

use super::{
expand_glob,
Expand Down Expand Up @@ -122,6 +122,31 @@ pub struct FixOptions {
/// Fix as many issues as possible. Only unfixed issues are reported in the output
#[bpaf(switch)]
pub fix: bool,
/// Apply all safe fixes and more-risky suggestions. Overrides `--fix`.
#[bpaf(switch)]
pub fix_suggestions: bool,

/// Apply all fixes and suggestions, including dangerous ones. Overrides
/// `--fix` and `--fix-suggestions`. These fixes may create incorrect code.
#[bpaf(switch)]
pub fix_dangerously: bool,
}
impl FixOptions {
pub fn fix_kind(&self) -> Option<FixKind> {
if self.fix_dangerously {
Some(FixKind::DangerousFix)
} else if self.fix_suggestions {
Some(FixKind::Suggestion)
} else if self.fix {
Some(FixKind::SafeFix)
} else {
None
}
}

pub fn is_enabled(&self) -> bool {
self.fix || self.fix_suggestions || self.fix_dangerously
}
}

/// Handle Warnings
Expand Down
2 changes: 1 addition & 1 deletion apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl Runner for LintRunner {
let lint_options = LintOptions::default()
.with_filter(filter)
.with_config_path(basic_options.config)
.with_fix(fix_options.fix)
.with_fix(fix_options.fix_kind())
.with_react_plugin(enable_plugins.react_plugin)
.with_unicorn_plugin(enable_plugins.unicorn_plugin)
.with_typescript_plugin(enable_plugins.typescript_plugin)
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_language_server/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use oxc_linter::{
AstroPartialLoader, JavaScriptSource, SveltePartialLoader, VuePartialLoader,
LINT_PARTIAL_LOADER_EXT,
},
LintContext, Linter,
FixKind, LintContext, Linter,
};
use oxc_parser::Parser;
use oxc_semantic::SemanticBuilder;
Expand Down Expand Up @@ -388,7 +388,7 @@ pub struct ServerLinter {

impl ServerLinter {
pub fn new() -> Self {
let linter = Linter::default().with_fix(true);
let linter = Linter::default().with_fix(Some(FixKind::SafeFix));
Self { linter: Arc::new(linter) }
}

Expand Down
6 changes: 4 additions & 2 deletions crates/oxc_language_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use futures::future::join_all;
use globset::Glob;
use ignore::gitignore::Gitignore;
use log::{debug, error, info};
use oxc_linter::{LintOptions, Linter};
use oxc_linter::{FixKind, LintOptions, Linter};
use serde::{Deserialize, Serialize};
use tokio::sync::{Mutex, OnceCell, RwLock, SetError};
use tower_lsp::{
Expand Down Expand Up @@ -345,7 +345,9 @@ impl Backend {
let mut linter = self.server_linter.write().await;
*linter = ServerLinter::new_with_linter(
Linter::from_options(
LintOptions::default().with_fix(true).with_config_path(Some(config_path)),
LintOptions::default()
.with_fix(Some(FixKind::SafeFix))
.with_config_path(Some(config_path)),
)
.expect("should have initialized linter with new options"),
);
Expand Down
99 changes: 88 additions & 11 deletions crates/oxc_linter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use oxc_syntax::module_record::ModuleRecord;
use crate::{
config::OxlintRules,
disable_directives::{DisableDirectives, DisableDirectivesBuilder},
fixer::{CompositeFix, Message, RuleFixer},
fixer::{FixKind, Message, RuleFix, RuleFixer},
javascript_globals::GLOBALS,
AllowWarnDeny, OxlintConfig, OxlintEnv, OxlintGlobals, OxlintSettings,
};
Expand All @@ -29,7 +29,7 @@ pub struct LintContext<'a> {
/// Whether or not to apply code fixes during linting. Defaults to `false`.
///
/// Set via the `--fix` CLI flag.
fix: bool,
fix: Option<FixKind>,

file_path: Rc<Path>,

Expand Down Expand Up @@ -69,7 +69,7 @@ impl<'a> LintContext<'a> {
semantic,
diagnostics: RefCell::new(Vec::with_capacity(DIAGNOSTICS_INITIAL_CAPACITY)),
disable_directives: Rc::new(disable_directives),
fix: false,
fix: None,
file_path: file_path.into(),
eslint_config: Arc::new(OxlintConfig::default()),
current_rule_name: "",
Expand All @@ -79,7 +79,7 @@ impl<'a> LintContext<'a> {

/// Enable/disable automatic code fixes.
#[must_use]
pub fn with_fix(mut self, fix: bool) -> Self {
pub fn with_fix(mut self, fix: Option<FixKind>) -> Self {
self.fix = fix;
self
}
Expand Down Expand Up @@ -190,26 +190,103 @@ impl<'a> LintContext<'a> {
/// Report a lint rule violation.
///
/// Use [`LintContext::diagnostic_with_fix`] to provide an automatic fix.
#[inline]
pub fn diagnostic(&self, diagnostic: OxcDiagnostic) {
self.add_diagnostic(Message::new(diagnostic, None));
}

/// 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`].
/// returns something that can turn into a `CompositeFix`.
///
/// Fixes created this way should not create parse errors or change the
/// semantics of the linted code. If your fix may change the code's
/// semantics, use [`LintContext::diagnostic_with_suggestion`] instead. If
/// your fix has the potential to create parse errors, use
/// [`LintContext::diagnostic_with_dangerous_fix`].
///
/// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
#[inline]
pub fn diagnostic_with_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
where
C: Into<CompositeFix<'a>>,
C: Into<RuleFix<'a>>,
F: FnOnce(RuleFixer<'_, 'a>) -> C,
{
self.diagnostic_with_fix_of_kind(diagnostic, FixKind::SafeFix, fix);
}

/// Report a lint rule violation and provide a suggestion for fixing it.
///
/// The second argument is a [closure] that takes a [`RuleFixer`] and
/// returns something that can turn into a `CompositeFix`.
///
/// Fixes created this way should not create parse errors, but have the
/// potential to change the code's semantics. If your fix is completely safe
/// and definitely does not change semantics, use [`LintContext::diagnostic_with_fix`].
/// If your fix has the potential to create parse errors, use
/// [`LintContext::diagnostic_with_dangerous_fix`].
///
/// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
#[inline]
pub fn diagnostic_with_suggestion<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
where
C: Into<RuleFix<'a>>,
F: FnOnce(RuleFixer<'_, 'a>) -> C,
{
self.diagnostic_with_fix_of_kind(diagnostic, FixKind::Suggestion, fix);
}

/// Report a lint rule violation and provide a potentially dangerous
/// automatic fix for it.
///
/// The second argument is a [closure] that takes a [`RuleFixer`] and
/// returns something that can turn into a `CompositeFix`.
///
/// Dangerous fixes should be avoided and are not applied by default with
/// `--fix`. Use this method if:
/// - Your fix is experimental and you want to test it out in the wild
/// before marking it as safe.
/// - Your fix is extremely aggressive and risky, but you want to provide
/// it as an option to users.
///
/// When possible, prefer [`LintContext::diagnostic_with_fix`]. If the only
/// risk your fix poses is minor(ish) changes to code semantics, use
/// [`LintContext::diagnostic_with_suggestion`] instead.
///
/// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
///
#[inline]
pub fn diagnostic_with_dangerous_fix<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
where
C: Into<RuleFix<'a>>,
F: FnOnce(RuleFixer<'_, 'a>) -> C,
{
if self.fix {
let fixer = RuleFixer::new(self);
let composite_fix: CompositeFix = fix(fixer).into();
let fix = composite_fix.normalize_fixes(self.source_text());
self.add_diagnostic(Message::new(diagnostic, Some(fix)));
self.diagnostic_with_fix_of_kind(diagnostic, FixKind::DangerousFix, fix);
}

pub fn diagnostic_with_fix_of_kind<C, F>(
&self,
diagnostic: OxcDiagnostic,
fix_kind: FixKind,
fix: F,
) where
C: Into<RuleFix<'a>>,
F: FnOnce(RuleFixer<'_, 'a>) -> C,
{
if let Some(accepted_fix_kind) = self.fix {
let fixer = RuleFixer::new(fix_kind, self);
let rule_fix: RuleFix<'a> = fix(fixer).into();
let diagnostic = match (rule_fix.message(), &diagnostic.help) {
(Some(message), None) => diagnostic.with_help(message.to_owned()),
_ => diagnostic,
};
if rule_fix.kind() <= accepted_fix_kind {
let fix = rule_fix.into_fix(self.source_text());
self.add_diagnostic(Message::new(diagnostic, Some(fix)));
} else {
self.diagnostic(diagnostic);
}
} else {
self.diagnostic(diagnostic);
}
Expand Down
Loading

0 comments on commit 8d98d2c

Please sign in to comment.