Skip to content

perf(oxlint): return first found config for resolving config#21055

Draft
Sysix wants to merge 1 commit intomainfrom
04-05-perf_oxlint_return_first_found_config_for_nested_search
Draft

perf(oxlint): return first found config for resolving config#21055
Sysix wants to merge 1 commit intomainfrom
04-05-perf_oxlint_return_first_found_config_for_nested_search

Conversation

@Sysix
Copy link
Copy Markdown
Member

@Sysix Sysix commented Apr 4, 2026

No description provided.

Copy link
Copy Markdown
Member Author

Sysix commented Apr 4, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-performance Category - Solution not expected to change functional behavior, only performance labels Apr 4, 2026
@Sysix Sysix changed the title perf(oxlint): return first found config for nested search perf(oxlint): return first found config for resolving config Apr 4, 2026
@Sysix Sysix requested a review from Copilot April 4, 2026 22:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes oxlint’s nested config discovery when walking ancestor directories by returning only the first config found per directory instead of collecting all possible config files.

Changes:

  • Updated ancestor config discovery to use a single optional config per directory (early-return).
  • Changed find_configs_in_directory from returning Vec<DiscoveredConfig> to Option<DiscoveredConfig>.
Comments suppressed due to low confidence (1)

apps/oxlint/src/config_loader.rs:72

  • In discover_configs_in_ancestors, the “stop at base config” boundary relies on find_configs_in_directory returning the base config when it exists. After changing that helper to return only a single (first-found) config, a directory with multiple config files can cause the base config path to be missed, so the walk can continue past the intended root and also skip emitting a conflict error. Consider preserving multi-config discovery here (or explicitly detecting conflict + base-config presence) so ancestor traversal is correctly bounded and conflicts are still diagnosed.
            if let Some(config) = find_configs_in_directory(dir) {
                if config.path() == base_config_path {
                    // Stop if we've reached the base config file (e.g., root oxlintrc)
                    // to avoid duplicate loading and filling nested config with configs outside from the root config.
                    break;
                }
                config_paths.insert(config);
            }

Comment on lines +107 to 120
fn find_configs_in_directory(dir: &Path) -> Option<DiscoveredConfig> {
let json_path = dir.join(DEFAULT_OXLINTRC_NAME);
if json_path.is_file() {
configs.push(DiscoveredConfig::Json(json_path));
return Some(DiscoveredConfig::Json(json_path));
}
let jsonc_path = dir.join(DEFAULT_JSONC_OXLINTRC_NAME);
if jsonc_path.is_file() {
configs.push(DiscoveredConfig::Jsonc(jsonc_path));
return Some(DiscoveredConfig::Jsonc(jsonc_path));
}

let ts_path = dir.join(DEFAULT_TS_OXLINTRC_NAME);
if ts_path.is_file() {
configs.push(DiscoveredConfig::Js(ts_path));
return Some(DiscoveredConfig::Js(ts_path));
}
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_configs_in_directory now returns only the first matching config file (JSON → JSONC → TS) and stops checking the other filenames. This changes behavior from the previous implementation that discovered all config files in a directory, which is needed so load_many can detect and error on per-directory config conflicts (see config_conflict_diagnostic). With the new logic, a directory containing multiple config files will silently pick one and will no longer surface the conflict diagnostic during ancestor discovery, potentially applying the wrong config.

Copilot uses AI. Check for mistakes.
@camc314 camc314 self-assigned this Apr 5, 2026
@graphite-app graphite-app bot changed the base branch from 04-04-fix_oxlint_don_t_search_for_nested_config_outside_cwd to graphite-base/21055 April 5, 2026 11:50
@graphite-app graphite-app bot force-pushed the graphite-base/21055 branch from 69594fa to 64a1a7e Compare April 5, 2026 11:57
@graphite-app graphite-app bot force-pushed the 04-05-perf_oxlint_return_first_found_config_for_nested_search branch from ba61312 to 6029159 Compare April 5, 2026 11:57
@graphite-app graphite-app bot changed the base branch from graphite-base/21055 to main April 5, 2026 11:57
@graphite-app graphite-app bot force-pushed the 04-05-perf_oxlint_return_first_found_config_for_nested_search branch from 6029159 to 1171ce7 Compare April 5, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli Area - CLI A-linter Area - Linter C-performance Category - Solution not expected to change functional behavior, only performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants