Repository navigation
fix(template-no-positive-tabindex): allow dynamic values whose type is provably safe - #2850
Conversation
| tsNode: tsClass, | ||
| }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Why are we not using vinitors to get this information?
There was a problem hiding this comment.
Good call — switched to ClassDeclaration/ClassExpression enter/exit visitors with a class stack instead of walking node.parent (d1b6883). Happy to restructure further if you had other parts of the implementation in mind.
— Claude
NullVoxPopuli
left a comment
There was a problem hiding this comment.
Thatks for submitting this! Tests look good, but i have concerns over the style of implementation in the rule itself
…s provably safe When type information is available, resolve `this.foo`, `@foo` and in-scope variables used as tabindex values (directly, in concat, or in if/unless branches) and accept them when every member of their type is a numeric literal <= 0 or null/undefined, e.g. `0 | -1`. The TypeScript resolution of Glimmer paths lives in lib/utils/glimmer-path-type.js so other rules can reuse it. Fixes ember-cli#2805 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
c865d9b to
86ab88f
Compare
|
Reworked the implementation (squashed into 86ab88f): the TypeScript resolution of Glimmer paths now lives in a reusable — Claude |
- Resolve `this`/`@args` only for templates directly in a class body;
nested template-only components (`static Inner = <template>`) no longer
borrow the enclosing class's types.
- Treat a mustache with named arguments (`{{this.x foo=1}}`) as a helper
call, like positional arguments.
- Report concatenated values with more than one part (`"{{-1}}5"`), which
render as a different number than the first part.
- Correct the comment on null/undefined: inside quotes it renders
`tabindex=""`, which browsers ignore.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fixes #2805
Problem
ember/template-no-positive-tabindexreports every dynamictabindexvalue, even when the value is typed so it can only be safe (e.g.0 | -1).Change
When the file is linted with type information (
parserOptions.project/projectService), dynamic path expressions are resolved with the TypeScript checker:this.foo(andthis.foo.bar): property type on the enclosing class instance@foo: property type on the enclosing class'sargsfoo: an in-scope JS binding (module const, import, …)The value is accepted when every member of its type is a numeric literal (or numeric string literal)
<= 0, ornull/undefined. A type with a positive literal (e.g.0 | 1) is reported aspositive. Anything that can't be resolved or proven safe (number,boolean, block params, unknown properties, helper calls) keeps the existingmustBeNegativeNumericerror. This applies totabindex={{…}},tabindex="{{…}}"and the branches of{{if}}/{{unless}}.Without type information the rule behaves exactly as before.
Tests
Added a typed
RuleTesterblock, with fixtures intests/lib/rules-preprocessor/template-no-positive-tabindex/, that covers class fields, getters, nested properties, args, imports, local consts, class expressions andifbranches, in both valid and invalid cases. Docs updated.🤖 Generated with Claude Code