Skip to content

Extract rule: template-no-whitespace-within-word#2593

Open
NullVoxPopuli wants to merge 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-whitespace-within-word
Open

Extract rule: template-no-whitespace-within-word#2593
NullVoxPopuli wants to merge 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-whitespace-within-word

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-no-whitespace-within-word

Compared against the original ember-template-lint rule no-whitespace-within-word.

What's done well

  • The WHITESPACE_ENTITY_LIST is identical to the original -- all 52 entries match exactly.
  • The regex construction logic is faithfully ported. The 5-alternation pattern (whitespace)(char)(whitespace)(char)(whitespace) is preserved, including the comment explaining why 5 alternations avoids false positives.
  • Parent node filtering for AttrNode and <style> elements is correctly implemented, matching the original's parents.some(...) checks.
  • All original test cases are ported -- both good and bad, including the HTML attribute ignore case (enable-background="a b c d e f g h i j k l m"), the <style> element case, and the &nbsp;/&emsp; entity cases.
  • Both gjs and hbs parser modes are tested.
  • Error message matches exactly: "Excess whitespace in layout detected."

Observations and potential issues

  1. Parent traversal approach differs. The original uses path.parents() (provided by the ember-template-lint traversal API) to walk up the tree. The new rule uses a while (parent) loop on node.parent. This is the correct ESLint equivalent and should work, assuming the Glimmer AST nodes have parent references set (which they should in ember-eslint-parser).

  2. Uses context.getSourceCode() (deprecated in ESLint 9+). Same note as PR #2592 -- consider using const sourceCode = context.sourceCode || context.getSourceCode(); for forward compatibility.

  3. meta.type is set to 'layout'. Note that ESLint deprecated the 'layout' type in ESLint 9 in favor of other types. This isn't an immediate issue but worth being aware of for future ESLint version compatibility. The original rule doesn't have an ESLint-style meta.type so there's no direct comparison, but 'suggestion' might be more future-proof.

  4. The regex is compiled at module level (as a constant WHITESPACE_WITHIN_WORD_REGEX) rather than in the constructor as in the original. This is actually better -- it avoids recompiling the regex on every file. Good optimization.

  5. CHARACTER_REGEX only matches ASCII letters [a-zA-Z]. This matches the original, but it means spaced-out Unicode words (e.g., accented characters) would not be caught. This is a pre-existing limitation, not introduced by this port.

Summary

Excellent port. The implementation is arguably slightly improved over the original (module-level regex compilation). All test cases are covered, error messages match, and the parent-node filtering logic is correctly adapted to the ESLint API.


Automated review comparing with ember-template-lint source

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-whitespace-within-word branch from f7c847f to 2f764a1 Compare March 21, 2026 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants