From 73fedb965f6a21e34ba950eddc44ba8102d39fb8 Mon Sep 17 00:00:00 2001 From: leaysgur <6259812+leaysgur@users.noreply.github.com> Date: Mon, 19 Jan 2026 09:17:55 +0000 Subject: [PATCH] fix(oxfmt): Apply `.editorconfig` root section with `oxfmtrc.overrides` (#18210) Fixed a bug found through self-review. --- apps/oxfmt/src/core/config.rs | 36 +++++++++---------- .../oxfmtrc_overrides.test.ts.snap | 15 ++++++++ .../editorconfig_root_only/.editorconfig | 2 ++ .../editorconfig_root_only/.oxfmtrc.json | 3 ++ .../fixtures/editorconfig_root_only/test.js | 3 ++ .../oxfmtrc_overrides.test.ts | 15 ++++++++ 6 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.editorconfig create mode 100644 apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.oxfmtrc.json create mode 100644 apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/test.js diff --git a/apps/oxfmt/src/core/config.rs b/apps/oxfmt/src/core/config.rs index 11b46be3fba65..cabf4527ba36d 100644 --- a/apps/oxfmt/src/core/config.rs +++ b/apps/oxfmt/src/core/config.rs @@ -249,15 +249,16 @@ impl ConfigResolver { } /// Resolve options for a specific file path. - /// Priority: oxfmtrc base → oxfmtrc overrides → editorconfig (fallback for unset fields) + /// Priority: oxfmtrc base → oxfmtrc overrides → editorconfig (fallback for unset fields) -> defaults fn resolve_options(&self, path: &Path) -> (OxfmtOptions, Value) { - let editorconfig_overrides = - self.editorconfig.as_ref().and_then(|ec| get_editorconfig_overrides(ec, path)); + let has_editorconfig_overrides = + self.editorconfig.as_ref().is_some_and(|ec| has_editorconfig_overrides(ec, path)); let has_oxfmtrc_overrides = self.oxfmtrc_overrides.as_ref().is_some_and(|o| o.has_match(path)); // Fast path: no per-file overrides - if editorconfig_overrides.is_none() && !has_oxfmtrc_overrides { + // `.editorconfig` root section is already applied during `build_and_validate()` + if !has_editorconfig_overrides && !has_oxfmtrc_overrides { return self .cached_options .clone() @@ -265,6 +266,7 @@ impl ConfigResolver { } // Slow path: reconstruct `FormatConfig` to apply overrides + // See `build_and_validate()` for why we cannot base this on cached options directly let mut format_config: FormatConfig = serde_json::from_value(self.raw_config.clone()) .expect("`build_and_validate()` should catch this before"); @@ -274,10 +276,10 @@ impl ConfigResolver { format_config.merge(options); } } - - // Apply editorconfig as fallback (fills in unset fields only) - if let Some(props) = &editorconfig_overrides { - apply_editorconfig(&mut format_config, props); + // Apply `.editorconfig` as fallback (fills in unset fields only) + if let Some(ec) = &self.editorconfig { + let props = ec.resolve(path); + apply_editorconfig(&mut format_config, &props); } let oxfmt_options = format_config @@ -364,8 +366,7 @@ struct OxfmtrcOverrideEntry { /// Check if `.editorconfig` has per-file overrides for this path. /// -/// Returns `Some(props)` if the resolved properties differ from the root `[*]` section. -/// Returns `None` if no overrides. +/// Returns `true` if the resolved properties differ from the root `[*]` section. /// /// Currently, only the following properties are considered for overrides: /// - max_line_length @@ -373,15 +374,12 @@ struct OxfmtrcOverrideEntry { /// - indent_style /// - indent_size /// - insert_final_newline -fn get_editorconfig_overrides( - editorconfig: &EditorConfig, - path: &Path, -) -> Option { +fn has_editorconfig_overrides(editorconfig: &EditorConfig, path: &Path) -> bool { let sections = editorconfig.sections(); // No sections, or only root `[*]` section → no overrides if sections.is_empty() || matches!(sections, [s] if s.name == "*") { - return None; + return false; } let resolved = editorconfig.resolve(path); @@ -390,7 +388,7 @@ fn get_editorconfig_overrides( let root_props = sections.iter().find(|s| s.name == "*").map(|s| &s.properties); // Compare only the properties we care about - let has_overrides = match root_props { + match root_props { Some(root) => { resolved.max_line_length != root.max_line_length || resolved.end_of_line != root.end_of_line @@ -406,15 +404,13 @@ fn get_editorconfig_overrides( || resolved.indent_size != EditorConfigProperty::Unset || resolved.insert_final_newline != EditorConfigProperty::Unset } - }; - - if has_overrides { Some(resolved) } else { None } + } } /// Apply `.editorconfig` properties to `FormatConfig`. /// /// Only applies values that are not already set in the user's config. -/// NOTE: Only properties checked by [`get_editorconfig_overrides`] are applied here. +/// NOTE: Only properties checked by [`has_editorconfig_overrides`] are applied here. fn apply_editorconfig(config: &mut FormatConfig, props: &EditorConfigProperties) { #[expect(clippy::cast_possible_truncation)] if config.print_width.is_none() diff --git a/apps/oxfmt/test/cli/oxfmtrc_overrides/__snapshots__/oxfmtrc_overrides.test.ts.snap b/apps/oxfmt/test/cli/oxfmtrc_overrides/__snapshots__/oxfmtrc_overrides.test.ts.snap index e64d9746b2968..63c1c60623964 100644 --- a/apps/oxfmt/test/cli/oxfmtrc_overrides/__snapshots__/oxfmtrc_overrides.test.ts.snap +++ b/apps/oxfmt/test/cli/oxfmtrc_overrides/__snapshots__/oxfmtrc_overrides.test.ts.snap @@ -47,6 +47,21 @@ Finished in ms on 7 files using 1 threads. --------------------" `; +exports[`oxfmtrc overrides > editorconfig root section applies with oxfmtrc overrides 1`] = ` +"-------------------- +arguments: --check test.js +working directory: oxfmtrc_overrides/fixtures/editorconfig_root_only +exit code: 0 +--- STDOUT --------- +Checking formatting... + +All matched files use the correct format. +Finished in ms on 1 files using 1 threads. +--- STDERR --------- + +--------------------" +`; + exports[`oxfmtrc overrides > multiple overrides - later overrides take precedence 1`] = ` "-------------------- arguments: --check . diff --git a/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.editorconfig b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.editorconfig new file mode 100644 index 0000000000000..0b225afd48019 --- /dev/null +++ b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.editorconfig @@ -0,0 +1,2 @@ +[*] +indent_style = tab diff --git a/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.oxfmtrc.json b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.oxfmtrc.json new file mode 100644 index 0000000000000..bef78bd1fc0f5 --- /dev/null +++ b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/.oxfmtrc.json @@ -0,0 +1,3 @@ +{ + "overrides": [{ "files": ["*.js"], "options": { "semi": false } }] +} diff --git a/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/test.js b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/test.js new file mode 100644 index 0000000000000..b1fdef04eaac3 --- /dev/null +++ b/apps/oxfmt/test/cli/oxfmtrc_overrides/fixtures/editorconfig_root_only/test.js @@ -0,0 +1,3 @@ +if (true) { + console.log("hello") +} diff --git a/apps/oxfmt/test/cli/oxfmtrc_overrides/oxfmtrc_overrides.test.ts b/apps/oxfmt/test/cli/oxfmtrc_overrides/oxfmtrc_overrides.test.ts index 7fa7ed8ad55bc..8e3651e0b0d12 100644 --- a/apps/oxfmt/test/cli/oxfmtrc_overrides/oxfmtrc_overrides.test.ts +++ b/apps/oxfmt/test/cli/oxfmtrc_overrides/oxfmtrc_overrides.test.ts @@ -36,6 +36,21 @@ describe("oxfmtrc overrides", () => { expect(snapshot).toMatchSnapshot(); }); + // .editorconfig: + // [*] indent_style=tab + // .oxfmtrc.json: + // overrides: [{ files: ["*.js"], options: { semi: false } }] + // + // Expected: + // - test.js: useTabs=true (from editorconfig [*]), semi=false (from oxfmtrc overrides) + // + // This test verifies editorconfig [*] section is applied even when oxfmtrc overrides trigger slow path + it("editorconfig root section applies with oxfmtrc overrides", async () => { + const cwd = join(fixturesDir, "editorconfig_root_only"); + const snapshot = await runAndSnapshot(cwd, [["--check", "test.js"]]); + expect(snapshot).toMatchSnapshot(); + }); + // .oxfmtrc.json: // tabWidth: 2 // overrides: [{ files: ["src/*.js"], options: { tabWidth: 4 } }]