Skip to content

Extract rule: template-quotes#2598

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

Extract rule: template-quotes#2598
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-quotes

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-quotes (PR #2598)

Comparison with ember-template-lint quotes.js

General correctness:

  1. Schema matches original config: The original accepts 'double', 'single', or { curlies, html } object. The PR's schema correctly models this with oneOf: [enum, object]. Good.

  2. AttrNode handling: The original uses node.quoteType === badChars.html to detect wrong quotes, which relies on the AST providing a quoteType property. The PR instead extracts the raw source text and looks for the quote character after =. This is a reasonable workaround since the Glimmer ESLint AST may not expose quoteType. However, potential issue: if an attribute value is a MustacheStatement (e.g., class={{something}}), the raw text approach could behave unexpectedly. The PR doesn't explicitly filter these out — it relies on afterEq !== badChars.html to skip them (since { is not a quote char). This should work in practice.

  3. Fix approach: The original uses node.quoteType = goodChars.html (direct AST mutation via ember-template-recast). The PR uses fixer.replaceText on the raw source. This is the correct ESLint approach. The fix looks correct — it replaces the outer quotes while preserving the inner content.

  4. StringLiteral handling: Correctly checks raw[0] (first char of source text) against the bad quote char. Fix replaces outer quotes. Matches original logic.

  5. Missing gjs-specific invalid tests: The ruleTester (gjs mode) has valid tests but invalid: [] is empty. All invalid tests are only in the hbs tester. Consider adding at least one or two invalid gjs test cases for completeness, e.g., <template><div class='foo'></div></template> with the default "double" config.

Scope analysis (gjs/gts):

This rule is purely about quote style — it checks GlimmerAttrNode and GlimmerStringLiteral for syntactic/stylistic properties (which quote character is used). No helper/component name matching involved. No scope analysis needed.

Overall: Good migration. The main suggestion is to add some invalid gjs test cases for better coverage.

🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-quotes branch from 974002c to 164ac58 Compare March 20, 2026 21:16
@NullVoxPopuli NullVoxPopuli merged commit 6c80cb5 into ember-cli:master Mar 20, 2026
9 checks passed
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