Automattic / Automattic/harper
`replace_with_match_case` causes problems due to working by single character index at a time
- Dominant language
- Rust
- Stars
- 15.4k
- Forks
- 627
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 106
Description
This has come up again and again and I was pretty sure there was already a bug report about it. Maybe there is but it's named for the symptoms rather than the cause?
The cause is that there is only one method for generating a `Suggestion` that should "match" the case of the flagged error, and that method matches case on a character-by-character basis.
This works fine most of the time when the word is all-lowercase, all caps, or when it's just one word with its first letter uppercase and the rest lowercase.
But words with mixed case, or multiple words in a match with each beginning with a capital, frequently lead to unexpected results.
I'm hitting it right now in a new Weir rule that corrects "link list" to "linked list" where it's changing:
> 006 Link List
into:
> 006 LinkEd list
instead of:
> 006 Linked List
It seems the whitespace is ignored but "e" is getting uppercased to "E" because the "L" is the next letter after "k".
I know I've inserted comments in code and tests over a period of months and I've mentioned this in PRs and issues before. I'll attempt to gather them here below:
- ignored test in `ChockFull`
- ignored test in `Everyday`
- ignored test in `MostNumber`
- NOTE comment in `NoLongerPronoun`
- two ignored tests in `NotOnlyInversion`
- TODO comment in `OverPlus`
- ignored test in `PluralWrongWordOfPhrase`
- TODO comment in a test in `TheTheToThatThe`
- ignored test in `ThereIsAgreement`
- ignored test in `WouldNeverHave`
- ignored test in `WrongApostrophe`
- two ignored tests in `NounCountability`
- three ignored tests in `phrase_set_corrections`
A good fix for this will probably be multi-pronged and may perhaps be a breaking change.
- It should be data driven and work with the examples mentioned in all the linters mentioned above.
- It should not break any other tests.
- It should probably have some heuristics to decide a strategy based on characters, words, etc.
- It should probably be able to return multiple suggestions rather than just one, which may be a breaking change.
Contributor guide
Research direction
Start at replace_with_match_case and the Suggestion-generation method, then inspect the ignored tests and TODO/NOTE cases listed for the affected rules. Use those examples as regression cases, including “Link List” to “Linked list,” while checking that existing tests remain passing. Done means mixed-case and multiword corrections preserve the intended capitalization without regressions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100