From ec0035c1102584215e59a6970a3aa56138956f0d Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 4 Apr 2025 14:56:28 +0000 Subject: [PATCH] perf(formatter): remove `write!` macro where unnecessary (#10230) Replace usages of `write!` macro with either `fmt.write_str(...)` for static strings or `Display::fmt` / `Debug::fmt` for other values, where it's not being used to concatenate multiple values. `Formatter::write_str` is more performant, as it avoids various checks. `Display::fmt` / `Debug::fmt` may also perform a little better in some cases, and will be equivalent in others. But in all cases it should be better for compile times, due to avoiding macro expansion and trait resolution. --- .../src/formatter/comments/mod.rs | 16 ++-- .../src/formatter/diagnostics.rs | 12 +-- .../src/formatter/format_element/mod.rs | 6 +- .../oxc_formatter/src/formatter/token_text.rs | 17 ++-- crates/oxc_formatter/src/options.rs | 91 ++++++++++--------- 5 files changed, 77 insertions(+), 65 deletions(-) diff --git a/crates/oxc_formatter/src/formatter/comments/mod.rs b/crates/oxc_formatter/src/formatter/comments/mod.rs index e9421d12eeec2..1be9904a43219 100644 --- a/crates/oxc_formatter/src/formatter/comments/mod.rs +++ b/crates/oxc_formatter/src/formatter/comments/mod.rs @@ -79,7 +79,11 @@ mod map; #[cfg(debug_assertions)] use std::cell::{Cell, RefCell}; -use std::{marker::PhantomData, rc::Rc}; +use std::{ + fmt::{self, Debug}, + marker::PhantomData, + rc::Rc, +}; use oxc_ast::Comment; use oxc_span::Span; @@ -1040,9 +1044,9 @@ struct CommentsData { checked_suppressions: RefCell>, } -impl std::fmt::Debug for CommentsData { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", self.comments) +impl Debug for CommentsData { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Debug::fmt(&self.comments, f) // let mut comments = Vec::new(); // if let Some(root) = &self.root { @@ -1085,8 +1089,8 @@ impl DebugComment<'_> { } } -impl std::fmt::Debug for DebugComment<'_> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl Debug for DebugComment<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { DebugComment::Leading { node, comment } => { f.debug_struct("Leading").field("node", node).field("comment", comment).finish() diff --git a/crates/oxc_formatter/src/formatter/diagnostics.rs b/crates/oxc_formatter/src/formatter/diagnostics.rs index 421323a1847ff..67693d927ef4b 100644 --- a/crates/oxc_formatter/src/formatter/diagnostics.rs +++ b/crates/oxc_formatter/src/formatter/diagnostics.rs @@ -41,12 +41,9 @@ impl std::fmt::Display for FormatError { fmt, "Invalid document: {error}\n\n This is an internal Biome error. Please report if necessary." ), - FormatError::PoorLayout => { - std::write!( - fmt, - "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Biome error. Please report if necessary." - ) - } + FormatError::PoorLayout => fmt.write_str( + "Poor layout: The formatter wasn't able to pick a good layout for your document. This is an internal Biome error. Please report if necessary.", + ), } } } @@ -120,8 +117,7 @@ impl From<&PrintError> for FormatError { // } // FormatError::InvalidDocument(error) => std::write!(fmt, "Invalid document: {error}"), // FormatError::PoorLayout => { -// std::write!( -// fmt, +// fmt.write_str( // "Poor layout: The formatter wasn't able to pick a good layout for your document." // ) // } diff --git a/crates/oxc_formatter/src/formatter/format_element/mod.rs b/crates/oxc_formatter/src/formatter/format_element/mod.rs index 581c92c1aa2bf..63be78c84e282 100644 --- a/crates/oxc_formatter/src/formatter/format_element/mod.rs +++ b/crates/oxc_formatter/src/formatter/format_element/mod.rs @@ -68,9 +68,9 @@ pub enum FormatElement { impl std::fmt::Debug for FormatElement { fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { match self { - FormatElement::Space | FormatElement::HardSpace => write!(fmt, "Space"), + FormatElement::Space | FormatElement::HardSpace => fmt.write_str("Space"), FormatElement::Line(mode) => fmt.debug_tuple("Line").field(mode).finish(), - FormatElement::ExpandParent => write!(fmt, "ExpandParent"), + FormatElement::ExpandParent => fmt.write_str("ExpandParent"), FormatElement::StaticText { text } => { fmt.debug_tuple("StaticText").field(text).finish() } @@ -80,7 +80,7 @@ impl std::fmt::Debug for FormatElement { FormatElement::LocatedTokenText { slice, .. } => { fmt.debug_tuple("LocatedTokenText").field(slice).finish() } - FormatElement::LineSuffixBoundary => write!(fmt, "LineSuffixBoundary"), + FormatElement::LineSuffixBoundary => fmt.write_str("LineSuffixBoundary"), FormatElement::BestFitting(best_fitting) => { fmt.debug_tuple("BestFitting").field(&best_fitting).finish() } diff --git a/crates/oxc_formatter/src/formatter/token_text.rs b/crates/oxc_formatter/src/formatter/token_text.rs index f9f384de022c6..32c58a5b84336 100644 --- a/crates/oxc_formatter/src/formatter/token_text.rs +++ b/crates/oxc_formatter/src/formatter/token_text.rs @@ -1,4 +1,7 @@ -use std::{fmt, ops::Deref}; +use std::{ + fmt::{self, Debug, Display}, + ops::Deref, +}; use super::TextRange; @@ -30,14 +33,14 @@ impl Deref for TokenText { } } -impl fmt::Display for TokenText { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.text()) +impl Display for TokenText { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Display::fmt(self.text(), f) } } -impl fmt::Debug for TokenText { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", self.text()) +impl Debug for TokenText { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + Debug::fmt(self.text(), f) } } diff --git a/crates/oxc_formatter/src/options.rs b/crates/oxc_formatter/src/options.rs index 1aafced708b4b..ad86b92cc54db 100644 --- a/crates/oxc_formatter/src/options.rs +++ b/crates/oxc_formatter/src/options.rs @@ -140,10 +140,11 @@ impl FromStr for IndentStyle { impl fmt::Display for IndentStyle { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - IndentStyle::Tab => std::write!(f, "Tab"), - IndentStyle::Space => std::write!(f, "Space"), - } + let s = match self { + IndentStyle::Tab => "Tab", + IndentStyle::Space => "Space", + }; + f.write_str(s) } } @@ -201,11 +202,12 @@ impl FromStr for LineEnding { impl fmt::Display for LineEnding { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - LineEnding::Lf => std::write!(f, "LF"), - LineEnding::Crlf => std::write!(f, "CRLF"), - LineEnding::Cr => std::write!(f, "CR"), - } + let s = match self { + LineEnding::Lf => "LF", + LineEnding::Crlf => "CRLF", + LineEnding::Cr => "CR", + }; + f.write_str(s) } } @@ -456,10 +458,11 @@ impl FromStr for QuoteStyle { impl fmt::Display for QuoteStyle { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - QuoteStyle::Double => std::write!(f, "Double Quotes"), - QuoteStyle::Single => std::write!(f, "Single Quotes"), - } + let s = match self { + QuoteStyle::Double => "Double Quotes", + QuoteStyle::Single => "Single Quotes", + }; + f.write_str(s) } } @@ -508,10 +511,11 @@ impl FromStr for QuoteProperties { impl fmt::Display for QuoteProperties { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - QuoteProperties::AsNeeded => std::write!(f, "As needed"), - QuoteProperties::Preserve => std::write!(f, "Preserve"), - } + let s = match self { + QuoteProperties::AsNeeded => "As needed", + QuoteProperties::Preserve => "Preserve", + }; + f.write_str(s) } } @@ -548,10 +552,11 @@ impl FromStr for Semicolons { impl fmt::Display for Semicolons { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Semicolons::AsNeeded => std::write!(f, "As needed"), - Semicolons::Always => std::write!(f, "Always"), - } + let s = match self { + Semicolons::AsNeeded => "As needed", + Semicolons::Always => "Always", + }; + f.write_str(s) } } @@ -588,10 +593,11 @@ impl FromStr for ArrowParentheses { impl fmt::Display for ArrowParentheses { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ArrowParentheses::AsNeeded => std::write!(f, "As needed"), - ArrowParentheses::Always => std::write!(f, "Always"), - } + let s = match self { + ArrowParentheses::AsNeeded => "As needed", + ArrowParentheses::Always => "Always", + }; + f.write_str(s) } } @@ -697,11 +703,12 @@ impl FromStr for TrailingCommas { impl fmt::Display for TrailingCommas { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - TrailingCommas::Es5 => std::write!(f, "ES5"), - TrailingCommas::All => std::write!(f, "All"), - TrailingCommas::None => std::write!(f, "None"), - } + let s = match self { + TrailingCommas::Es5 => "ES5", + TrailingCommas::All => "All", + TrailingCommas::None => "None", + }; + f.write_str(s) } } @@ -714,10 +721,11 @@ pub enum AttributePosition { impl fmt::Display for AttributePosition { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - AttributePosition::Auto => std::write!(f, "Auto"), - AttributePosition::Multiline => std::write!(f, "Multiline"), - } + let s = match self { + AttributePosition::Auto => "Auto", + AttributePosition::Multiline => "Multiline", + }; + f.write_str(s) } } @@ -759,7 +767,7 @@ impl From for BracketSpacing { impl fmt::Display for BracketSpacing { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::write!(f, "{:?}", self.value()) + fmt::Display::fmt(&self.value(), f) } } @@ -797,7 +805,7 @@ impl From for BracketSameLine { impl fmt::Display for BracketSameLine { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - std::write!(f, "{}", self.value()) + fmt::Display::fmt(&self.value(), f) } } @@ -841,10 +849,11 @@ impl FromStr for Expand { impl fmt::Display for Expand { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Expand::Auto => std::write!(f, "Auto"), - Expand::Always => std::write!(f, "Always"), - Expand::Never => std::write!(f, "Never"), - } + let s = match self { + Expand::Auto => "Auto", + Expand::Always => "Always", + Expand::Never => "Never", + }; + f.write_str(s) } }