From aaae83345efde8ed94a34ec0166b11a2aabdab89 Mon Sep 17 00:00:00 2001 From: coyaSONG <66289470+coyaSONG@users.noreply.github.com> Date: Thu, 16 Jul 2026 04:10:00 +0900 Subject: [PATCH 1/2] fix(core): avoid overall quantifier false positives --- harper-core/src/linting/closed_compounds.rs | 85 ++++++++++++++++++- .../Alice's Adventures in Wonderland.snap.yml | 19 ----- ...Constitution of the United States.snap.yml | 11 --- .../text/linters/The Great Gatsby.snap.yml | 9 -- 4 files changed, 83 insertions(+), 41 deletions(-) diff --git a/harper-core/src/linting/closed_compounds.rs b/harper-core/src/linting/closed_compounds.rs index 55bccde6df..79034bdfec 100644 --- a/harper-core/src/linting/closed_compounds.rs +++ b/harper-core/src/linting/closed_compounds.rs @@ -1,7 +1,53 @@ -use crate::linting::LintGroup; +use crate::{ + Token, + expr::Expr, + linting::{ + ExprLinter, Lint, LintGroup, + expr_linter::{Chunk, preceded_by_word}, + }, +}; use super::MapPhraseLinter; +struct Overall { + inner: MapPhraseLinter, +} + +impl Default for Overall { + fn default() -> Self { + Self { + inner: MapPhraseLinter::new_closed_compound(["over all"], "overall"), + } + } +} + +impl ExprLinter for Overall { + type Unit = Chunk; + + fn expr(&self) -> &dyn Expr { + self.inner.expr() + } + + fn match_to_lint_with_context( + &self, + matched_tokens: &[Token], + source: &[char], + context: Option<(&[Token], &[Token])>, + ) -> Option { + // "Over all" is normally a preposition followed by a quantifier. Only treat it as the + // adjective "overall" when a preceding determiner establishes that grammatical role. + if !preceded_by_word(context, |word| word.kind.is_determiner()) { + return None; + } + + self.inner.match_to_lint(matched_tokens, source) + } + + fn description(&self) -> &str { + self.inner.description() + } +} + pub fn lint_group() -> LintGroup { let mut group = LintGroup::empty(); @@ -57,7 +103,6 @@ pub fn lint_group() -> LintGroup { "Nothing" => (&["no thing"][..], "nothing"), "Notwithstanding" => (&["not with standing"][..], "notwithstanding"), "Nowhere" => (&["no where"][..], "nowhere"), - "Overall" => (&["over all"][..], "overall"), "Overclocking" => (&["over clocking"][..], "overclocking"), "Overload" => (&["over load"][..], "overload"), "Overnight" => (&["over night"][..], "overnight"), @@ -83,6 +128,8 @@ pub fn lint_group() -> LintGroup { "Worthwhile" => (&["worth while", "worth-while"][..], "worthwhile"), }); + group.add("Overall", Box::new(Overall::default())); + group.set_all_rules_to(Some(true)); group @@ -157,6 +204,40 @@ mod tests { assert_suggestion_result(test_sentence, lint_group(), expected); } + #[test] + fn allow_over_all_the() { + assert_no_lints("Iterate over all the save data in the slot.", lint_group()); + } + + #[test] + fn allow_over_all_of_the() { + assert_no_lints( + "Iterate over all of the save data in the slot.", + lint_group(), + ); + } + + #[test] + fn allow_over_all_plural_noun() { + assert_no_lints( + "Congress may exercise authority over all places purchased by consent.", + lint_group(), + ); + } + + #[test] + fn allow_over_all_pronoun() { + assert_no_lints("A thrill passed over all of us.", lint_group()); + } + + #[test] + fn allow_over_all_at_sentence_start() { + assert_no_lints( + "Over all these years, the rule stayed the same.", + lint_group(), + ); + } + #[test] fn how_ever() { let test_sentence = "This is true, how ever, details matter."; diff --git a/harper-core/tests/text/linters/Alice's Adventures in Wonderland.snap.yml b/harper-core/tests/text/linters/Alice's Adventures in Wonderland.snap.yml index 3cccb6378c..c7faf5e981 100644 --- a/harper-core/tests/text/linters/Alice's Adventures in Wonderland.snap.yml +++ b/harper-core/tests/text/linters/Alice's Adventures in Wonderland.snap.yml @@ -472,15 +472,6 @@ Suggest: -Lint: Miscellaneous (31 priority) -Message: | - 238 | puzzle!” And she began thinking over all the children she knew that were of the - | ^~~~~~~~ Did you mean the closed compound `overall`? -Suggest: - - Replace with: “overall” - - - Lint: Capitalization (31 priority) Message: | 243 | all sorts of things, and she, oh! she knows such a very little! Besides, she’s @@ -1703,16 +1694,6 @@ Suggest: -Lint: Miscellaneous (31 priority) -Message: | - 1415 | dropped, and the party sat silent for a minute, while Alice thought over all she - | ^~~~~~~~ Did you mean the closed compound `overall`? - 1416 | could remember about ravens and writing-desks, which wasn’t much. -Suggest: - - Replace with: “overall” - - - Lint: Readability (127 priority) Message: | 1433 | The March Hare took the watch and looked at it gloomily: then he dipped it into diff --git a/harper-core/tests/text/linters/The Constitution of the United States.snap.yml b/harper-core/tests/text/linters/The Constitution of the United States.snap.yml index b31db1590c..d347e76f72 100644 --- a/harper-core/tests/text/linters/The Constitution of the United States.snap.yml +++ b/harper-core/tests/text/linters/The Constitution of the United States.snap.yml @@ -685,17 +685,6 @@ Suggest: -Lint: Miscellaneous (31 priority) -Message: | - 253 | the Acceptance of Congress, become the Seat of the Government of the United - 254 | States, and to exercise like Authority over all Places purchased by the Consent - | ^~~~~~~~ Did you mean the closed compound `overall`? - 255 | of the Legislature of the State in which the Same shall be, for the Erection of -Suggest: - - Replace with: “overall” - - - Lint: Readability (127 priority) Message: | 265 | The Migration or Importation of such Persons as any of the diff --git a/harper-core/tests/text/linters/The Great Gatsby.snap.yml b/harper-core/tests/text/linters/The Great Gatsby.snap.yml index e32d0dfe28..1da1d211ce 100644 --- a/harper-core/tests/text/linters/The Great Gatsby.snap.yml +++ b/harper-core/tests/text/linters/The Great Gatsby.snap.yml @@ -1549,15 +1549,6 @@ Suggest: -Lint: Miscellaneous (31 priority) -Message: | - 1307 | A thrill passed over all of us. The three Mr. Mumbles bent forward and listened - | ^~~~~~~~ Did you mean the closed compound `overall`? -Suggest: - - Replace with: “overall” - - - Lint: Spelling (63 priority) Message: | 1310 | “I don’t think it’s so much that,” argued Lucille sceptically; “it’s more that From ab04b4e9e817e396cfefa2ff217d884d194e6b2c Mon Sep 17 00:00:00 2001 From: coyaSONG <66289470+coyaSONG@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:44:59 +0900 Subject: [PATCH 2/2] test(core): clarify overall pronoun test name --- harper-core/src/linting/closed_compounds.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/harper-core/src/linting/closed_compounds.rs b/harper-core/src/linting/closed_compounds.rs index 79034bdfec..382c9af463 100644 --- a/harper-core/src/linting/closed_compounds.rs +++ b/harper-core/src/linting/closed_compounds.rs @@ -226,7 +226,7 @@ mod tests { } #[test] - fn allow_over_all_pronoun() { + fn allow_over_all_of_pronoun() { assert_no_lints("A thrill passed over all of us.", lint_group()); }