Skip to content

Extract rule: template-no-unnecessary-curly-parens#2587

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-unnecessary-curly-parens
Mar 21, 2026
Merged

Extract rule: template-no-unnecessary-curly-parens#2587
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-unnecessary-curly-parens

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-no-unnecessary-curly-parens (comparing with ember-template-lint no-unnecessary-curly-parens)

Coverage

Good migration that captures the core behavior of the original rule.

What's done well

  • Correct detection logic: isFixableMustache checks that node.path is a GlimmerSubExpression with params or hash pairs, matching the original's check.
  • Auto-fix: Correctly reconstructs the mustache without the unnecessary parentheses.
  • Good test coverage: Tests cover helpers with positional params, named params (hash), and the important edge case of parenthesized expressions inside ConcatStatement (attribute interpolations like "{{index}}X{{(someHelper foo)}}").
  • Both parser modes tested: gjs and hbs test sections included.

Items needing attention

  1. Missing ConcatStatement visitor: The original rule has a ConcatStatement visitor that, in fix mode, iterates over node.parts and fixes any MustacheStatement parts that have unnecessary parens. The ESLint version only has a GlimmerMustacheStatement visitor. This might still work if the ESLint parser emits GlimmerMustacheStatement nodes inside concat statements (and the tests for <FooBar @x="{{index}}X{{(someHelper foo)}}" /> do seem to pass), but it's worth verifying that the fix works correctly for these cases. The fact that the tests include these cases and pass is reassuring.

  2. {{(foo)}}/{{(this.helper)}} valid cases: These are correctly marked as valid -- when there are no params or hash pairs, the parens may serve a purpose (invoking a helper that returns a value). This matches the original.

  3. No GlimmerBlockStatement handling: The original only handles MustacheStatement (and ConcatStatement), so omitting block statement handling is correct and matches the original scope.

Verdict

Clean migration. The ConcatStatement visitor difference is worth noting, but the tests demonstrate the fix works in the concatenation context. No blocking issues.


🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-unnecessary-curly-parens branch from 821a80f to d1299c4 Compare March 21, 2026 02:04
@NullVoxPopuli NullVoxPopuli merged commit 3142567 into ember-cli:master Mar 21, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-no-unnecessary-curly-parens branch March 21, 2026 02:18
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