diff --git a/docs/rules/template-no-positive-tabindex.md b/docs/rules/template-no-positive-tabindex.md
index dd5cb20cbe..78cb24729f 100644
--- a/docs/rules/template-no-positive-tabindex.md
+++ b/docs/rules/template-no-positive-tabindex.md
@@ -12,6 +12,8 @@
This rule prevents usage of any `tabindex` values other than `0` and `-1`. It does allow for dynamic values (choosing which value to show based on some condition / helper / etc), but only if that inline `if` condition has static `0`/`-1` as the value.
+In `.gts` files linted with type information (`parserOptions.project` or `parserOptions.projectService`), dynamic values such as `this.tabIndex`, `@tabIndex` or an in-scope variable are also allowed when their TypeScript type only contains numeric literals `<= 0` (and optionally `null`/`undefined`), e.g. `0 | -1`. Types like `number` are still reported, since they can't be proven safe.
+
This rule takes no arguments.
## Examples
@@ -26,6 +28,16 @@ This rule **allows** the following:
baz
```
+With type information:
+
+```gts
+import Component from '@glimmer/component';
+
+export default class Tab extends Component<{ Args: { tabIndex: 0 | -1 } }> {
+
baz
+}
+```
+
This rule **forbids** the following:
```hbs
diff --git a/lib/rules/template-no-positive-tabindex.js b/lib/rules/template-no-positive-tabindex.js
index 6764634a43..68fe84d5e8 100644
--- a/lib/rules/template-no-positive-tabindex.js
+++ b/lib/rules/template-no-positive-tabindex.js
@@ -1,91 +1,106 @@
-/**
- * Check a tabindex attribute value and return the violation type, if any.
- * Returns null if safe, 'positive' if the value is a positive integer,
- * or 'mustBeNegativeNumeric' if the value is non-numeric/dynamic/boolean.
- */
-function getTabindexViolation(attrValue) {
- if (!attrValue) {
- return null;
- }
+'use strict';
- // Handle simple text values like tabindex="0" or tabindex="-1"
- if (attrValue.type === 'GlimmerTextNode') {
- const value = Number.parseInt(attrValue.chars, 10);
- if (Number.isNaN(value)) {
- return 'mustBeNegativeNumeric';
- }
- return value > 0 ? 'positive' : null;
- }
+const { createGlimmerPathTypeResolver } = require('../utils/glimmer-path-type');
- // Handle mustache statements like tabindex={{-1}} or tabindex={{someProperty}}
- if (attrValue.type === 'GlimmerMustacheStatement') {
- const path = attrValue.path;
+// ts.TypeFlags values, hardcoded to avoid adding a direct `typescript`
+// dependency (see also template-no-deprecated).
+const TS_UNDEFINED_FLAG = 32_768;
+const TS_NULL_FLAG = 65_536;
- if (path.type === 'GlimmerNumberLiteral') {
- return Number.parseInt(path.original, 10) > 0 ? 'positive' : null;
- }
- if (path.type === 'GlimmerStringLiteral') {
- const value = Number.parseInt(path.original, 10);
- if (Number.isNaN(value)) {
- return 'mustBeNegativeNumeric';
- }
- return value > 0 ? 'positive' : null;
- }
-
- // Handle conditional expressions like {{if this.show -1 0}}
- if (
- path.type === 'GlimmerPathExpression' &&
- (path.original === 'if' || path.original === 'unless')
- ) {
- return getConditionalTabindexViolation(attrValue.params);
- }
-
- // Any other dynamic value (variable, boolean, etc.) is not verifiably safe
+function getNumericViolation(value) {
+ if (Number.isNaN(value)) {
return 'mustBeNegativeNumeric';
}
+ return value > 0 ? 'positive' : null;
+}
- // Handle concat statements like tabindex="{{-1}}" or tabindex="{{false}}"
- if (attrValue.type === 'GlimmerConcatStatement') {
- const parts = attrValue.parts || [];
- if (parts.length > 0 && parts[0].type === 'GlimmerMustacheStatement') {
- return getTabindexViolation(parts[0]);
+/**
+ * Every member of the type must be a numeric (string) literal <= 0, or
+ * null/undefined, e.g. `0 | -1`. null/undefined omits the attribute, or
+ * renders `tabindex=""` inside quotes, which browsers ignore (the same as a
+ * missing `{{if}}` branch).
+ */
+function getTypeViolation(type) {
+ let violation = null;
+ for (const member of type.isUnion() ? type.types : [type]) {
+ // eslint-disable-next-line no-bitwise
+ if (member.flags & (TS_UNDEFINED_FLAG | TS_NULL_FLAG)) {
+ continue;
}
- return 'mustBeNegativeNumeric';
+ const literal = member.isNumberLiteral() || member.isStringLiteral() ? member.value : '';
+ const memberViolation = getNumericViolation(Number.parseInt(literal, 10));
+ if (memberViolation === 'mustBeNegativeNumeric') {
+ return memberViolation;
+ }
+ violation ||= memberViolation;
}
-
- return 'mustBeNegativeNumeric';
+ return violation;
}
/**
- * Check that all branches of a conditional (if/unless) expression are safe.
+ * Check a single value expression: a literal, or a path whose TypeScript type
+ * proves it safe. Anything else is not verifiably safe.
*/
-function getConditionalTabindexViolation(params) {
- if (!params) {
- return 'mustBeNegativeNumeric';
+function getExpressionViolation(node, getPathType) {
+ switch (node.type) {
+ case 'GlimmerNumberLiteral':
+ case 'GlimmerStringLiteral': {
+ return getNumericViolation(Number.parseInt(node.original, 10));
+ }
+ case 'GlimmerPathExpression': {
+ const type = getPathType?.(node);
+ return type ? getTypeViolation(type) : 'mustBeNegativeNumeric';
+ }
+ default: {
+ return 'mustBeNegativeNumeric';
+ }
}
+}
- // Check the value branches (params[1] and optionally params[2])
- for (let i = 1; i < params.length && i < 3; i++) {
- const param = params[i];
- if (param.type === 'GlimmerNumberLiteral') {
- if (Number.parseInt(param.original, 10) > 0) {
- return 'positive';
- }
- } else if (param.type === 'GlimmerStringLiteral') {
- const val = Number.parseInt(param.original, 10);
- if (Number.isNaN(val)) {
- return 'mustBeNegativeNumeric';
- }
- if (val > 0) {
- return 'positive';
+/**
+ * Check a tabindex attribute value and return the violation type, if any.
+ * Returns null if safe, 'positive' if the value is a positive integer,
+ * or 'mustBeNegativeNumeric' if the value is non-numeric/dynamic/boolean.
+ */
+function getTabindexViolation(attrValue, getPathType) {
+ switch (attrValue.type) {
+ // tabindex="0"
+ case 'GlimmerTextNode': {
+ return getNumericViolation(Number.parseInt(attrValue.chars, 10));
+ }
+ // tabindex={{-1}}, tabindex={{this.tabIndex}}, tabindex={{if this.show -1 0}}
+ case 'GlimmerMustacheStatement': {
+ const { path, params, hash } = attrValue;
+ if (
+ path.type === 'GlimmerPathExpression' &&
+ (path.original === 'if' || path.original === 'unless')
+ ) {
+ // Every value branch must be safe
+ for (const branch of params.slice(1, 3)) {
+ const violation = getExpressionViolation(branch, getPathType);
+ if (violation) {
+ return violation;
+ }
+ }
+ return null;
}
- } else {
- // Dynamic value in branch — not verifiably safe
+ // A path with arguments is a helper call, which is not verifiably safe
+ return params.length > 0 || hash?.pairs.length > 0
+ ? 'mustBeNegativeNumeric'
+ : getExpressionViolation(path, getPathType);
+ }
+ // tabindex="{{-1}}". Multiple parts are concatenated (e.g. "{{-1}}5"
+ // renders "-15"), so only a single mustache is verifiably safe.
+ case 'GlimmerConcatStatement': {
+ const parts = attrValue.parts ?? [];
+ return parts.length === 1 && parts[0].type === 'GlimmerMustacheStatement'
+ ? getTabindexViolation(parts[0], getPathType)
+ : 'mustBeNegativeNumeric';
+ }
+ default: {
return 'mustBeNegativeNumeric';
}
}
-
- return null;
}
/** @type {import('eslint').Rule.RuleModule} */
@@ -113,7 +128,11 @@ module.exports = {
},
create(context) {
+ const typeResolver = createGlimmerPathTypeResolver(context);
+
return {
+ ...typeResolver?.visitors,
+
GlimmerElementNode(node) {
const tabindexAttr = node.attributes?.find((attr) => attr.name === 'tabindex');
@@ -121,7 +140,7 @@ module.exports = {
return;
}
- const violation = getTabindexViolation(tabindexAttr.value);
+ const violation = getTabindexViolation(tabindexAttr.value, typeResolver?.getPathType);
if (violation) {
context.report({
node: tabindexAttr,
diff --git a/lib/utils/glimmer-path-type.js b/lib/utils/glimmer-path-type.js
new file mode 100644
index 0000000000..dd8989f1c4
--- /dev/null
+++ b/lib/utils/glimmer-path-type.js
@@ -0,0 +1,90 @@
+'use strict';
+
+/**
+ * Resolve the TypeScript type of Glimmer path expressions (`this.foo.bar`,
+ * `@foo`, `foo`) in gjs/gts templates.
+ *
+ * Returns `null` when no type information is available. Otherwise returns
+ * `{ visitors, getPathType }`: `visitors` must be merged into the rule's
+ * visitors (they track the class that owns the current template), and
+ * `getPathType(path)` returns the path's `ts.Type`, or `undefined` when it
+ * can't be resolved (block params, unknown properties, template-only
+ * components, ...).
+ */
+function createGlimmerPathTypeResolver(context) {
+ const sourceCode = context.sourceCode;
+ const services = sourceCode.parserServices;
+ if (!services?.program || !services.esTreeNodeToTSNodeMap) {
+ return null;
+ }
+
+ const checker = services.program.getTypeChecker();
+ // The class whose body directly contains each open , or null
+ // for template-only components (including those nested inside a class,
+ // e.g. `static Inner = ...`), where `this` and `@args`
+ // don't refer to the enclosing class.
+ const templateClassStack = [];
+
+ function getClassInstanceType() {
+ const tsClass = services.esTreeNodeToTSNodeMap.get(templateClassStack.at(-1));
+ if (!tsClass) {
+ return undefined;
+ }
+ // Class declarations resolve to the instance type, class expressions to
+ // the constructor type.
+ const type = checker.getTypeAtLocation(tsClass);
+ return type.getConstructSignatures()[0]?.getReturnType() ?? type;
+ }
+
+ function getPropertyType(type, name) {
+ const symbol = type?.getProperty(name);
+ const location = services.esTreeNodeToTSNodeMap.get(
+ templateClassStack.at(-1) ?? sourceCode.ast
+ );
+ return symbol && checker.getTypeOfSymbolAtLocation(symbol, location);
+ }
+
+ function getHeadType(path) {
+ switch (path.head.type) {
+ case 'ThisHead': {
+ return getClassInstanceType();
+ }
+ case 'AtHead': {
+ const argsType = getPropertyType(getClassInstanceType(), 'args');
+ return getPropertyType(argsType, path.head.name.slice(1));
+ }
+ case 'VarHead': {
+ const ref = sourceCode
+ .getScope(path)
+ .references.find((reference) => reference.identifier === path.head);
+ const tsNode = services.esTreeNodeToTSNodeMap.get(ref?.resolved?.defs[0]?.name);
+ return tsNode && checker.getTypeAtLocation(tsNode);
+ }
+ default: {
+ return undefined;
+ }
+ }
+ }
+
+ function getPathType(path) {
+ try {
+ return path.tail.reduce(getPropertyType, getHeadType(path));
+ } catch {
+ return undefined;
+ }
+ }
+
+ return {
+ visitors: {
+ GlimmerTemplate(node) {
+ templateClassStack.push(node.parent?.type === 'ClassBody' ? node.parent.parent : null);
+ },
+ 'GlimmerTemplate:exit'() {
+ templateClassStack.pop();
+ },
+ },
+ getPathType,
+ };
+}
+
+module.exports = { createGlimmerPathTypeResolver };
diff --git a/tests/lib/rules-preprocessor/template-no-positive-tabindex/component-stub.ts b/tests/lib/rules-preprocessor/template-no-positive-tabindex/component-stub.ts
new file mode 100644
index 0000000000..e926929a71
--- /dev/null
+++ b/tests/lib/rules-preprocessor/template-no-positive-tabindex/component-stub.ts
@@ -0,0 +1,3 @@
+export default class ComponentBase {
+ declare args: S['Args'];
+}
diff --git a/tests/lib/rules-preprocessor/template-no-positive-tabindex/tabindex.ts b/tests/lib/rules-preprocessor/template-no-positive-tabindex/tabindex.ts
new file mode 100644
index 0000000000..7234d16d5b
--- /dev/null
+++ b/tests/lib/rules-preprocessor/template-no-positive-tabindex/tabindex.ts
@@ -0,0 +1,2 @@
+export const safeTabindex: 0 | -1 = 0;
+export const positiveTabindex: 0 | 1 = 0;
diff --git a/tests/lib/rules-preprocessor/template-no-positive-tabindex/usage.gts b/tests/lib/rules-preprocessor/template-no-positive-tabindex/usage.gts
new file mode 100644
index 0000000000..2df5fd9f47
--- /dev/null
+++ b/tests/lib/rules-preprocessor/template-no-positive-tabindex/usage.gts
@@ -0,0 +1,2 @@
+// Placeholder file — actual code is provided inline by tests.
+// Its presence lets TypeScript include this path in the program.
diff --git a/tests/lib/rules/template-no-positive-tabindex.js b/tests/lib/rules/template-no-positive-tabindex.js
index 560e8f8ac8..7f0c15a417 100644
--- a/tests/lib/rules/template-no-positive-tabindex.js
+++ b/tests/lib/rules/template-no-positive-tabindex.js
@@ -91,6 +91,16 @@ ruleTester.run('template-no-positive-tabindex', rule, {
output: null,
errors: [{ message: 'Avoid positive integer values for tabindex.' }],
},
+ {
+ code: '',
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ code: '',
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
],
});
@@ -175,5 +185,192 @@ hbsRuleTester.run('template-no-positive-tabindex', rule, {
output: null,
errors: [{ message: 'Avoid positive integer values for tabindex.' }],
},
+ {
+ code: '',
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ ],
+});
+
+// Type-aware checking of dynamic values. The filename must physically exist
+// so the tsconfig includes it in the TypeScript program.
+
+const path = require('node:path');
+
+const PREPROCESSOR_DIR = path.join(__dirname, '../rules-preprocessor');
+const FIXTURE = path.join(PREPROCESSOR_DIR, 'template-no-positive-tabindex/usage.gts');
+
+const ruleTesterTyped = new RuleTester({
+ parser: require.resolve('ember-eslint-parser'),
+ parserOptions: {
+ project: path.join(PREPROCESSOR_DIR, 'tsconfig.eslint.json'),
+ tsconfigRootDir: PREPROCESSOR_DIR,
+ ecmaVersion: 2022,
+ sourceType: 'module',
+ extraFileExtensions: ['.gts'],
+ },
+});
+
+ruleTesterTyped.run('template-no-positive-tabindex (with TS project)', rule, {
+ valid: [
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex: 0 | -1 = 0;
+
+}`,
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ get tabIndex(): -1 | '0' | undefined { return undefined; }
+
+}`,
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ state = { tabIndex: -1 as const };
+
+}`,
+ },
+ {
+ filename: FIXTURE,
+ code: `import ComponentBase from './component-stub';
+export default class Foo extends ComponentBase<{ Args: { tabIndex: 0 | -1 } }> {
+
+}`,
+ },
+ {
+ filename: FIXTURE,
+ code: `import { safeTabindex } from './tabindex';
+`,
+ },
+ {
+ filename: FIXTURE,
+ code: `export const Foo = class {
+ tabIndex: 0 | -1 = 0;
+
+};`,
+ },
+ {
+ filename: FIXTURE,
+ code: `const tabIndex = -1;
+`,
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = -1 as const;
+
+}`,
+ },
+ ],
+ invalid: [
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = 0;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex: 0 | 1 = 0;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'positive' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex: 0 | -1 | boolean = 0;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `import ComponentBase from './component-stub';
+export default class Foo extends ComponentBase<{ Args: { tabIndex: number } }> {
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `import { positiveTabindex } from './tabindex';
+`,
+ output: null,
+ errors: [{ messageId: 'positive' }],
+ },
+ {
+ filename: FIXTURE,
+ code: '{{#each this.items as |item|}}{{/each}}',
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = 2 as const;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'positive' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = -1 as const;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = -1 as const;
+
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ // `this` in a nested template-only component is not the enclosing class
+ filename: FIXTURE,
+ code: `export default class Foo {
+ tabIndex = -1 as const;
+ get Inner() { return ; }
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
+ {
+ // `@args` in a nested template-only component are not the enclosing class's args
+ filename: FIXTURE,
+ code: `import ComponentBase from './component-stub';
+export default class Foo extends ComponentBase<{ Args: { tabIndex: 0 | -1 } }> {
+ static Inner = ;
+}`,
+ output: null,
+ errors: [{ messageId: 'mustBeNegativeNumeric' }],
+ },
],
});