Skip to content

fix(template-no-positive-tabindex): allow dynamic values whose type is provably safe - #2850

Merged
NullVoxPopuli merged 2 commits into
ember-cli:masterfrom
BoussonKarel:fix/template-no-positive-tabindex-types
Oct 7, 2026
Merged

NullVoxPopuli merged 2 commits into
ember-cli:masterfrom
BoussonKarel:fix/template-no-positive-tabindex-types

Conversation

@BoussonKarel

Copy link
Copy Markdown
Contributor

Fixes #2805

Problem

ember/template-no-positive-tabindex reports every dynamic tabindex value, 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 (and this.foo.bar): property type on the enclosing class instance
  • @foo: property type on the enclosing class's args
  • foo: 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, or null/undefined. A type with a positive literal (e.g. 0 | 1) is reported as positive. Anything that can't be resolved or proven safe (number, boolean, block params, unknown properties, helper calls) keeps the existing mustBeNegativeNumeric error. This applies to tabindex={{…}}, tabindex="{{…}}" and the branches of {{if}}/{{unless}}.

Without type information the rule behaves exactly as before.

Tests

Added a typed RuleTester block, with fixtures in tests/lib/rules-preprocessor/template-no-positive-tabindex/, that covers class fields, getters, nested properties, args, imports, local consts, class expressions and if branches, in both valid and invalid cases. Docs updated.

🤖 Generated with Claude Code

tsNode: tsClass,
};
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not using vinitors to get this information?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 NullVoxPopuli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@BoussonKarel
BoussonKarel force-pushed the fix/template-no-positive-tabindex-types branch from c865d9b to 86ab88f Compare October 6, 2026 13:07
@BoussonKarel

Copy link
Copy Markdown
Contributor Author

Reworked the implementation (squashed into 86ab88f): the TypeScript resolution of Glimmer paths now lives in a reusable lib/utils/glimmer-path-type.js (enclosing class tracked via visitors), and the rule itself is reduced to literal/type checks, with one shared helper for mustache values and if/unless branches. If there's a specific pattern you'd prefer, e.g. how other typed rules here are structured, point me at it and I'll align.

— 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>
@NullVoxPopuli
NullVoxPopuli merged commit dc1f8de into ember-cli:master Oct 7, 2026
10 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ember/template-no-positive-tabindex errors with dynamic value that is of type 0 | -1

2 participants