Skip to content

Extract rule: template-no-potential-path-strings#2565

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-potential-path-strings
Mar 21, 2026
Merged

Extract rule: template-no-potential-path-strings#2565
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-potential-path-strings

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-potential-path-strings

Compared against the original ember-template-lint rule no-potential-path-strings.

What looks good

  • The attribute-value detection correctly handles both this. and @ prefixes.
  • The FINE_SYMBOLS exclusion from the original (|, /, \) is implemented via the regex ^@[\w-]+$ (which implicitly excludes /, \, |).
  • Test cases match the original test suite for attribute-level detection.
  • Both .gjs and .hbs parsers are tested.
  • originallyFrom metadata is present.

Items worth attention

  1. The GlimmerTextNode handler is NOT in the original rule: The original no-potential-path-strings rule only checks AttrNode values. It does not check text content within elements. The ESLint implementation adds a GlimmerTextNode handler that flags text content like <div>foo.bar</div> or <div>this.propertyName</div>. This is a significant scope expansion beyond the original rule. It could cause many false positives -- for example, <p>See example.com for details</p> or <code>object.property</code> would be flagged. The pattern /\b(this\.\w+|\w+\.\w+)\b/ is very broad.

  2. Error message differs from original: The original generates a helpful, specific message: "Potential path in attribute string detected. Did you mean {{<path>}}?" with the actual path name. The ESLint version uses a generic message: "Potential path string detected. Use dynamic values instead of path strings." The original's message is more actionable because it shows the suggested fix.

  3. @ prefix handling difference: The original checks chars.startsWith('@') and then filters out strings containing |, /, \. The ESLint version uses ^@[\w-]+$ which is functionally similar but subtly different -- it requires the entire string to match @ followed by word chars and hyphens. The original would also flag @foo.bar (no special symbols), while the regex ^@[\w-]+$ would not flag @foo.bar because . is not in [\w-]. This means the ESLint rule is slightly less strict for @-prefixed paths containing dots.

  4. Missing this. prefix with special symbols check: The original checks chars.startsWith('this.') but the ESLint rule uses ^this\.\w+ which does not apply the FINE_SYMBOLS exclusion. However, paths starting with this. containing |, /, \ are unlikely in practice, so this is minor.

Suggestions

  • Consider removing or gating the GlimmerTextNode handler behind an option, since it's a scope expansion that could cause false positives.
  • Consider making the error message include the detected path, as the original does.
  • Document the text-content detection as an enhancement if it's intentional.

The attribute-level detection is a faithful port. The text-content detection is the main concern.

🤖 Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-potential-path-strings branch from 5929f00 to 8f5a5e7 Compare March 21, 2026 16:50
@NullVoxPopuli NullVoxPopuli merged commit 87cbd47 into ember-cli:master Mar 21, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-no-potential-path-strings branch March 21, 2026 17:51
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