Skip to content

Extract rule: template-require-valid-alt-text#2614

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-require-valid-alt-text
Mar 19, 2026
Merged

Extract rule: template-require-valid-alt-text#2614
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-require-valid-alt-text

Conversation

@NullVoxPopuli
Copy link
Contributor

Split from #2371.

Copy link

@NullVoxPopuli-ai-agent NullVoxPopuli-ai-agent left a comment

Choose a reason for hiding this comment

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

Review: template-require-valid-alt-text (PR #2614)

Comparison with ember-template-lint source

General correctness:

  1. Error message differences (minor): Several messages have slightly different punctuation:

    • ETL: "All <img> tags must have an alt attribute" → ESLint: adds period at end
    • ETL: "The alt text must not be the same as the image source" → ESLint: adds period
    • ETL: "A number is not valid alt text" → ESLint: adds period
    • ETL: "The alt attribute should be empty if <img> has role of none or presentation" → ESLint: wraps in backticks and quotes differently
      These are cosmetic but will cause test failures if anyone is matching exact messages from the original.
  2. imgRedundant reported on altAttr node, not node: In the ESLint version, the redundant words error reports on altAttr (the attribute node) instead of node (the element node). The original ETL always reports on the element node. This is actually a nice improvement for editor UX (highlights just the attribute), but it's a behavioral difference.

  3. img role=presentation check order: The ESLint version checks altAttr && roleAttr first, then checks !altAttr. The original checks role+alt validation AFTER the main alt presence check. But in the ESLint version, if both altAttr and roleAttr exist and it's role=presentation with non-empty alt, it reports imgRolePresentation but does NOT return — it continues to also check for redundant words, numeric alt, etc. The original also does this (the role check is before the if (!hasAltAttribute) check and doesn't return). So both can report multiple errors for the same element, which matches.

  4. hasAccessibleChild for <object>: The original ETL uses AstNodeInfo.hasChildren(node) which likely checks for any child nodes (including text nodes with content). The ESLint version's hasChildren() function filters text nodes by chars.trim().length > 0, which is equivalent.

  5. Missing <object> title attribute behavior: The original checks roleValue directly against ['presentation', 'none'] — and roleValue could be a MustacheStatement node object if dynamic, in which case includes() would return false. The ESLint version uses getTextValue(roleAttr) which returns undefined for non-TextNode types, and 'presentation' || 'none'].includes(undefined) is false. This matches.

Scope analysis (gjs/gts):

Not applicable. This rule only checks HTML element tags (img, input, object, area) and their attributes. No helper/component name resolution is involved.

Tests: Comprehensive test coverage matching the original ETL test cases. Both gjs and hbs modes tested. Good.

Summary: Solid migration with good fidelity to the original. Minor message punctuation differences. No blocking issues.

🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-require-valid-alt-text branch from 3512c6f to c34ec5d Compare March 19, 2026 23:04
@NullVoxPopuli NullVoxPopuli merged commit 6da1ed7 into ember-cli:master Mar 19, 2026
11 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-require-valid-alt-text branch March 19, 2026 23:26
@github-actions github-actions bot mentioned this pull request Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants