Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Status: ready for review. Rebased onto 9b4284f on 2026-10-21.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — a mechanical, behavior-preserving collapse of five kind bools into one enum.
What was reviewed:
- Each
Semanticspredicate insrc/lint/linter/config/flat.rsagainst the literals the four constructors used to pass (Legacy T/T/T/F/F, Rc F/T/F/T/F, Flat T/F/F/F/F, BuiltIn T/F/F/F/T) — all match. - The two rewritten matches (
written_forinReader::rules, the globalsTablespick inConfig::merge) and theeslint_majorseed infinish()— each arm maps to the same result the old guard/tuple match produced, givenhas_every_ruleandprefers_typescript_ruleswere never both true. - Grepped
src/lint/linter/configfor leftover reads of the removed fields — none;Semanticswas already imported ineslintrc.rsandrc.rs.
Extended reasoning...
The change touches only the bun lint configuration loader (src/lint/linter/config/{flat,mod,eslintrc,rc}.rs) and drops one mordant baseline line; it replaces five booleans on Reader and Config with a single Semantics enum whose predicates reproduce the old per-constructor values. It touches no security-sensitive surface (no auth, crypto, path handling, or untrusted-input parsing changes). It was approved because every predicate and both rewritten matches were checked against the pre-change literals and found equivalent, the diff is self-contained within one private module, no CODEOWNERS entry covers these paths, and the bug hunt ran dry with no findings.
|
Updated 5:34 AM PT - Oct 21st, 2026
❌ @robobun, your commit fd87712 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 45664That installs a local version of the PR into your bun-45664 --bun |
The kind of the configuration file was spelled by five bools on the Reader and on Config: keeps_options, accepts_all_plugins, is_legacy, prefers_typescript_rules and has_every_rule. The six constructors make four of their 32 combinations, one per kind: a flat configuration, the built-in configuration, the files of ESLint 8, and an .oxlintrc.json. Semantics is now an enum with those four variants. The Reader of each constructor names its kind once, Reader::finish takes no argument, and Config stores the kind in place of its five bools. Each bool is a predicate of the enum with an exhaustive match, so a fifth kind stops at every one of them. Removes the mordant baseline line "bool_cluster:src/lint/linter/config/flat.rs" = 1.
b7c3c12 to
fd87712
Compare
There was a problem hiding this comment.
LGTM — a mechanical bool-cluster-to-enum refactor that reproduces the old flag combinations exactly.
What was reviewed:
- Checked each
Semanticspredicate against the oldReader/finishliterals offrom_flat(bothhas_every_rulevalues),from_legacyandfrom_rc_json— all four rows match. - Checked the three rewritten matches (
builtin_plugininread_rules,eslint_majorinfinish,env_tablesinmerge) — each is equivalent to the old bool conditions, sincehas_every_ruleholds only forBuiltInandprefers_typescript_rulesonly forRc. - Grepped for leftover reads of the removed fields: the remaining
.has_every_rule/.prefers_typescript_rulesuses are onResolvedConfig, which keeps its bools; no otherReaderconstructor exists.
Extended reasoning...
The change touches only src/lint/linter/config (flat.rs, mod.rs, eslintrc.rs, rc.rs) and drops one mordant-baseline.toml line; it replaces five bools on Reader and Config with a single pub(super) enum and turns field reads into method calls. It touches no security-sensitive surface (no parsing of untrusted input changes, no auth or crypto). The approve decision rests on having verified the per-variant truth tables and the three rewritten matches against the pre-change constructor literals, the absence of any remaining constructor or reader of the removed fields, no CODEOWNERS coverage of these paths, and the bug hunt ending on a dry streak with no objections visible in the timeline.
Problem
bun lintholds the kind of the configuration file as five bools onReaderandConfig(src/lint/linter/config). Six constructors make four of the 32 combinations.bool_clusterreports three of them: "linter::config::flat::Semanticshas 3 bool fields (keeps_options,accepts_all_plugins,is_legacy), and the crate only makes 3 of their 8 combinations".Fix
Semanticsis an enum:Flat,BuiltIn,Legacy,Rc. Each former bool is a predicate with an exhaustivematch. Each constructor names its kind, andConfigstores it.ResolvedConfigandLanguageOptionskeep their kind bools. Both are public and are also built without aConfig(see Notes)."bool_cluster:src/lint/linter/config/flat.rs" = 1.bun run rust:mordant(564c8da, three targets) exits 0. Eighttest/cli/lintfiles (debug build): 2615 pass, 1 fail.mainfails that test the same way (see Notes).Background
eslint.config.*(Flat), the ESLint 8 files (Legacy), an.oxlintrc.json(Rc), and the configuration for a project with none (BuiltIn).Reader::finishquiets the lint, butConfigkeeps three bools. Three variants leave two bools. Sharing the driver'sFlavoradds public surface.Downsides
.textofbun_lintgrew by 119 bytes on x86-64 and 28 on arm64, and the per-file path by 1 instruction. Measured on the first base (5866b5f), not again after the rebase.Notes
What stays, and why:
ResolvedConfigkeepsskips_unknown_rules,prefers_typescript_rules,has_every_ruleandunderstands_oxlint_comments(src/lint/linter/resolved.rs).Config::mergecopies the kind into them and the linter decides by them. Thebool_clusterfinding is on the private types oflinter::config.ResolvedConfigis a public type with a second producer that has noConfig(ResolvedConfig::from_json).LanguageOptionskeepsrejects_what_parser_rejects,is_oxlint,has_no_foreign_configandhas_oxc_scopes(src/lint/language.rs).Config::mergeis one of several producers.bun fmt(src/lint/driver/fmt/mod.rs) and the module graph (src/lint/graph/lib.rs) build one without aConfig. For oxfmt,bun fmtsetsrejects_what_parser_rejectsto false withis_oxlintfalse, a combination that noSemanticsvariant gives.Flavorbeside theConfig.from_flatkeeps its privatehas_every_rule: boolparameter and picksFlatorBuiltInfrom it.eslint_majorstays a field because the driver can set it after construction.Truth table, checked against the literals of the constructors on 9b4284f (
Ttrue,Ffalse):Flatfrom_flat_json,from_flat_json_with_pluginsBuiltInwithout_config_fileLegacyfrom_legacyRcfrom_rc_json,from_rc_json_with_pluginsThree reads became a
matchon the kind:builtin_plugininReader::rules, the seed ofeslint_majorinReader::finish, andenv_tablesinConfig::merge. The other 43 are a predicate call in place of a field read.Verification on the commit of this PR (base 9b4284f):
bun run rust:mordantexits 0 and prints no finding over the baseline.cargo clippy -p bun_lint --no-depsprints no warning, andcargo fmt --checkis clean.bun bd test --max-concurrency=4 --timeout=120000, one file at a time:config-files(842 pass, 260 skip),bunfig(406),defaults(261),lint(711 pass, 14 skip, 17 todo, 1 fail),js-plugins(146 pass, 1 skip),explain(19),native-rules(193),oxc-names(37).conformance.test.tswas not run.bun lintruns use all of the memory.eslintcomment nested about as deep as the stack allows is reported as one error", whichbun check: fix errors lost since #45800, and 9 to 37% faster #46001 added. Itsbun lintrun needs 140 s on this debug build, and the test helper ends a child after 100 s (test/cli/children.ts). Run by hand, the command exits 1 and reports one error for each of the 274 files, which is what the test expects.main(9b4284f, without this change) behaves the same on the same machine: 146 s by hand with byte-identical output, and the same failure in the test.test/change: the diff changes no behaviour, so no test can fail before it and pass after it. The check that the finding is gone is themordantjob on this PR. That job is advisory, so read its result directly.Measurements. These were taken on the first base (5866b5f), with and without the change as it was then. They were not taken again after the rebase:
size_of:Config848 -> 840 bytes,Reader272 -> 272 bytes,Semantics3 -> 1 byte (x86-64)..textofbun_lint(llvm-size,codegen-units=1): 2,711,095 -> 2,711,214 bytes on x86-64, 2,517,448 -> 2,517,476 bytes on arm64.Semanticspredicate exists as a symbol, and nothing calls one.Within::get640/87/30 -> 640/87/30.Config::get_with_frameworks340/27/10 -> 341/27/10.Config::merge5036/616/337 -> 5066/609/338.Config::js_rule285/32/17 -> 285/32/17.Reader::native_rule78/9/3 -> 86/9/3.Reader::rules998/135/69 -> 1001/134/69.Reader::finish1263/127/54 -> 1261/128/54.Within::get642/83/34 -> 641/83/34.Config::get_with_frameworks186/25/10 -> 187/25/10.Config::merge5231/662/376 -> 5225/657/376.Config::js_rule267/33/22 -> 269/33/22.Reader::native_rule72/9/3 -> 76/10/3.Reader::rules910/145/70 -> 925/149/71.Reader::finish1192/136/51 -> 1177/136/51.Self-review: five concerns, all about this description and none about the code. State the two exclusions with their true reasons (two concerns). Promise no follow-up. Write the verification to what ran on the commit that ships. Name the overlap with other pull requests.
Overlap, as of 2026-10-21:
bun lint: JSON and Markdown natively, programs in parallel.bun check -b: share work between projects.bun fmt:package.jsonand Svelte without Prettier #45595,bun check: 1.4x to 7.8x faster on large projects. Fixes forbun check,bun lintandbun fmt#45800 andbun check: fix errors lost since #45800, and 9 to 37% faster #46001 landed first and changed these files (names, comments, and three new reads of the kind). This PR is rebased onto 9b4284f. It is the same change, applied again to the code as it is now.Reader::native_ruleinflat.rs. Whichever lands second has a textual conflict there.mordant-baseline.toml. It merges with this PR without a conflict.mainwithout a textual conflict but does not compile.bun check: 1.4x to 7.8x faster on large projects. Fixes forbun check,bun lintandbun fmt#45800 was in that state before it landed.Behaviour change: none
no test proof · iteration 0 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check