fix: don't read the pronoun one as a count before a participle (#3819) - #3889
fix: don't read the pronoun one as a count before a participle (#3819)#3889lukiod wants to merge 2 commits into
one as a count before a participle (#3819)#3889Conversation
…attic#3819) "affecting" is the participle of "affect" and carries mass noun metadata, so a quantifier before it matched NounCountability. In "the scaling of one affecting the other" that produced the suggestion "one piece of affecting". There is already a guard for verbal -ing forms, but it looks for a modifier chain ending in a head noun, as in "much programming knowledge", and it treats a determiner as the end of that chain. "affecting the other" is the other verbal shape: the determiner opens the participle's own object rather than closing a noun phrase, so the existing guard stopped at "the" and the lint was emitted. A determiner directly after an -ing form is a sufficient signal on its own, because a mass noun does not take an object. "much writing the letter" is not a noun phrase however it is read. Keyed on the -ing form so plain mass nouns are unaffected: "one advice the other day" is still flagged, which a test covers alongside the reported sentence and a second participle. 5681 tests pass.
|
Please use the provided PR template. All -ing forms of verbs can act as both present participles and gerunds. Gerunds behave as mass nouns by default (though may also have countable senses) and can still take objects:
Here "eating" is both a noun, the object of the verb "to like" and a verb, with its own object, "peas". This kind of thing makes it very tricky to parse the context around a potential mistake to determine if it's really a mistake or actually a legitimate usage. We have to be aware of when it's not a simple "either/or" such as "it can't be a noun because it's a verb" and have to see if there's a way to determine whether it's acting as a noun or a verb etc. in this particular context. Which can require analysing the next word along, which may also have some kind of ambiguity. Coding AIs tend to ignore this unless you spell it out for them and even then you might have to find concrete examples to convince them, and even then they make mistakes about how whitespace and punctuation are tokenized. We don't ban AI and humans make many of these same kinds of assumptions, but we should know what we're up against when we ask an AI to write a grammar checker rule. |
Review raised that a gerund can head a noun phrase and still take an object, so an -ing word with an object says nothing about whether it is nominal. That is correct, and testing turned up two concrete cases the previous approach got wrong. Keying on a determiner after an -ing form silences real errors, because the determiner may open a relative clause modifying the -ing word rather than an object of it. "Every training the company offers is mandatory" is a genuine error and the earlier guard hid it. Verified by disabling only that guard, at which point the case is caught again. Keying on "of" before "one" is also wrong, because "of one" occurs with a real quantifier too, as in "the cost of one advice". Requiring both, the pronoun "one" after "of" and a following participle, keeps the change to the construction the issue actually reports. This matches the report's own title, which identifies "one" as a pronoun rather than anything about -ing words. Tests cover the reported sentence, the gerund-with-object example from review, both relative clause cases, and "of one" before a plain mass noun.
lukiod
changed the title
one as a count before a participle (#3819)
|
Thanks, both points taken, and the second one turned out to be right in a way I could reproduce. I've filled in the PR template, including the AI disclosure. Apologies for missing it. On the grammar: you're correct that all Rather than argue the point I went looking for a sentence where my rule actually breaks, and there is one:
Here Going back to your issue title, the real problem is narrower than what I fixed. In I also checked the obvious narrower rule, " On invented test sentences, that's a fair caution and I'd flag it against my own PR: the issue sentence and
|
yah sry about that my ai went a way to abroad but yan need things for a big project so i thought of keeping things automated so as i can focus on the main issue but yah from now on will take care of things before creating a pr mb man |
|
@hippietrail brother can i expect a review or anything on this |
2 participants
Issues
Fixes #3819
Description
NounCountabilitymatches a count quantifier followed by a mass-noun-only word. In the reported sentence:affectingcarries mass noun metadata andoneis in the quantifier set, so the rule fired and suggestedone piece of affecting.But
onehere is a pronoun, the object ofof, withaffecting the othera participial phrase modifying it. There is no count quantifier before a mass noun, which is the only shape this rule targets. That is what the issue title says as well.The guard therefore skips when the matched quantifier is
one, it is preceded byof, and the mass noun is an-ingform.This is narrower than my first attempt, and the reason is worth recording. @hippietrail pointed out in review that a gerund can head a noun phrase and still take an object, so an
-ingword having an object tells you nothing about whether it is nominal. That is correct, and testing turned up two concrete cases where wider rules silence real errors:-ingform makes it verbal"Every training the company offers is mandatory.— the determiner opens a relative clause modifyingtraining, not an object of itoneafterofis a pronoun"The cost of one advice.—of onealso occurs with a genuine quantifierBoth are real errors that
mastercatches. Requiring both conditions keeps the change to the construction actually reported.For a linter, trading a false positive for missed errors is the worse deal, so I checked for that specifically rather than only checking that the reported sentence stopped firing.
Demo
Not applicable, this is a
harper-corerule change with no UI surface.How Has This Been Tested?
cargo test -p harper-core— 5684 passed, 0 failed.cargo fmt --checkclean,cargo clippyintroduces no new warnings.The two false-negative cases were confirmed by disabling only the new guard and re-running: with it disabled both are caught, with the first version of the guard enabled both were missed. That is how they were found rather than assumed.
New tests:
Some people do not like eating peas.— the gerund-with-object example from reviewEvery training the company offers is mandatory.andEvery funding the government provides is audited.— relative clause, must still flagThey questioned the cost of one advice.—of onewith a real quantifier, must still flagAI Disclosure
If Your PR Implements or Enhances a Linter
The issue sentence and the
eating peasexample come from the bug report and from review respectively. The relative clause andof onesentences are constructed, because they exist to pin specific failure modes; I have not verified them against a corpus, and I would treat that as a real limitation of this PR given the point raised in review about invented test cases.Checklist