fix(core): stop LetsConfusion flagging quantifiers after let's

[mauropereiira]

Issues

Fixes #4037

Description

LetUsRedundancy matched let's followed by any pronoun:

SequenceExpr::aco("let's").then_whitespace().then_pronoun()

The curated dictionary tags all, both, everyone and everybody as pronouns, so ordinary sentences got flagged as a redundant subject. all and both are floating quantifiers in apposition to the us already inside let's. "Let's all go home." expands to "Let us all go home.", not "Let us us go home."

This matches an object pronoun instead. In the curated metadata, is_object separates the two groups exactly:

word	pronoun.is_object	determiner.is_quantifier
us	true	not set
me	true	not set
him	true	not set
her	true	not set
them	true	not set
it	true	not set
you	true	not set
all	not set	true
both	not set	true
everyone	not set	not set
everybody	not set	not set

In the issue thread @hippietrail suggested filtering on is_quantifier, or a shortlist. I tried the is_quantifier route first and it does not cover the set: everyone and everybody carry no quantifier tag, so they would keep firing. then_object_pronoun already exists in expr/sequence_expr.rs, so no new helper is needed.

No snapshot files change. The Alice in Wonderland line in harper-core/tests/text/ is written let’s all move one place on. with a typographic apostrophe, and aco("let's") cannot match that, so this false positive never fired on the corpus. It becomes visible only once Word normalises apostrophes, which is #3202.

#3696 covers let's you. Since you is an object pronoun, this change leaves that behaviour exactly as it was.

Demo

Before:

harper-cli lint "Let's all go home."          ->  <LetsConfusion: 1>
harper-cli lint "Let's both try again."       ->  <LetsConfusion: 1>
harper-cli lint "Let's everyone take a seat." ->  <LetsConfusion: 1>

After:

Let's all go home.            ->  (none)
Let's both try again.         ->  (none)
Let's everyone take a seat.   ->  (none)
Let's everybody take a seat.  ->  (none)
let's all move one place on.  ->  (none)

let's us do                   ->  <LetsConfusion: 1>
let's me do                   ->  <LetsConfusion: 1>
The crutch let's him walk.    ->  <LetsConfusion: 1>
let's them do                 ->  <LetsConfusion: 1>

How Has This Been Tested?

cargo test -p harper-core --lib

test result: ok. 6133 passed; 0 failed; 290 ignored; 0 measured; 0 filtered out

The existing LetsConfusion tests still pass unchanged, including walking ("The crutch let's him walk."), issue_426_us, issue_426_me and issue_548. cargo fmt is clean and no files under harper-core/tests/text/ are modified.

AI Disclosure

• I am a human and didn't use any AI.
• I used LLM features of my editor, but not an agent.
• I consulted one or more coding AIs, but didn't use an agent.
• I used an AI agent interactively.
• I am an agent or I got an agent to do the work autonomously.

If Your PR Implements or Enhances a Linter

• I made up the sentences in the unit tests.
• The sentences in the unit tests were generated by an AI.
• I'm using examples from the bug report / feature request.
• I collected real-world sentences for the unit tests.

Three of the four test sentences are verbatim from the issue. The fourth, "Let's everybody take a seat.", comes from @hippietrail's question in the issue thread about whether everybody behaves the same way. It does, and it is the case the is_quantifier approach would have missed, so it seemed worth pinning.

Checklist

• I have performed a self-review of my own code
• I have added tests to cover my changes
• I have considered splitting this into smaller pull requests.

[hippietrail]

Oh at some point I added a "personal pronoun" category. This is a term from linguistics meaning that pronouns that encode 1st/2nd/3rd person and not the everyday sense which would imply they're pronouns which refer to people and not to things.

Anyway, pronouns such as "all" are not personal pronouns, so maybe another/better approach might be to switch a .then_pronoun() to .then_personal_pronoun() or something like that?

[mauropereiira]

That distinction is useful, thanks, and both predicates do encode it. The metadata for the words involved:

word	is_personal	is_object
us	true	true
me	true	true
him	true	true
her	true	true
them	true	true
it	true	true
you	true	true
I	true	not set
we	true	not set
he	true	not set
she	true	not set
they	true	not set
all	not set	not set
both	not set	not set
everyone	not set	not set
everybody	not set	not set

So either predicate fixes the reported bug, since none of the quantifiers is a personal pronoun. The difference is that personal_pronoun also picks up the subject pronouns.

I think that tips it toward object_pronoun, because of what the rule suggests rather than what it matches. The first suggestion is built as:

format!("lets {pronoun}")

so the matched word becomes the object of lets, which only works if it is an object pronoun. I built both and compared:

                     personal_pronoun            object_pronoun
let's us do it       Replace with: "lets us"     Replace with: "lets us"
let's we do it       Replace with: "lets we"     no lint
let's they do it     Replace with: "lets they"   no lint
let's I do it        Replace with: "lets I"      no lint

"lets we do it" and "lets I do it" are not English, so personal_pronoun widens the match into a region where the rule cannot produce a correct fix. Those inputs are rare enough that it hardly matters in practice, but object_pronoun is the set the existing suggestion is actually valid for.

Happy to switch if you would rather have the wider match and put a guard on the suggestion instead.

[hippietrail]

Yep my apologies if I send you down any rabbit holes do due commenting after only skimming rather than deeply reading and understanding.

[mauropereiira]

No apology needed, both of those went somewhere useful.

The .is_auxiliary_verb() question turned into #4086, which I would not have gone looking for otherwise. And the personal_pronoun one made me actually build both variants and compare the suggestions, which is how I found that the wider match produces "lets we". Before that I only had a hunch that object_pronoun was the right set, not a reason.

A skim that asks "have you considered X" is worth a lot when the answer turns out to be "no, and here is what happens when I try it".