Extract rule: template-no-unknown-arguments-for-builtin-components#2583
Conversation
NullVoxPopuli-ai-agent
left a comment
There was a problem hiding this comment.
Review: template-no-unknown-arguments-for-builtin-components (comparing with ember-template-lint no-unknown-arguments-for-builtin-components)
Coverage
This is the most complex rule in this batch. The migration covers the core detection logic well.
What's done well
- Complete
KnownArgumentsdata: The argument maps forLinkTo,Input, andTextareaare identical to the original -- all deprecated arguments, deprecated events, conflicts, and required argument lists are faithfully reproduced. - Error message functions (
deprecateArgument,deprecateEvent) produce the same messages as the original. - Conflict detection is correctly implemented.
- Comprehensive tests: Both gjs and hbs modes tested with a good variety of cases (typos, deprecated args, deprecated events, conflicts, unknown args).
templateMode: 'both'is appropriate since<Input>,<Textarea>, and<LinkTo>exist in both modes.
Items needing attention
-
Fuzzy matching differs from original: The original uses
fuse.jsfor fuzzy matching, while this PR implements a customfuzzyMatchfunction. This means suggestion quality may differ. For example, in the HBS test,@randomsuggests@dragEnd-- the original with Fuse.js may produce a different suggestion. This is a pragmatic tradeoff (avoiding a new dependency), but worth documenting. -
Missing
requiredargument validation: The original rule checksnodeMeta.requiredand reports when none of the required argument variants are present (e.g.,LinkTorequires at least one of@route,@query,@model, or@models). Therequireddata is present inKnownArgumentsbut thecreate()function does not check it. This means cases like<LinkTo />(with no route-related arguments) will not produce an error. The original has aREQUIRED_MESSAGEfunction and corresponding logic. -
Missing fixability: The original rule supports auto-fixing deprecated arguments (converting
@elementIdtoid, converting deprecated events to{{on}}modifiers). The ESLint version hasfixable: null, so no auto-fix is provided. This is understandable given the complexity of AST manipulation in the ESLint context, but worth noting as a feature gap. -
Conflict message wording difference: The original uses
"only one should exists"(with typo). The PR uses"only one should exist."(corrected). This is actually an improvement, but test assertions should match.
Verdict
Good migration overall, especially given the complexity. The missing required argument check is the most significant gap -- it means <LinkTo /> without any route/query/model args won't be flagged. Consider adding that check or documenting it as a known limitation.
🤖 Automated review comparing with ember-template-lint source
…mports In gjs/gts files, when a developer imports a custom component with a name that matches an Ember built-in (Input, Textarea, LinkTo), the rule would incorrectly flag unknown arguments. This checks scope analysis to determine if the tag name resolves to a JS-scope variable with a definition (e.g. an import binding), and skips validation in that case. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
The rule already had logic to check that required arguments are present (e.g., LinkTo requires at least one of @route, @query, @model, or @models), but this code path had no test coverage. Add test cases for both GJS and HBS parsers to verify that <LinkTo /> without any required argument correctly reports a requiredArgument error. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
bd3f119 to
40bdae0
Compare
Split from #2371.