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

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:
 <div role='tab' tabindex={{if this.isHidden '-1' '0'}}>baz</div>
 ```
 
+With type information:
+
+```gts
+import Component from '@glimmer/component';
+
+export default class Tab extends Component<{ Args: { tabIndex: 0 | -1 } }> {
+  <template><div role='tab' tabindex={{@tabIndex}}>baz</div></template>
+}
+```
+
 This rule **forbids** the following:
 
 ```hbs

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,15 +128,19 @@ module.exports = {
   },
 
   create(context) {
+    const typeResolver = createGlimmerPathTypeResolver(context);
+
     return {
+      ...typeResolver?.visitors,
+
       GlimmerElementNode(node) {
         const tabindexAttr = node.attributes?.find((attr) => attr.name === 'tabindex');
 
         if (!tabindexAttr || !tabindexAttr.value) {
           return;
         }
 
-        const violation = getTabindexViolation(tabindexAttr.value);
+        const violation = getTabindexViolation(tabindexAttr.value, typeResolver?.getPathType);
         if (violation) {
           context.report({
             node: tabindexAttr,

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 <template>, or null
+  // for template-only components (including those nested inside a class,
+  // e.g. `static Inner = <template>...</template>`), 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 };

tests/lib/rules-preprocessor/template-no-positive-tabindex/component-stub.ts

@@ -0,0 +1,3 @@
+export default class ComponentBase<S extends { Args?: object } = object> {
+  declare args: S['Args'];
+}

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;

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.