Skip to content

Extract rule: template-require-splattributes#2612

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-require-splattributes
Mar 20, 2026
Merged

Extract rule: template-require-splattributes#2612
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-require-splattributes

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-splattributes (PR #2612)

Comparison with ember-template-lint source

General correctness:

  1. Missing case for non-element-only templates: The original ETL rule reports on ALL templates that lack splattributes — including templates with no element nodes at all (e.g., {{yield}}). It has two branches: single root element → report on element, otherwise → report on template node. The ESLint version adds an extra gate: elementNodes.length > 1 for the multi-element path, but if elementNodes.length === 0 (template has only text/mustache nodes), no error is reported. The original ETL would still report "At least one element in this template should use ...attributes". This is a behavioral gap — templates like <template>{{yield}}</template> or <template>Hello world</template> would not be flagged in the ESLint version but would be flagged by the original.

  2. Single element with significant text nodes: The ESLint version has a special check hasOnlyOneElement that requires significantTextNodes.length === 0. So <template>Hello <div></div></template> would NOT get the "root element" message and would also NOT get the "at least one element" message (since elementNodes.length is 1, not > 1). The original ETL treats this as elementNodes.length === 1 && nonEmptyTextNodes.length === 0 failing, so it falls through to the else branch and reports on the template node. Bug: The ESLint version silently passes for templates with one element + text content.

  3. messages is empty object: The rule uses inline message strings in context.report() rather than messageId. This works but is inconsistent with best practices. Consider using messageId with the messages object.

  4. Deep traversal for splattributes: The ESLint version does a manual recursive checkNode traversal. The original ETL uses AttrNode visitor which fires for all attribute nodes in the tree. Both achieve the same result — checking all descendants.

Scope analysis (gjs/gts):

Not applicable. This rule checks for ...attributes spread syntax and element structure — no helper/component name matching.

Tests: Missing test cases for templates with no elements (only mustache/text), which is where the behavioral gap exists. Consider adding <template>{{yield}}</template> as an invalid case to match ETL behavior.

🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-require-splattributes branch from 67e2360 to 63695f3 Compare March 20, 2026 00:00
@NullVoxPopuli NullVoxPopuli merged commit 97c7d56 into ember-cli:master Mar 20, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-require-splattributes branch March 20, 2026 02:43
@github-actions github-actions bot mentioned this pull request Mar 20, 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