Skip to content

Extract rule: template-style-concatenation#2622

Merged
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-style-concatenation
Mar 19, 2026
Merged

Extract rule: template-style-concatenation#2622
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-style-concatenation

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

Split from #2371.

Copy link
Copy Markdown

@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-style-concatenation

Compared against ember-template-lint style-concatenation.js.

General Correctness

  1. Faithful port: The logic correctly mirrors the original. The original checks ElementNode for a style attribute whose value is either a ConcatStatement or a MustacheStatement with a concat helper path. The ESLint port does the same with the appropriate Glimmer-prefixed AST node types. Good.

  2. Error message difference: The original uses 'Concatenated styles must be marked as \htmlSafe`.'but the ESLint port uses'Avoid string concatenation in style attributes. Use a computed property with htmlSafe instead.'. This is a meaningful difference in messaging. The original's message is more specific and actionable (telling users to use htmlSafe`). The port's message is more prescriptive (suggesting computed properties). Consider aligning with the original message for consistency, or at least noting the intentional change.

  3. Attribute finding approach: The original uses AstNodeInfo.findAttribute(node, 'style') while the port uses node.attributes?.find((a) => a.name === 'style'). These are functionally equivalent. Good.

  4. Tests: Good coverage for both gjs and hbs modes. The test cases cover ConcatStatement (interpolated strings like style="color: {{this.color}}") and MustacheStatement with concat helper. Valid cases correctly include html-safe wrapping around concat.

Scope Analysis (gjs/gts)

This rule checks path.original === 'concat' to detect the concat helper used in style attributes. In gjs/gts, concat could theoretically be shadowed by a local import. However, the concat helper is a very common Ember built-in, and the rule is specifically checking for it within a style attribute's mustache value -- a very specific context. The risk of false positives from shadowing is minimal. That said, for completeness, scope analysis could verify that concat refers to the built-in, but this is low priority.


🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli merged commit 3b5e6b9 into ember-cli:master Mar 19, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-style-concatenation branch March 19, 2026 17:54
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