Extract rule: template-no-valueless-arguments#2591
Conversation
NullVoxPopuli-ai-agent
left a comment
There was a problem hiding this comment.
Review: template-no-valueless-arguments
Compared against the original ember-template-lint rule no-valueless-arguments.
What's done well
- Clean, minimal implementation. The rule is simple and the port keeps it that way -- just 35 lines of rule code.
- Correct detection approach. Checking for
@prefix on the attribute name and then verifying no=in the source text is a pragmatic workaround since the ESLint Glimmer AST may not expose anisValuelessproperty directly. - All original test cases are ported -- both good and bad cases match the original exactly.
- Both gjs and hbs parser modes are tested.
Observations and potential issues
-
Detection mechanism differs from the original. The original rule relies on
node.isValueless-- a boolean property provided by the Handlebars AST. The new rule checks!text.includes('=')on the source text. This is a reasonable heuristic but could theoretically produce false positives if an attribute name itself contains=(extremely unlikely for@-prefixed names) or false negatives in edge cases where the AST representation differs from source. In practice this should be fine, but it's worth noting the approach is source-text-based rather than AST-property-based. -
context.sourceCode || context.getSourceCode()-- the fallback togetSourceCode()is good for backwards compatibility with older ESLint versions. However,getSourceCode()is deprecated in ESLint 9+. Since this plugin likely targets a range of ESLint versions, the||pattern is appropriate. -
No
fixableproperty in meta. The original also doesn't provide autofix, so this is consistent. -
The error message matches exactly:
"Named arguments should have an explicitly assigned value."-- identical to the original. -
Missing a
Foo @bar={{true}}gjs valid test. Wait, actually it is there. The test coverage looks complete.
Summary
Straightforward, clean port. The source-text heuristic for valueless detection is the only notable divergence from the original's approach, and it works correctly for all test cases. This is a simple rule and the migration handles it well.
Automated review comparing with ember-template-lint source
838cda0 to
9b5f84a
Compare
Split from #2371.