fix(core): flag possessive "it's" after "has" and "had"

[mauropereiira]

Issues

Fixes #3627

Description

ItsPossessive's mid-sentence expression required the token immediately before it's to tag as VERB or ADP:

let mid_sentence = SequenceExpr::with(UPOSSet::new(&[UPOS::VERB, UPOS::ADP]))

Harper's Brill tagger labels possessive "has" and "had" as AUX, not VERB. So the whole has it's <adjective> <noun> class was never matched, which is why the reporter's "but has it's own stack" went unflagged.

The word "own" turned out to be a red herring. It tags as ADJ and already matched fine after a true verb. On master, before this patch:

The company lost it's own rules.   ->  ItsPossessive fires
The company has  it's own rules.   ->  no lint
The company lost it's new rules.   ->  ItsPossessive fires
The company has  it's new rules.   ->  no lint

Only the preceding word differs between those pairs, so the gap is the tag on the verb, not anything about the noun phrase.

This adds "has" and "had" to the leading token set. I deliberately did not add UPOS::AUX wholesale, because the copula also tags AUX, and that would introduce false positives on sentences like "The problem is it's poor design.", where it's genuinely means "it is". Both of those cases are covered by new regression tests.

I also checked "have", "hasn't" and "hadn't": they already tag as VERB in this position and match without any change, so they are not in the set.

Scope note: this touches one file and does not modify default_config.json, since ItsPossessive is already registered and enabled.

Demo

Not applicable, this is a linter rule change. CLI output is below.

How Has This Been Tested?

cargo test -p harper-core — 6125 + 41 tests pass, 0 failures, no snapshot changes.

Manually with cargo run --bin harper-cli --release -- lint, comparing a build of master against this branch.

Now flagged, previously silent:

The child process has it's own stack.   ->  ItsPossessive
The company has it's own rules.         ->  ItsPossessive
The process had it's own stack.         ->  ItsPossessive
The company has it's new rules.         ->  ItsPossessive

Still clean, regression guards:

The problem is it's poor design.
The reason is it's bad code.
It's hard to tell from here.
it's good practice to review the general settings

Unchanged, paths that already worked:

Some libraries have it's own parser.    ->  ItsPossessive
The process hasn't it's own stack.      ->  ItsPossessive
The company lost it's various colors.   ->  ItsPossessive

AI Disclosure

• I am an agent or I got an agent to do the work autonomously.

If Your PR Implements or Enhances a Linter

• I'm using examples from the bug report / feature request.

The "has it's own stack" case comes directly from the issue. The remaining sentences are minimal variations built to isolate the AUX vs VERB distinction and to pin the copular false positives.

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.

Related but deliberately excluded, happy to open these separately:

1. ItsPossessive does not match typographic apostrophes. The project drifted from it’s goal. is silent, while the straight-quote version flags. That is issue False negative: from it’s goal not flagged to correct to from its goal #3202, and the cause is different: t_aco is backed by Word, which compares chars directly, whereas WordSet normalises the apostrophe (see the supports_typographic_apostrophes test in word_set.rs). Worth deciding whether Word should normalise too, since that would affect many linters, so I did not touch it here.
2. Possible pre-existing false positive, unrelated to this change: I have heard it's good news. flags on master today, where it's correctly means "it is".

[hippietrail]

Just sharing a thought about UPOS vs TokenKind.

[hippietrail]

I never got into UPOS but in the regular DictWordMetadata via TokenKind, auxiliary verb is a property on verb so you can use .is_auxiliary_verb() or just .is_verb(), which seems cleaner to me than special-casing "has" and "had" but I'm not really sure here. Might be worth trying out?

[mauropereiira]

Good question, and it sent me down a useful path. I tried it and the answer is that TokenKind does not work here, but the reason is more interesting than I expected.

First, .is_auxiliary_verb() specifically will not do it. has and had are not tagged as auxiliary in the curated dictionary, only have and be are:

WORD     verb     auxiliary  linking
has      true     not set    not set
had      true     not set    not set
have     true     true       not set
is       true     not set    true
was      true     not set    true

So that predicate misses exactly the two words this PR is about.

.is_verb() does cover them, and I had to exclude linking verbs to keep the copula guard, so I built and tested this version:

let mid_sentence_lead = UPOSSet::new(&[UPOS::VERB, UPOS::ADP])
    .or(|tok: &Token, _: &[char]| tok.kind.is_verb() && !tok.kind.is_linking_verb());

It compiles, passes all 6125 tests with no snapshot changes, and behaves identically to the current version on every sentence in this PR, including the copula cases. It looks like a clean win right up until you try inputs no test covers:

                                  current    is_verb()
In the end it's good news.        ok         FLAG
The water it's clear surface.     ok         FLAG
The report it's main findings.    ok         FLAG
The process it's own stack.       ok         FLAG
The design it's poor quality.     ok         FLAG

That is the crux of your UPOS versus TokenKind question. TokenKind::is_verb() is a dictionary lookup asking whether the word has a verb sense anywhere, with no disambiguation. UPOS is what the tagger decided for this token in this sentence. So every noun that also happens to be a verb, and English has a lot of them, becomes a valid lead token.

So I am leaving the PR as it is. The two-word set is not elegant, but it is scoped to a known tagger gap rather than trading a precise signal for a fuzzy one. Happy to be talked out of it if you see a third option I have missed.

Incidentally, two of my probes fail on both versions, so they are pre-existing rather than anything this PR introduces:

At the start it's hard work.      FLAG
After the show it's late night.   FLAG

Those look like the same family as #4038.

[hippietrail]

First, .is_auxiliary_verb() specifically will not do it. has and had are not tagged as auxiliary in the curated dictionary, only have and be are:

You're right! In dictionary.dict only some of them have the /A flag and I didn't check all the negative contractions either.

This is its own bug that needs to be fixed!

One problem is that "auxiliary verb" is a somewhat fuzzy term and not all sources put the same things in it. I think I have a tab open with research I was doing on this very question still open... Hmm no I must've cleaned up excessive "open for later use" tabs.

[mauropereiira]

Agreed, and I audited it so the data is written down somewhere. Opened #4086.

Short version: the modals are the one clean group, all nine carry the flag. The rest is not.

be    A      have   A      do     A
am    -      has    -      does   A
is    -      had    -      did    A
are   -      having -      doing  -
was   -                    done   -
were  -
being -
been  -

The negative contractions split down the middle too, with isn't flagged but aren't, wasn't and weren't not, and haven't flagged but hasn't and hadn't not.

Also trojan/NgSVAdG carries it, which came in with #2202. best and better do as well, though better is at least arguable through the "had better" construction.

On your fuzziness point, I deliberately have not turned the missing list into a patch. Where the boundary sits for the semi-modals and for being/been/doing/done decides which of those gaps are real errors, so it wants a definition first. Happy to do the dictionary pass once there is a call on it.

None of this changes this PR, the two-word set stands regardless.