Skip to content

Commit 6013e68

Browse files
committed
refactor(oxfmt, formatter): clean up embedded formatter integration (#15937)
Aims to remove `embedded_formatter: Option<EmbeddedFormatter>` from the `Formatter` struct, which violates the `clippy::struct_field_names` rule. Store the `embedded_formatter` in the `Formatter` is unnecessary because it is immediately passed into the FormatContext.
1 parent 117f5df commit 6013e68

File tree

3 files changed

+49
-33
lines changed

3 files changed

+49
-33
lines changed

apps/oxfmt/src/service.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use cow_utils::CowUtils;
44
use rayon::prelude::*;
55

66
use oxc_allocator::AllocatorPool;
7-
use oxc_diagnostics::{DiagnosticSender, DiagnosticService};
7+
use oxc_diagnostics::{DiagnosticSender, DiagnosticService, OxcDiagnostic};
88
use oxc_formatter::{FormatOptions, Formatter, enable_jsx_source_type, get_parse_options};
99
use oxc_parser::Parser;
1010

@@ -93,24 +93,37 @@ impl FormatService {
9393
}
9494

9595
let base_formatter = Formatter::new(&allocator, self.format_options.clone());
96-
let formatter = if self.format_options.embedded_language_formatting.is_off() {
97-
base_formatter
96+
let formatted = if cfg!(not(feature = "napi"))
97+
|| self.format_options.embedded_language_formatting.is_off()
98+
|| self.external_formatter.is_none()
99+
{
100+
base_formatter.format(&ret.program)
98101
} else {
99102
#[cfg(feature = "napi")]
100103
{
101-
let external_formatter = self
102-
.external_formatter
103-
.as_ref()
104-
.map(crate::prettier_plugins::ExternalFormatter::to_embedded_formatter);
105-
base_formatter.with_embedded_formatter(external_formatter)
106-
}
107-
#[cfg(not(feature = "napi"))]
108-
{
109-
base_formatter
104+
let Some(external_formatter) = &self.external_formatter else {
105+
unreachable!("Already checked above that external_formatter is Some");
106+
};
107+
let embedded_formatter = external_formatter.to_embedded_formatter();
108+
base_formatter.format_with_embedded(&ret.program, embedded_formatter)
110109
}
111110
};
112111

113-
let code = formatter.build(&ret.program);
112+
let code = match formatted.print() {
113+
Ok(printed) => printed.into_code(),
114+
Err(err) => {
115+
let oxc_diagnostic =
116+
OxcDiagnostic::error(format!("Failed to print formatted code: {err}"));
117+
let diagnostics = DiagnosticService::wrap_diagnostics(
118+
self.cwd.clone(),
119+
path,
120+
&source_text,
121+
vec![oxc_diagnostic],
122+
);
123+
tx_error.send(diagnostics).unwrap();
124+
return;
125+
}
126+
};
114127

115128
#[cfg(feature = "detect_code_removal")]
116129
{

crates/oxc_formatter/src/formatter/context.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ impl<'ast> FormatContext<'ast> {
5252
comments: &'ast [Comment],
5353
allocator: &'ast Allocator,
5454
options: FormatOptions,
55+
embedded_formatter: Option<EmbeddedFormatter>,
5556
) -> Self {
5657
let source_text = SourceText::new(source_text);
5758
Self {
@@ -61,7 +62,7 @@ impl<'ast> FormatContext<'ast> {
6162
comments: Comments::new(source_text, comments),
6263
allocator,
6364
cached_elements: FxHashMap::default(),
64-
embedded_formatter: None,
65+
embedded_formatter,
6566
}
6667
}
6768

@@ -77,11 +78,6 @@ impl<'ast> FormatContext<'ast> {
7778
}
7879
}
7980

80-
/// Set the embedded formatter for handling embedded languages
81-
pub fn set_embedded_formatter(&mut self, embedded_formatter: Option<EmbeddedFormatter>) {
82-
self.embedded_formatter = embedded_formatter;
83-
}
84-
8581
/// Get the embedded formatter if one is set
8682
pub fn embedded_formatter(&self) -> Option<&EmbeddedFormatter> {
8783
self.embedded_formatter.as_ref()

crates/oxc_formatter/src/lib.rs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,11 @@ pub struct Formatter<'a> {
5050
allocator: &'a Allocator,
5151
source_text: &'a str,
5252
options: FormatOptions,
53-
embedded_formatter: Option<EmbeddedFormatter>,
5453
}
5554

5655
impl<'a> Formatter<'a> {
5756
pub fn new(allocator: &'a Allocator, options: FormatOptions) -> Self {
58-
Self { allocator, source_text: "", options, embedded_formatter: None }
59-
}
60-
61-
/// Set the embedded formatter for handling embedded languages in templates
62-
#[must_use]
63-
pub fn with_embedded_formatter(
64-
mut self,
65-
embedded_formatter: Option<EmbeddedFormatter>,
66-
) -> Self {
67-
self.embedded_formatter = embedded_formatter;
68-
self
57+
Self { allocator, source_text: "", options }
6958
}
7059

7160
/// Formats the given AST `Program` and returns the formatted string.
@@ -74,7 +63,25 @@ impl<'a> Formatter<'a> {
7463
formatted.print().unwrap().into_code()
7564
}
7665

77-
pub fn format(mut self, program: &'a Program<'a>) -> Formatted<'a> {
66+
#[inline]
67+
pub fn format(self, program: &'a Program<'a>) -> Formatted<'a> {
68+
self.format_impl(program, None)
69+
}
70+
71+
#[inline]
72+
pub fn format_with_embedded(
73+
self,
74+
program: &'a Program<'a>,
75+
embedded_formatter: EmbeddedFormatter,
76+
) -> Formatted<'a> {
77+
self.format_impl(program, Some(embedded_formatter))
78+
}
79+
80+
pub fn format_impl(
81+
mut self,
82+
program: &'a Program<'a>,
83+
embedded_formatter: Option<EmbeddedFormatter>,
84+
) -> Formatted<'a> {
7885
let parent = self.allocator.alloc(AstNodes::Dummy());
7986
let program_node = AstNode::new(program, parent, self.allocator);
8087

@@ -89,8 +96,8 @@ impl<'a> Formatter<'a> {
8996
&program.comments,
9097
self.allocator,
9198
self.options,
99+
embedded_formatter,
92100
);
93-
context.set_embedded_formatter(self.embedded_formatter);
94101

95102
let mut formatted = formatter::format(
96103
context,

0 commit comments

Comments
 (0)