From ddf4ac9cc23048e2e8caa876532124ceef5fa11a Mon Sep 17 00:00:00 2001 From: Boshen Date: Sun, 7 May 2023 17:58:57 +0800 Subject: [PATCH] refactor(formatter): remove whitespace minification from formatter (#337) --- .github/codecov.yml | 2 -- Cargo.lock | 1 - crates/oxc_formatter/README.md | 2 +- crates/oxc_formatter/src/gen.rs | 8 ++------ crates/oxc_formatter/src/lib.rs | 31 ++++++++----------------------- crates/oxc_wasm/src/lib.rs | 12 ++---------- tasks/coverage/Cargo.toml | 1 - tasks/coverage/src/formatter.rs | 27 --------------------------- 8 files changed, 13 insertions(+), 71 deletions(-) diff --git a/.github/codecov.yml b/.github/codecov.yml index b3440d1dd..f2f667ca2 100644 --- a/.github/codecov.yml +++ b/.github/codecov.yml @@ -14,8 +14,6 @@ ignore: - "**/main.rs" - "**/examples" - "tasks" - - "crates/oxc_hir" - - "crates/oxc_ast_lower" - "crates/oxc_wasm" # Remove this once wasm is completed - "crates/oxc_parser/fuzz" - "crates/oxc_diagnostics" diff --git a/Cargo.lock b/Cargo.lock index e38049378..c960dd634 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1040,7 +1040,6 @@ dependencies = [ "miette", "oxc_allocator", "oxc_ast", - "oxc_ast_lower", "oxc_diagnostics", "oxc_formatter", "oxc_minifier", diff --git a/crates/oxc_formatter/README.md b/crates/oxc_formatter/README.md index 74529c118..4bf60694b 100644 --- a/crates/oxc_formatter/README.md +++ b/crates/oxc_formatter/README.md @@ -1,3 +1,3 @@ -# Formatter +# Formatter (Prettier) TBD diff --git a/crates/oxc_formatter/src/gen.rs b/crates/oxc_formatter/src/gen.rs index 3c76d564d..3f1f3997e 100644 --- a/crates/oxc_formatter/src/gen.rs +++ b/crates/oxc_formatter/src/gen.rs @@ -985,12 +985,8 @@ impl<'a> Gen for ObjectExpression<'a> { if i != 0 { p.print_comma(); } - if p.options.minify_whitespace { - p.print_space(); - } else { - p.print_newline(); - p.print_indent(); - } + p.print_newline(); + p.print_indent(); item.gen(p); } p.print_newline(); diff --git a/crates/oxc_formatter/src/lib.rs b/crates/oxc_formatter/src/lib.rs index bca3cb7a5..ce954c807 100644 --- a/crates/oxc_formatter/src/lib.rs +++ b/crates/oxc_formatter/src/lib.rs @@ -15,13 +15,12 @@ pub use crate::gen::Gen; #[derive(Debug, Clone, Copy)] pub struct FormatterOptions { - pub minify_whitespace: bool, pub indentation: u8, } impl Default for FormatterOptions { fn default() -> Self { - Self { minify_whitespace: false, indentation: 4 } + Self { indentation: 4 } } } @@ -54,14 +53,10 @@ pub enum Separator { impl Formatter { #[must_use] pub fn new(source_len: usize, options: FormatterOptions) -> Self { - // Initialize the output code buffer to reduce memory reallocation. - // Minification will reduce by at least half the original size, - // so in fact no reallocation should happen at all. - let capacity = if options.minify_whitespace { source_len / 2 } else { source_len }; Self { options, symbols: Rc::new(SymbolTable::default()), - code: Vec::with_capacity(capacity), + code: Vec::with_capacity(source_len), indentation: 0, needs_semicolon: false, prev_op_end: 0, @@ -109,16 +104,12 @@ impl Formatter { #[inline] pub fn print_space(&mut self) { - if !self.options.minify_whitespace { - self.code.push(b' '); - } + self.code.push(b' '); } #[inline] pub fn print_newline(&mut self) { - if !self.options.minify_whitespace { - self.code.push(b'\n'); - } + self.code.push(b'\n'); } #[inline] @@ -174,12 +165,8 @@ impl Formatter { } fn print_semicolon_after_statement(&mut self) { - if self.options.minify_whitespace { - self.needs_semicolon = true; - } else { - self.print_semicolon(); - self.print(b'\n'); - } + self.print_semicolon(); + self.print(b'\n'); } fn print_semicolon_if_needed(&mut self) { @@ -205,10 +192,8 @@ impl Formatter { } pub fn print_indent(&mut self) { - if !self.options.minify_whitespace { - for _ in 0..self.indentation { - self.print(b' '); - } + for _ in 0..self.indentation { + self.print(b' '); } } diff --git a/crates/oxc_wasm/src/lib.rs b/crates/oxc_wasm/src/lib.rs index e207d5b4e..3696d2f13 100644 --- a/crates/oxc_wasm/src/lib.rs +++ b/crates/oxc_wasm/src/lib.rs @@ -54,9 +54,6 @@ pub struct OxcLinterOptions; #[wasm_bindgen] #[derive(Default, Clone, Copy)] pub struct OxcFormatterOptions { - mangle: bool, - #[wasm_bindgen(js_name = minifyWhitespace)] - pub minify_whitespace: bool, pub indentation: u8, } @@ -139,13 +136,8 @@ impl Oxc { } if let Some(o) = &self.options.formatter { - let formatter_options = FormatterOptions { - minify_whitespace: o.minify_whitespace, - indentation: o.indentation, - }; - let printed = Formatter::new(source_text.len(), formatter_options) - .with_symbol_table(&semantic.symbols(), o.mangle) - .build(program); + let formatter_options = FormatterOptions { indentation: o.indentation }; + let printed = Formatter::new(source_text.len(), formatter_options).build(program); self.printed_text = printed; } diff --git a/tasks/coverage/Cargo.toml b/tasks/coverage/Cargo.toml index 784e0693d..b28909516 100644 --- a/tasks/coverage/Cargo.toml +++ b/tasks/coverage/Cargo.toml @@ -18,7 +18,6 @@ oxc_formatter = { workspace = true } oxc_diagnostics = { workspace = true } oxc_semantic = { workspace = true } oxc_minifier = { workspace = true } -oxc_ast_lower = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/tasks/coverage/src/formatter.rs b/tasks/coverage/src/formatter.rs index 30bbae281..937e9cabb 100644 --- a/tasks/coverage/src/formatter.rs +++ b/tasks/coverage/src/formatter.rs @@ -2,7 +2,6 @@ use std::path::{Path, PathBuf}; use oxc_allocator::Allocator; use oxc_ast::SourceType; -use oxc_ast_lower::AstLower; use oxc_formatter::{Formatter, FormatterOptions}; use oxc_parser::Parser; @@ -39,20 +38,8 @@ impl Case for FormatterTest262Case { let source_text = self.base.code(); let is_module = self.base.meta().flags.contains(&TestFlag::Module); let source_type = SourceType::default().with_module(is_module); - - // Test formatter let formatter_options = FormatterOptions::default(); let result = get_result(source_text, source_type, formatter_options); - - if !matches!(result, TestResult::Passed) { - self.base.set_result(result); - return; - } - - // Test whitespace minification - let formatter_options = - FormatterOptions { minify_whitespace: true, ..FormatterOptions::default() }; - let result = get_result(source_text, source_type, formatter_options); self.base.set_result(result); } } @@ -87,20 +74,8 @@ impl Case for FormatterBabelCase { fn run(&mut self) { let source_text = self.base.code(); let source_type = self.base.source_type(); - - // Test formatter let formatter_options = FormatterOptions::default(); let result = get_result(source_text, source_type, formatter_options); - - if !matches!(result, TestResult::Passed) { - self.base.set_result(result); - return; - } - - // Test whitespace minification - let formatter_options = - FormatterOptions { minify_whitespace: true, ..FormatterOptions::default() }; - let result = get_result(source_text, source_type, formatter_options); self.base.set_result(result); } } @@ -108,10 +83,8 @@ impl Case for FormatterBabelCase { fn get_result(source_text: &str, source_type: SourceType, options: FormatterOptions) -> TestResult { let allocator = Allocator::default(); let program1 = Parser::new(&allocator, source_text, source_type).parse().program; - let _ = AstLower::new(&allocator).build(&program1); // temporary Stub for testing ast_lower let source_text1 = Formatter::new(source_text.len(), options).build(&program1); let program2 = Parser::new(&allocator, &source_text1, source_type).parse().program; - let _ = AstLower::new(&allocator).build(&program2); // temporary Stub for testing ast_lower let source_text2 = Formatter::new(source_text1.len(), options).build(&program2); if source_text1 == source_text2 { TestResult::Passed