| β¦ | β¦ | @@ -0,0 +1,178 @@ | |
| 148 | + | if (loop.test != null && !isCounterTest(loop.test)) return true; | |
| 149 | + | return failsAfterLoop(loop); | |
| 150 | + | } | |
| 151 | + | return !isTrueLiteral(loop.test); | |
π‘ Medium rules/no-polling-loop-in-tests.ts:151
A fixed-count while or do/while flush is reported as polling even though its condition is only a counter bound, so harmless bounded loops get false-positive diagnostics. hasPredicateExit applies isCounterTest only to ForStatement; apply the same counter-test exemption to the other loop forms.
| 1 | - return !isTrueLiteral(loop.test); |
| 2 | + return !isTrueLiteral(loop.test) && !isCounterTest(loop.test); |
π Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @oxlint-plugin-t3code/rules/no-polling-loop-in-tests.ts around line 151: |
| 2 | |
| 3 | A fixed-count `while` or `do/while` flush is reported as polling even though its condition is only a counter bound, so harmless bounded loops get false-positive diagnostics. `hasPredicateExit` applies `isCounterTest` only to `ForStatement`; apply the same counter-test exemption to the other loop forms. |
Fixed in be3c9f3d643: the counter-bound exemption now applies to for, while, and do-while alike, with rule tests for fixed-N while and do-while flushes.
I still see while/do-while falling through to !isTrueLiteral(loop.test); isCounterTest remains limited to ForStatement, and the new cases are not present. Could this commit be missing from the current PR head?
That re-review ran on 07fb63e, an intermediate head from the #22557 rebase. The pushed head 8ab4d34 carries the rewrite: isCounterTest accepts only a plain identifier compared with a number/identifier that the loop advances via ++/+=/-= (so events.length < 1 is reported), and for/while/do-while all share the same bounded-counter exemption, with rule tests for each.
| β¦ | β¦ | @@ -0,0 +1,178 @@ | |
| 120 | + | const expression = unwrapExpression(test); | |
| 121 | + | if (Option.isNone(expression) || expression.value.type !== "BinaryExpression") return false; | |
| 122 | + | if (!COUNTER_OPERATORS.has(expression.value.operator)) return false; | |
| 123 | + | return !someDescendant(expression.value, (node) => node.type === "CallExpression"); | |
π‘ Medium rules/no-polling-loop-in-tests.ts:123
A loop gated by events.length < 1 is silently accepted as a bounded counter loop, so this polling loop is not reported. isCounterTest only rejects comparisons containing calls and never checks that the compared variable is updated by the loop; verify that the test uses an induction variable before exempting it.
π Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @oxlint-plugin-t3code/rules/no-polling-loop-in-tests.ts around line 123: |
| 2 | |
| 3 | A loop gated by `events.length < 1` is silently accepted as a bounded counter loop, so this polling loop is not reported. `isCounterTest` only rejects comparisons containing calls and never checks that the compared variable is updated by the loop; verify that the test uses an induction variable before exempting it. |
Fixed in be3c9f3d643: a counter bound now requires a plain identifier compared to a numeric literal or identifier and advanced by ++/+=/-= in the body or for update, so events.length < 1 and never-advanced identifiers are reported (rule tests added).
I still see the previous isCounterTest implementation: it only rejects call expressions, so events.length < 1 remains exempted and no counter advancement is checked. Could you confirm the referenced commit is included in this PRβs current head?
That re-review ran on 07fb63e, an intermediate head from the #22557 rebase. The pushed head 8ab4d34 carries the rewrite: isCounterTest accepts only a plain identifier compared with a number/identifier that the loop advances via ++/+=/-= (so events.length < 1 is reported), and for/while/do-while all share the same bounded-counter exemption, with rule tests for each.
