fix(core): resolve Lang and Transform in section order, not map order - #1143
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's broken
When more than one config section matches a file, the resolved
LangandTransformare picked at random, and can differ between two files in the same run.With
[*] Lang = enand[*.md] Lang = ja, callingNewFileon the same.mdpath 200 times gaveen169 times andja31 times. Linting 40 identical.mdfiles in one invocation sentenfor 36 of them andjafor 6.Why
Both loops in
NewFilerange over a map, and Go randomises map iteration, sobreakon the first match picks an arbitrary winner per call.Langis not cosmetic here:internal/nlp/prose.gosends it into the POS-tagging request, andinternal/nlp/provider.gobranches onLang != "en"to decide whether to bypass the English-only segmenter.The fix
The same function already solves this twenty lines above, for styles and rule levels, by walking
config.RuleKeysin written order so a later section wins. That loop carries the comment referring to #965. These two loops now do the same, using the compiled patterns already inconfig.SecToPatinstead of recompiling a glob for every file.Two details worth calling out:
[*]is set throughglobalOptsand deliberately excluded fromRuleKeysandSecToPat, so the global value is seeded before the loop and any matching section overrides it.This changes the semantics from first-match-in-random-order to last-match-in-written-order. That is the point: it makes the behaviour match what the rule loop already documents, and the old order was never well defined.
Verification
internal/core/file_test.gocovers a path matched by several sections, asserting the last one wins, plus the[*]fallback. It passes 20 out of 20 with-count=20. With the change reverted it fails on all 20, reportingexpected Lang "it" ... got "en".go test ./internal/...is green, including the e2e package.