From d8b74b8caea7ca6cdcc4922ed9f51fa8121627e6 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Fri, 3 Mar 2023 21:23:08 +0800 Subject: [PATCH 01/10] test(oxc_linter): add testing for code fixer --- crates/oxc_linter/src/autofix/fixer.rs | 78 +++++++++++++++++++++++--- crates/oxc_linter/src/autofix/mod.rs | 21 ++----- crates/oxc_linter/src/context.rs | 8 +-- 3 files changed, 79 insertions(+), 28 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index 2a781c492f62a..98f6705a50889 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -4,11 +4,11 @@ use super::Fix; pub struct Fixer<'a> { source_text: &'a str, - fixes: Vec, + fixes: Vec>, } impl<'a> Fixer<'a> { - pub fn new(source_text: &'a str, fixes: Vec) -> Self { + pub fn new(source_text: &'a str, fixes: Vec>) -> Self { Self { source_text, fixes } } @@ -16,11 +16,75 @@ impl<'a> Fixer<'a> { if self.fixes.is_empty() { Cow::Borrowed(self.source_text) } else { - let mut fixed_source = String::new(); - for fix in &self.fixes { - fixed_source = fix.apply(self.source_text); - } - Cow::Owned(fixed_source) + let source_text = self.source_text; + let mut output = String::new(); + // To record the position of the last fix. + let mut last_pos = 0; + self.fixes.iter().for_each(|Fix { content, span }| { + let start = span.start; + // Current fix may conflict with the last fix, so let's skip it. + if start < last_pos { + return; + } + + let offset = last_pos.max(0) as usize; + output.push_str(&source_text[offset..start as usize]); + output.push_str(content); + last_pos = span.end; + }); + + let offset = last_pos.max(0) as usize; + output.push_str(&source_text[offset..]); + + return Cow::Owned(output); } } } + +#[cfg(test)] +mod test { + use oxc_ast::Span; + + use super::Fixer; + use crate::autofix::Fix; + + const TEST_CODE: &str = "var answer = 6 * 7"; + const INSERT_AT_END: Fix = Fix { span: Span { start: 18, end: 18 }, content: "// end" }; + const INSERT_AT_START: Fix = Fix { span: Span { start: 0, end: 0 }, content: "// start" }; + const INSERT_AT_MIDDLE: Fix = Fix { span: Span { start: 13, end: 13 }, content: "5 *" }; + + #[test] + fn insert_at_the_end() { + let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_END]); + assert_eq!(fixer.fix(), TEST_CODE.to_string() + INSERT_AT_END.content); + } + + #[test] + fn insert_at_the_beginning() { + let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_START]); + assert_eq!(fixer.fix(), INSERT_AT_START.content.to_string() + TEST_CODE); + } + + #[test] + fn insert_at_the_middle() { + let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_MIDDLE]); + assert_eq!( + fixer.fix(), + TEST_CODE.replace("6 *", &format!("{}{}", INSERT_AT_MIDDLE.content, "6 *")) + ); + } + + #[test] + fn insert_at_the_beginning_middle_end() { + let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_START, INSERT_AT_MIDDLE, INSERT_AT_END]); + assert_eq!( + fixer.fix(), + format!( + "{}{}{}", + INSERT_AT_START.content, + TEST_CODE.replace("6 *", &format!("{}{}", INSERT_AT_MIDDLE.content, "6 *")), + INSERT_AT_END.content + ) + ); + } +} diff --git a/crates/oxc_linter/src/autofix/mod.rs b/crates/oxc_linter/src/autofix/mod.rs index 372e4acd2c717..78f18bafd9e45 100644 --- a/crates/oxc_linter/src/autofix/mod.rs +++ b/crates/oxc_linter/src/autofix/mod.rs @@ -5,26 +5,13 @@ mod fixer; pub use fixer::Fixer; #[derive(Debug)] -pub struct Fix { - pub content: String, +pub struct Fix<'a> { + pub content: &'a str, pub span: Span, } -impl<'a> Fix { +impl<'a> Fix<'a> { pub const fn delete(span: Span) -> Self { - Self { content: String::new(), span } - } - - pub fn apply(&self, source_text: &'a str) -> String { - let mut output = String::new(); - - let slice = &source_text[..self.span.start as usize]; - let remainder = &source_text[self.span.end as usize..]; - - output.push_str(slice); - output.push_str(&self.content); - output.push_str(remainder); - - output + Self { content: "", span } } } diff --git a/crates/oxc_linter/src/context.rs b/crates/oxc_linter/src/context.rs index 3580afe3b34c7..aad844b489406 100644 --- a/crates/oxc_linter/src/context.rs +++ b/crates/oxc_linter/src/context.rs @@ -13,7 +13,7 @@ pub struct LintContext<'a> { diagnostics: RefCell>, - fixes: RefCell>, + fixes: RefCell>>, } impl<'a> LintContext<'a> { @@ -30,7 +30,7 @@ impl<'a> LintContext<'a> { self.source_text } - pub fn into_diagnostics(self) -> (Vec, Vec) { + pub fn into_diagnostics(self) -> (Vec>, Vec) { (self.fixes.into_inner(), self.diagnostics.into_inner()) } @@ -42,11 +42,11 @@ impl<'a> LintContext<'a> { self.diagnostics.borrow_mut().push(diagnostic.into()); } - pub fn fix(&self, fix: Fix) { + pub fn fix(&self, fix: Fix<'a>) { self.fixes.borrow_mut().push(fix); } - pub fn into_fixes(self) -> Vec { + pub fn into_fixes(self) -> Vec> { self.fixes.into_inner() } From 4fa7423375df2e1fcb8d47db30d1da4b53204a77 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 10:04:54 +0800 Subject: [PATCH 02/10] test(oxc_linter): ignore reverse range --- crates/oxc_linter/src/autofix/fixer.rs | 52 ++++++++++++++++++++++---- crates/oxc_linter/src/autofix/mod.rs | 10 ++++- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index 98f6705a50889..2588316644bb0 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -1,4 +1,6 @@ -use std::borrow::Cow; +use std::{borrow::Cow, cmp::Ordering}; + +use oxc_ast::Span; use super::Fix; @@ -8,7 +10,25 @@ pub struct Fixer<'a> { } impl<'a> Fixer<'a> { - pub fn new(source_text: &'a str, fixes: Vec>) -> Self { + pub fn new(source_text: &'a str, mut fixes: Vec>) -> Self { + fixes.sort_by( + |Fix { span: Span { start: a_start, end: a_end }, .. }, + Fix { span: Span { start: b_start, end: b_end }, .. }| { + if a_start < b_start { + Ordering::Less + } else if a_start > b_start { + Ordering::Greater + } else { + if a_end < b_end { + Ordering::Less + } else if a_end > b_end { + Ordering::Greater + } else { + Ordering::Equal + } + } + }, + ); Self { source_text, fixes } } @@ -17,11 +37,15 @@ impl<'a> Fixer<'a> { Cow::Borrowed(self.source_text) } else { let source_text = self.source_text; - let mut output = String::new(); + let mut output = String::with_capacity(source_text.len()); // To record the position of the last fix. let mut last_pos = 0; self.fixes.iter().for_each(|Fix { content, span }| { let start = span.start; + let end = span.end; + if start > end { + return; + } // Current fix may conflict with the last fix, so let's skip it. if start < last_pos { return; @@ -43,20 +67,26 @@ impl<'a> Fixer<'a> { #[cfg(test)] mod test { + use std::borrow::Cow; + use oxc_ast::Span; use super::Fixer; use crate::autofix::Fix; const TEST_CODE: &str = "var answer = 6 * 7"; - const INSERT_AT_END: Fix = Fix { span: Span { start: 18, end: 18 }, content: "// end" }; - const INSERT_AT_START: Fix = Fix { span: Span { start: 0, end: 0 }, content: "// start" }; - const INSERT_AT_MIDDLE: Fix = Fix { span: Span { start: 13, end: 13 }, content: "5 *" }; + const INSERT_AT_END: Fix = + Fix { span: Span { start: 18, end: 18 }, content: Cow::Borrowed("// end") }; + const INSERT_AT_START: Fix = + Fix { span: Span { start: 0, end: 0 }, content: Cow::Borrowed("// start") }; + const INSERT_AT_MIDDLE: Fix = + Fix { span: Span { start: 13, end: 13 }, content: Cow::Borrowed("5 *") }; + const REVERSE_RANGE: Fix = Fix { span: Span { start: 3, end: 0 }, content: Cow::Borrowed(" ") }; #[test] fn insert_at_the_end() { let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_END]); - assert_eq!(fixer.fix(), TEST_CODE.to_string() + INSERT_AT_END.content); + assert_eq!(fixer.fix(), TEST_CODE.to_string() + INSERT_AT_END.content.as_ref()); } #[test] @@ -76,7 +106,7 @@ mod test { #[test] fn insert_at_the_beginning_middle_end() { - let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_START, INSERT_AT_MIDDLE, INSERT_AT_END]); + let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_MIDDLE, INSERT_AT_START, INSERT_AT_END]); assert_eq!( fixer.fix(), format!( @@ -87,4 +117,10 @@ mod test { ) ); } + + #[test] + fn ignore_reverse_range() { + let fixer = Fixer::new(TEST_CODE, vec![REVERSE_RANGE]); + assert_eq!(fixer.fix(), TEST_CODE) + } } diff --git a/crates/oxc_linter/src/autofix/mod.rs b/crates/oxc_linter/src/autofix/mod.rs index 78f18bafd9e45..cda1c56610e16 100644 --- a/crates/oxc_linter/src/autofix/mod.rs +++ b/crates/oxc_linter/src/autofix/mod.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use oxc_ast::Span; mod fixer; @@ -6,12 +8,16 @@ pub use fixer::Fixer; #[derive(Debug)] pub struct Fix<'a> { - pub content: &'a str, + pub content: Cow<'a, str>, pub span: Span, } impl<'a> Fix<'a> { pub const fn delete(span: Span) -> Self { - Self { content: "", span } + Self { content: Cow::Borrowed(""), span } + } + + pub fn new>>(content: T, span: Span) -> Self { + Self { content: content.into(), span } } } From c5a67a4823935b2bddddeebae0f90e186a5625b4 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 10:20:05 +0800 Subject: [PATCH 03/10] test(oxc_linter): add replacement testing --- crates/oxc_linter/src/autofix/fixer.rs | 47 +++++++++++++++++++++----- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index 2588316644bb0..05d1215674ba4 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -74,30 +74,37 @@ mod test { use super::Fixer; use crate::autofix::Fix; - const TEST_CODE: &str = "var answer = 6 * 7"; + const TEST_CODE: &str = "var answer = 6 * 7;"; const INSERT_AT_END: Fix = - Fix { span: Span { start: 18, end: 18 }, content: Cow::Borrowed("// end") }; + Fix { span: Span { start: 19, end: 19 }, content: Cow::Borrowed("// end") }; const INSERT_AT_START: Fix = Fix { span: Span { start: 0, end: 0 }, content: Cow::Borrowed("// start") }; const INSERT_AT_MIDDLE: Fix = Fix { span: Span { start: 13, end: 13 }, content: Cow::Borrowed("5 *") }; + const REPLACE_ID: Fix = Fix { span: Span { start: 4, end: 10 }, content: Cow::Borrowed("foo") }; + const REPLACE_VAR: Fix = Fix { span: Span { start: 0, end: 3 }, content: Cow::Borrowed("let") }; + const REPLACE_NUM: Fix = Fix { span: Span { start: 13, end: 14 }, content: Cow::Borrowed("5") }; const REVERSE_RANGE: Fix = Fix { span: Span { start: 3, end: 0 }, content: Cow::Borrowed(" ") }; + fn create_fixer<'a>(fixes: Vec>) -> Fixer { + Fixer::new(TEST_CODE, fixes) + } + #[test] fn insert_at_the_end() { - let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_END]); + let fixer = create_fixer(vec![INSERT_AT_END]); assert_eq!(fixer.fix(), TEST_CODE.to_string() + INSERT_AT_END.content.as_ref()); } #[test] fn insert_at_the_beginning() { - let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_START]); + let fixer = create_fixer(vec![INSERT_AT_START]); assert_eq!(fixer.fix(), INSERT_AT_START.content.to_string() + TEST_CODE); } #[test] fn insert_at_the_middle() { - let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_MIDDLE]); + let fixer = create_fixer(vec![INSERT_AT_MIDDLE]); assert_eq!( fixer.fix(), TEST_CODE.replace("6 *", &format!("{}{}", INSERT_AT_MIDDLE.content, "6 *")) @@ -106,7 +113,7 @@ mod test { #[test] fn insert_at_the_beginning_middle_end() { - let fixer = Fixer::new(TEST_CODE, vec![INSERT_AT_MIDDLE, INSERT_AT_START, INSERT_AT_END]); + let fixer = create_fixer(vec![INSERT_AT_MIDDLE, INSERT_AT_START, INSERT_AT_END]); assert_eq!( fixer.fix(), format!( @@ -120,7 +127,31 @@ mod test { #[test] fn ignore_reverse_range() { - let fixer = Fixer::new(TEST_CODE, vec![REVERSE_RANGE]); - assert_eq!(fixer.fix(), TEST_CODE) + let fixer = create_fixer(vec![REVERSE_RANGE]); + assert_eq!(fixer.fix(), TEST_CODE); + } + + #[test] + fn replace_at_the_beginning() { + let fixer = create_fixer(vec![REPLACE_VAR]); + assert_eq!(fixer.fix(), TEST_CODE.replace("var", "let")); + } + + #[test] + fn replace_at_the_middle() { + let fixer = create_fixer(vec![REPLACE_ID]); + assert_eq!(fixer.fix(), TEST_CODE.replace("answer", "foo")); + } + + #[test] + fn replace_at_the_end() { + let fixer = create_fixer(vec![REPLACE_NUM]); + assert_eq!(fixer.fix(), TEST_CODE.replace("6", "5")); + } + + #[test] + fn replace_at_the_beginning_middle_end() { + let fixer = create_fixer(vec![REPLACE_ID, REPLACE_VAR, REPLACE_NUM]); + assert_eq!(fixer.fix(), "let foo = 5 * 7;"); } } From 90a33f046bb08a401f2267f22d3f4712b9eb9178 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 10:26:41 +0800 Subject: [PATCH 04/10] test(oxc_linter): add removal testing --- crates/oxc_linter/src/autofix/fixer.rs | 29 ++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index 05d1215674ba4..de2b515d41a0a 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -84,6 +84,9 @@ mod test { const REPLACE_ID: Fix = Fix { span: Span { start: 4, end: 10 }, content: Cow::Borrowed("foo") }; const REPLACE_VAR: Fix = Fix { span: Span { start: 0, end: 3 }, content: Cow::Borrowed("let") }; const REPLACE_NUM: Fix = Fix { span: Span { start: 13, end: 14 }, content: Cow::Borrowed("5") }; + const REMOVE_START: Fix = Fix::delete(Span { start: 0, end: 4 }); + const REMOVE_MIDDLE: Fix = Fix::delete(Span { start: 5, end: 10 }); + const REMOVE_END: Fix = Fix::delete(Span { start: 14, end: 18 }); const REVERSE_RANGE: Fix = Fix { span: Span { start: 3, end: 0 }, content: Cow::Borrowed(" ") }; fn create_fixer<'a>(fixes: Vec>) -> Fixer { @@ -97,7 +100,7 @@ mod test { } #[test] - fn insert_at_the_beginning() { + fn insert_at_the_start() { let fixer = create_fixer(vec![INSERT_AT_START]); assert_eq!(fixer.fix(), INSERT_AT_START.content.to_string() + TEST_CODE); } @@ -112,7 +115,7 @@ mod test { } #[test] - fn insert_at_the_beginning_middle_end() { + fn insert_at_the_start_middle_end() { let fixer = create_fixer(vec![INSERT_AT_MIDDLE, INSERT_AT_START, INSERT_AT_END]); assert_eq!( fixer.fix(), @@ -132,7 +135,7 @@ mod test { } #[test] - fn replace_at_the_beginning() { + fn replace_at_the_start() { let fixer = create_fixer(vec![REPLACE_VAR]); assert_eq!(fixer.fix(), TEST_CODE.replace("var", "let")); } @@ -150,8 +153,26 @@ mod test { } #[test] - fn replace_at_the_beginning_middle_end() { + fn replace_at_the_start_middle_end() { let fixer = create_fixer(vec![REPLACE_ID, REPLACE_VAR, REPLACE_NUM]); assert_eq!(fixer.fix(), "let foo = 5 * 7;"); } + + #[test] + fn remove_at_the_start() { + let fixer = create_fixer(vec![REMOVE_START]); + assert_eq!(fixer.fix(), TEST_CODE.replace("var ", "")); + } + + #[test] + fn remove_at_the_middle() { + let fixer = create_fixer(vec![REMOVE_MIDDLE]); + assert_eq!(fixer.fix(), TEST_CODE.replace("answer", "a")); + } + + #[test] + fn remove_at_the_end() { + let fixer = create_fixer(vec![REMOVE_END]); + assert_eq!(fixer.fix(), TEST_CODE.replace(" * 7", "")); + } } From 3a1f8d62878dea5f7fb89da55ae04038da115ade Mon Sep 17 00:00:00 2001 From: Shannon Rothe Date: Sat, 4 Mar 2023 13:31:56 +1100 Subject: [PATCH 05/10] feat(linter): wire up `--fix` CLI flag (#107) --- crates/oxc_cli/src/command.rs | 19 +++++++++++++++++++ crates/oxc_cli/src/lib.rs | 8 +++++--- crates/oxc_cli/src/options.rs | 2 ++ crates/oxc_linter/src/context.rs | 10 +++++++++- crates/oxc_linter/src/lib.rs | 9 +++++++-- crates/oxc_linter/src/tester.rs | 2 +- 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/crates/oxc_cli/src/command.rs b/crates/oxc_cli/src/command.rs index a5e93813b2bf5..90cbc4add0253 100644 --- a/crates/oxc_cli/src/command.rs +++ b/crates/oxc_cli/src/command.rs @@ -30,6 +30,13 @@ impl Command { .alias("check") .about("Lint this repository.") .arg_required_else_help(true) + .arg( + Arg::new("fix") + .long("fix") + .required(false) + .action(ArgAction::SetTrue) + .help("This option allows you to enable oxc to fix as many issues as possible. If enabled, only unfixed issues are reported in the output") + ) .arg( Arg::new("quiet") .long("quiet") @@ -134,6 +141,18 @@ mod test { assert!(!matches.get_flag("quiet")); } + #[test] + fn test_fix_true() { + let matches = get_lint_matches("oxc lint foo.js --fix"); + assert!(matches.get_flag("fix")); + } + + #[test] + fn test_fix_false() { + let matches = get_lint_matches("oxc lint foo.js"); + assert!(!matches.get_flag("fix")); + } + #[test] fn test_max_warnings_none() { let arg = "oxc lint foo.js"; diff --git a/crates/oxc_cli/src/lib.rs b/crates/oxc_cli/src/lib.rs index b279613f7950c..66910aad49f03 100644 --- a/crates/oxc_cli/src/lib.rs +++ b/crates/oxc_cli/src/lib.rs @@ -63,10 +63,12 @@ impl Cli { }) .collect::>(); + let fix = self.cli_options.fix; + let (_, (warnings, diagnostics)): (_, (usize, usize)) = rayon::join( move || { paths.par_iter().for_each(|path| { - let diagnostics = Self::lint_path(path); + let diagnostics = Self::lint_path(path, fix); for d in diagnostics { sender.send(d).unwrap(); @@ -118,7 +120,7 @@ impl Cli { } } - fn lint_path(path: &Path) -> Vec { + fn lint_path(path: &Path, fix: bool) -> Vec { let source_text = fs::read_to_string(path).expect("{name} not found"); let allocator = Allocator::default(); let source_type = SourceType::from_path(path).expect("incorrect {path:?}"); @@ -127,7 +129,7 @@ impl Cli { let result = if ret.errors.is_empty() { let program = allocator.alloc(ret.program); let semantic = SemanticBuilder::new().build(program, ret.trivias); - Linter::new().run(&Rc::new(semantic), &source_text) + Linter::new().run(&Rc::new(semantic), &source_text, fix) } else { LintRunResult { fixed_source: source_text.clone().into(), diagnostics: ret.errors } }; diff --git a/crates/oxc_cli/src/options.rs b/crates/oxc_cli/src/options.rs index 35b2ce149843a..6aafecce27172 100644 --- a/crates/oxc_cli/src/options.rs +++ b/crates/oxc_cli/src/options.rs @@ -5,6 +5,7 @@ use glob::Pattern; pub struct CliOptions { pub quiet: bool, + pub fix: bool, pub max_warnings: Option, pub paths: Vec, pub ignore_path: String, @@ -38,6 +39,7 @@ impl<'a> TryFrom<&'a ArgMatches> for CliOptions { Ok(Self { quiet: matches.get_flag("quiet"), + fix: matches.get_flag("fix"), max_warnings: matches.get_one("max-warnings").copied(), paths, ignore_path, diff --git a/crates/oxc_linter/src/context.rs b/crates/oxc_linter/src/context.rs index aad844b489406..f1ca81f3f6156 100644 --- a/crates/oxc_linter/src/context.rs +++ b/crates/oxc_linter/src/context.rs @@ -13,15 +13,20 @@ pub struct LintContext<'a> { diagnostics: RefCell>, + /// Whether or not to apply code fixes during linting. + fix: bool, + + /// Collection of applicable fixes based on reported issues. fixes: RefCell>>, } impl<'a> LintContext<'a> { - pub fn new(source_text: &'a str, semantic: Rc>) -> Self { + pub fn new(source_text: &'a str, semantic: Rc>, fix: bool) -> Self { Self { source_text, semantic, diagnostics: RefCell::new(vec![]), + fix, fixes: RefCell::new(vec![]), } } @@ -43,6 +48,9 @@ impl<'a> LintContext<'a> { } pub fn fix(&self, fix: Fix<'a>) { + if !self.fix { + return; + } self.fixes.borrow_mut().push(fix); } diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 4293e87758335..43e54220ef9df 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -65,8 +65,13 @@ impl Linter { } #[must_use] - pub fn run<'a>(&self, semantic: &Rc>, source_text: &'a str) -> LintRunResult<'a> { - let ctx = LintContext::new(source_text, semantic.clone()); + pub fn run<'a>( + &self, + semantic: &Rc>, + source_text: &'a str, + fix: bool, + ) -> LintRunResult<'a> { + let ctx = LintContext::new(source_text, semantic.clone(), fix); for node in semantic.nodes().iter() { for rule in &self.rules { diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index 1f74b1094e54a..7d7b654b9c81b 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -66,7 +66,7 @@ impl Tester { let semantic = std::rc::Rc::new(semantic); let rule = RULES.iter().find(|rule| rule.name() == self.rule_name).unwrap(); let rule = rule.read_json(config); - let result = Linter::from_rules(vec![rule]).run(&semantic, source_text); + let result = Linter::from_rules(vec![rule]).run(&semantic, source_text, false); if result.diagnostics.is_empty() { return true; } From 5de4bf960cfa2cd0773f5259518f017313208192 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 10:42:44 +0800 Subject: [PATCH 06/10] refactor(oxc_linter): derice PartialOrd and Ord for span --- crates/oxc_ast/src/span.rs | 2 +- crates/oxc_linter/src/autofix/fixer.rs | 23 ++--------------------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/crates/oxc_ast/src/span.rs b/crates/oxc_ast/src/span.rs index cffac198de553..d60eda24633ff 100644 --- a/crates/oxc_ast/src/span.rs +++ b/crates/oxc_ast/src/span.rs @@ -6,7 +6,7 @@ use serde::Serialize; #[allow(clippy::wildcard_imports)] use crate::ast::*; -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, PartialOrd, Ord)] pub struct Span { pub start: u32, pub end: u32, diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index de2b515d41a0a..865daefe63648 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -1,6 +1,4 @@ -use std::{borrow::Cow, cmp::Ordering}; - -use oxc_ast::Span; +use std::borrow::Cow; use super::Fix; @@ -11,24 +9,7 @@ pub struct Fixer<'a> { impl<'a> Fixer<'a> { pub fn new(source_text: &'a str, mut fixes: Vec>) -> Self { - fixes.sort_by( - |Fix { span: Span { start: a_start, end: a_end }, .. }, - Fix { span: Span { start: b_start, end: b_end }, .. }| { - if a_start < b_start { - Ordering::Less - } else if a_start > b_start { - Ordering::Greater - } else { - if a_end < b_end { - Ordering::Less - } else if a_end > b_end { - Ordering::Greater - } else { - Ordering::Equal - } - } - }, - ); + fixes.sort_by_key(|f| f.span); Self { source_text, fixes } } From 0fdaf532b7cd92a3d753a9da90e2d21840f02ec3 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 10:45:14 +0800 Subject: [PATCH 07/10] refactor(oxc_linter): delete unused method --- crates/oxc_linter/src/context.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/oxc_linter/src/context.rs b/crates/oxc_linter/src/context.rs index f1ca81f3f6156..82dde3661671a 100644 --- a/crates/oxc_linter/src/context.rs +++ b/crates/oxc_linter/src/context.rs @@ -54,10 +54,6 @@ impl<'a> LintContext<'a> { self.fixes.borrow_mut().push(fix); } - pub fn into_fixes(self) -> Vec> { - self.fixes.into_inner() - } - pub fn parent_kind(&self, node: &AstNode<'a>) -> AstKind<'a> { self.semantic().nodes().parent_kind(node) } From 3f8a60c153de50efb015ad6654382eba19ae3f28 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 11:27:54 +0800 Subject: [PATCH 08/10] test(oxc_linter): add combination testing --- crates/oxc_linter/src/autofix/fixer.rs | 40 +++++++++++++++++++++----- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index 865daefe63648..a04a1341a4ffd 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -20,7 +20,7 @@ impl<'a> Fixer<'a> { let source_text = self.source_text; let mut output = String::with_capacity(source_text.len()); // To record the position of the last fix. - let mut last_pos = 0; + let mut last_pos: i64 = -1; self.fixes.iter().for_each(|Fix { content, span }| { let start = span.start; let end = span.end; @@ -28,17 +28,18 @@ impl<'a> Fixer<'a> { return; } // Current fix may conflict with the last fix, so let's skip it. - if start < last_pos { + if i64::from(start) <= last_pos { return; } - let offset = last_pos.max(0) as usize; + // This is safe to unwrap because the minimal number is 0. + let offset = usize::try_from(last_pos.max(0)).ok().unwrap(); output.push_str(&source_text[offset..start as usize]); output.push_str(content); - last_pos = span.end; + last_pos = i64::from(end); }); - let offset = last_pos.max(0) as usize; + let offset = usize::try_from(last_pos.max(0)).ok().unwrap(); output.push_str(&source_text[offset..]); return Cow::Owned(output); @@ -70,7 +71,7 @@ mod test { const REMOVE_END: Fix = Fix::delete(Span { start: 14, end: 18 }); const REVERSE_RANGE: Fix = Fix { span: Span { start: 3, end: 0 }, content: Cow::Borrowed(" ") }; - fn create_fixer<'a>(fixes: Vec>) -> Fixer { + fn create_fixer(fixes: Vec) -> Fixer { Fixer::new(TEST_CODE, fixes) } @@ -130,7 +131,7 @@ mod test { #[test] fn replace_at_the_end() { let fixer = create_fixer(vec![REPLACE_NUM]); - assert_eq!(fixer.fix(), TEST_CODE.replace("6", "5")); + assert_eq!(fixer.fix(), TEST_CODE.replace('6', "5")); } #[test] @@ -156,4 +157,29 @@ mod test { let fixer = create_fixer(vec![REMOVE_END]); assert_eq!(fixer.fix(), TEST_CODE.replace(" * 7", "")); } + + #[test] + fn replace_at_start_remove_at_middle_insert_at_end() { + let fixer = create_fixer(vec![INSERT_AT_END, REMOVE_END, REPLACE_VAR]); + assert_eq!(fixer.fix(), "let answer = 6;// end"); + } + + #[test] + fn apply_one_fix_when_spans_overlap() { + let fixer = create_fixer(vec![REMOVE_MIDDLE, REPLACE_ID]); + assert_eq!(fixer.fix(), TEST_CODE.replace("answer", "foo")); + } + + #[test] + fn apply_one_fix_when_the_start_the_same_as_the_previous_end() { + let fixer = create_fixer(vec![REMOVE_START, REPLACE_ID]); + assert_eq!(fixer.fix(), TEST_CODE.replace("var ", "")); + } + + #[test] + fn apply_same_fix_when_span_overlap_regardless_of_order() { + let fixer1 = create_fixer(vec![REMOVE_MIDDLE, REPLACE_ID]); + let fixer2 = create_fixer(vec![REPLACE_ID, REMOVE_MIDDLE]); + assert_eq!(fixer1.fix(), fixer2.fix()); + } } From c92e5054ef2332addd31f5fc7f037dbaca534ad4 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 13:09:16 +0800 Subject: [PATCH 09/10] test(oxc_linter): add the remaining testing and ignore them --- crates/oxc_linter/src/autofix/fixer.rs | 121 +++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index a04a1341a4ffd..f6ce127ed5839 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -176,10 +176,131 @@ mod test { assert_eq!(fixer.fix(), TEST_CODE.replace("var ", "")); } + #[ignore] + #[test] + fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix() {} + #[test] fn apply_same_fix_when_span_overlap_regardless_of_order() { let fixer1 = create_fixer(vec![REMOVE_MIDDLE, REPLACE_ID]); let fixer2 = create_fixer(vec![REPLACE_ID, REMOVE_MIDDLE]); assert_eq!(fixer1.fix(), fixer2.fix()); } + + #[ignore] + #[test] + fn should_not_apply_fix_with_one_no_fix() {} + + #[ignore] + #[test] + fn sort_no_fix_messages_correctly() {} + + #[ignore] + #[test] + fn insert_bom_at_0() {} + + #[ignore] + #[test] + fn insert_bom_with_text_at_0() {} + + #[ignore] + #[test] + fn remove_bom_with_negative_range() {} + + #[ignore] + #[test] + fn replace_bom_with_negative_range_and_foobar() {} + + // With BOM + #[ignore] + #[test] + fn insert_at_the_end_with_bom() {} + + #[ignore] + #[test] + fn insert_at_the_start_with_bom() {} + + #[ignore] + #[test] + fn insert_at_the_middle_with_bom() {} + + #[ignore] + #[test] + fn insert_at_the_start_middle_end_with_bom() {} + + #[ignore] + #[test] + fn ignore_reverse_range_with_bom() {} + + #[ignore] + #[test] + fn replace_at_the_end_with_bom() {} + + #[ignore] + #[test] + fn replace_at_the_start_with_bom() {} + + #[ignore] + #[test] + fn replace_at_the_middle_with_bom() {} + + #[ignore] + #[test] + fn replace_at_the_start_middle_end_with_bom() {} + + #[ignore] + #[test] + fn remove_at_the_end_with_bom() {} + + #[ignore] + #[test] + fn remove_at_the_start_with_bom() {} + + #[ignore] + #[test] + fn remove_at_the_middle_with_bom() {} + + #[ignore] + #[test] + fn remove_at_the_start_middle_end_with_bom() {} + + #[ignore] + #[test] + fn replace_at_start_remove_at_middle_insert_at_end_with_bom() {} + + #[ignore] + #[test] + fn apply_one_fix_when_spans_overlap_with_bom() {} + + #[ignore] + #[test] + fn apply_one_fix_when_the_start_the_same_as_the_previous_end_with_bom() {} + + #[ignore] + #[test] + fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix_with_bom() {} + + #[ignore] + #[test] + fn apply_same_fix_when_span_overlap_regardless_of_order_with_bom() {} + + #[ignore] + #[test] + fn should_not_apply_fix_with_one_no_fix_with_bom() {} + + #[ignore] + #[test] + fn insert_bom_at_0_with_bom() {} + + #[ignore] + #[test] + fn insert_bom_with_text_at_0_with_bom() {} + + #[ignore] + #[test] + fn remove_bom_with_negative_range_with_bom() {} + + #[ignore] + #[test] + fn replace_bom_with_negative_range_and_foobar_with_bom() {} } From fa7074a13b711ccd3405abd95ab97253f7713901 Mon Sep 17 00:00:00 2001 From: Xuan <97ssps30212@gmail.com> Date: Sat, 4 Mar 2023 13:12:57 +0800 Subject: [PATCH 10/10] chore(oxc_linter): make linter happy --- crates/oxc_linter/src/autofix/fixer.rs | 120 ++++++++++++++++++------- 1 file changed, 90 insertions(+), 30 deletions(-) diff --git a/crates/oxc_linter/src/autofix/fixer.rs b/crates/oxc_linter/src/autofix/fixer.rs index f6ce127ed5839..043fba7b4c59f 100644 --- a/crates/oxc_linter/src/autofix/fixer.rs +++ b/crates/oxc_linter/src/autofix/fixer.rs @@ -178,7 +178,9 @@ mod test { #[ignore] #[test] - fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix() {} + fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix() { + let _fixer = create_fixer(vec![]); + } #[test] fn apply_same_fix_when_span_overlap_regardless_of_order() { @@ -189,118 +191,176 @@ mod test { #[ignore] #[test] - fn should_not_apply_fix_with_one_no_fix() {} + fn should_not_apply_fix_with_one_no_fix() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn sort_no_fix_messages_correctly() {} + fn sort_no_fix_messages_correctly() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_bom_at_0() {} + fn insert_bom_at_0() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_bom_with_text_at_0() {} + fn insert_bom_with_text_at_0() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_bom_with_negative_range() {} + fn remove_bom_with_negative_range() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_bom_with_negative_range_and_foobar() {} + fn replace_bom_with_negative_range_and_foobar() { + let _fixer = create_fixer(vec![]); + } // With BOM #[ignore] #[test] - fn insert_at_the_end_with_bom() {} + fn insert_at_the_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_at_the_start_with_bom() {} + fn insert_at_the_start_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_at_the_middle_with_bom() {} + fn insert_at_the_middle_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_at_the_start_middle_end_with_bom() {} + fn insert_at_the_start_middle_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn ignore_reverse_range_with_bom() {} + fn ignore_reverse_range_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_at_the_end_with_bom() {} + fn replace_at_the_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_at_the_start_with_bom() {} + fn replace_at_the_start_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_at_the_middle_with_bom() {} + fn replace_at_the_middle_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_at_the_start_middle_end_with_bom() {} + fn replace_at_the_start_middle_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_at_the_end_with_bom() {} + fn remove_at_the_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_at_the_start_with_bom() {} + fn remove_at_the_start_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_at_the_middle_with_bom() {} + fn remove_at_the_middle_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_at_the_start_middle_end_with_bom() {} + fn remove_at_the_start_middle_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_at_start_remove_at_middle_insert_at_end_with_bom() {} + fn replace_at_start_remove_at_middle_insert_at_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn apply_one_fix_when_spans_overlap_with_bom() {} + fn apply_one_fix_when_spans_overlap_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn apply_one_fix_when_the_start_the_same_as_the_previous_end_with_bom() {} + fn apply_one_fix_when_the_start_the_same_as_the_previous_end_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix_with_bom() {} + fn apply_one_fix_when_range_overlap_and_one_message_has_no_fix_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn apply_same_fix_when_span_overlap_regardless_of_order_with_bom() {} + fn apply_same_fix_when_span_overlap_regardless_of_order_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn should_not_apply_fix_with_one_no_fix_with_bom() {} + fn should_not_apply_fix_with_one_no_fix_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_bom_at_0_with_bom() {} + fn insert_bom_at_0_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn insert_bom_with_text_at_0_with_bom() {} + fn insert_bom_with_text_at_0_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn remove_bom_with_negative_range_with_bom() {} + fn remove_bom_with_negative_range_with_bom() { + let _fixer = create_fixer(vec![]); + } #[ignore] #[test] - fn replace_bom_with_negative_range_and_foobar_with_bom() {} + fn replace_bom_with_negative_range_and_foobar_with_bom() { + let _fixer = create_fixer(vec![]); + } }