fix(core): avoid overall quantifier false positives - #3830
Open
coyaSONG wants to merge 2 commits into
Open
Conversation
coyaSONG
marked this pull request as ready for review
July 15, 2026 19:58
hippietrail
requested changes
Jul 16, 2026
hippietrail
left a comment
Collaborator
There was a problem hiding this comment.
I've only had a cursory glance at this and the results look good but I haven't analysed the logic yet. I would like to be explicit about the name of one test needing "of", which is very very minor.
| } | ||
|
|
||
| #[test] | ||
| fn allow_over_all_pronoun() { |
Collaborator
There was a problem hiding this comment.
I would call this allow_over_all_of_pronoun.
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.
Issues
Fixes #1249
Description
The
Overallclosed-compound rule previously suggestedoverallfor every occurrence ofover all. In normal phrases such as “iterate over all the save data,” however,overis a preposition andallis a quantifier.This change gives
Overalla small context-aware wrapper and emits the existingMapPhraseLintercorrection only when a preceding determiner establishes the adjectival use, as in the existing “The over all performance...” positive test. It adds the two issue examples plus quantifier cases covering a plural noun, a pronoun, and sentence-initial usage. Regenerated corpus snapshots remove four existing false positives from the U.S. Constitution, Alice's Adventures in Wonderland, and The Great Gatsby.Related: #3455 was an earlier attempt that special-cased words following
over all; its author closed it. This patch uses grammatical context and covers the additional false positives already present in Harper's corpus snapshots.Demo
Not visual.
How Has This Been Tested?
just formatcargo test -p harper-core -- closed_compounds::tests(40 passed)cargo test -p harper-core --test linters -- test_most_lints(snapshot regeneration followed by a clean pass)cargo test -p harper-core --lib(5,526 passed, 284 existing ignored, 0 failed)cargo clippy -p harper-core --lib -- -D warnings -D clippy::dbg_macro -D clippy::needless_raw_string_hashescargo check -p harper-corejust test-rust(full Rust workspace, 0 failed)git diff --checkAI Disclosure
If Your PR Implements or Enhances a Linter
Checklist