fix(that-than): catch 'less/fewer [adj] that' periphrastic comparison typos#3786
fix(that-than): catch 'less/fewer [adj] that' periphrastic comparison typos#3786yakubka wants to merge 2 commits into
Conversation
… typos
Added a second SequenceExpr branch to ThatThan that matches
'less' or 'fewer' followed by a plain (positive) adjective, then 'that',
then any word except 'way'. This catches patterns like:
'way less common that mp3' -> 'way less common than mp3'
'less reliable that the previous one' -> 'less reliable than the previous one'
'fewer errors that expected' -> 'fewer errors than expected'
The original linter only handled comparative adjectives (words ending -er
like 'smaller', 'faster') preceding 'that'. Periphrastic comparisons using
'less'/'fewer' + plain adjective were missed.
'more [adj] that [...]' is deliberately excluded because it frequently
introduces a correct subordinate clause ('more clear that users need to...'),
making it too noisy to flag without deeper grammatical analysis.
match_to_lint now derives the 'that' token index from toks.len() instead
of hardcoding 2, supporting the 5-token (adj that word) and 7-token
(less adj that word) patterns.
Fixes Automattic#3736.
Co-authored-by: IbrokhimN <ibragimnurullayev@gmail.com>
"Fewer [adjective]" is not grammatical. "Fewer" can be used before a plural noun. You surely meant "less/more" here? |
hippietrail
left a comment
hippietrail
left a comment
There was a problem hiding this comment.
You surely mean "less <adj>" and "more <adj>" and not "fewer <adj>"?
| // "that" should be "than". "more [adj] that" is excluded because it | ||
| // frequently introduces a correct subordinate clause ("more clear that | ||
| // users need to..."). | ||
| let periphrastic_that_nextword = SequenceExpr::word_set(&["less", "fewer"]) |
There was a problem hiding this comment.
This is "fewer correct" than you think it might be (-;
yakubka
left a comment
yakubka
left a comment
There was a problem hiding this comment.
removed fewer from the word set. you are right that fewer pairs with countable nouns and would never precede an adjective in a valid comparison, so the pattern word_set([less, fewer]) was wrong. also dropped the fix_fewer_errors_that test since errors is a noun, not a positive adjective, so the periphrastic pattern never matched it. the pattern now only covers less [adj] that which is the genuine false-than case.
2 participants
Fixes #3736.
The ThatThan linter currently catches comparative adjective forms like "smaller that", "faster that", "longer that" by matching tokens where TokenKind::is_comparative_adjective precedes "that". It does not catch periphrastic comparisons of the form "less/fewer [positive adjective] that [...]", so "way less common that mp3" passes through unflagged.
This PR adds a second branch to the SequenceExpr that matches "less" or "fewer" followed by a positive adjective, then "that", then any word except "way". The match_to_lint function now derives the "that" token index from toks.len() to support both the 5-token pattern (adj ws that ws word) and the new 7-token pattern (less ws adj ws that ws word).
"more [adj] that" is excluded intentionally. "more [adj] that [clause]" frequently introduces a correct subordinate clause ("more clear that users need to...", "more explicit that those files are..."), so flagging it would produce too many false positives without deeper grammatical analysis. The existing test cases dont_flag_more_explicit_that and dont_flag_more_clear_that still pass.
Three new tests cover the added pattern. The remaining existing tests are unchanged.
Yokubjon Nurullaev identified the gap in the periphrastic branch, wrote the SequenceExpr and updated match_to_lint to handle the variable token index.
IbrokhimN reviewed the "more" exclusion decision, proposed the dont_flag tests that guard against subordinate clause false positives, and added the fix_fewer_errors_that test case.