Skip to content

Commit 7da211d

Browse files
johanrdjohanrd
committed
Fix template-no-unbound false positive in GJS/GTS
`unbound` is an ambient strict-mode keyword in Glimmer/Ember (registered in STRICT_MODE_KEYWORDS, backed by BUILTIN_KEYWORD_HELPERS.unbound), so `{{unbound foo}}` works in .gjs/.gts templates without an import. The rule should still flag those uses everywhere — but skip cases where `unbound` resolves to a JS import/const or a template block param. ember-eslint-parser registers template block params in scope, so a single sourceCode.getScope walk covers both JS bindings and block params. Also updates templateMode from 'loose' to 'both' to reflect that the rule now runs in strict mode.
1 parent 56f913d commit 7da211d

File tree

8 files changed

+279
-364
lines changed

8 files changed

+279
-364
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ rules in templates can be disabled with eslint directives with mustache or html
283283
| [template-require-valid-named-block-naming-format](docs/rules/template-require-valid-named-block-naming-format.md) | require valid named block naming format | | 🔧 | |
284284
| [template-self-closing-void-elements](docs/rules/template-self-closing-void-elements.md) | require self-closing on void elements | | 🔧 | |
285285
| [template-simple-modifiers](docs/rules/template-simple-modifiers.md) | require simple modifier syntax | | | |
286-
| [template-simple-unless](docs/rules/template-simple-unless.md) | require simple conditions in unless blocks | | 🔧 | |
286+
| [template-simple-unless](docs/rules/template-simple-unless.md) | require simple conditions in unless blocks | | | |
287287
| [template-sort-invocations](docs/rules/template-sort-invocations.md) | require sorted attributes and modifiers | | 🔧 | |
288288
| [template-splat-attributes-only](docs/rules/template-splat-attributes-only.md) | disallow ...spread other than ...attributes | | | |
289289
| [template-style-concatenation](docs/rules/template-style-concatenation.md) | disallow string concatenation in inline styles | | | |

‎docs/rules/template-no-unbound.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# ember/template-no-unbound
22

3-
> **HBS Only**: This rule applies to classic `.hbs` template files only (loose mode). It is not relevant for `gjs`/`gts` files (strict mode), where these patterns cannot occur.
4-
53
<!-- end auto-generated rule header -->
64

75
`{{unbound}}` is a legacy hold over from the days in which Ember's template engine was less performant. Its use today

‎docs/rules/template-simple-unless.md‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# ember/template-simple-unless
22

3-
🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
4-
53
<!-- end auto-generated rule header -->
64

75
Require simple conditions in `{{#unless}}` blocks. Complex expressions should use `{{#if}}` with negation instead.

‎lib/rules/template-no-unbound.js‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module.exports = {
66
description: 'disallow {{unbound}} helper',
77
category: 'Deprecations',
88
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-unbound.md',
9-
templateMode: 'loose',
9+
templateMode: 'both',
1010
},
1111
schema: [],
1212
messages: { unexpected: 'Unexpected {{unboundHelper}} usage.' },
@@ -18,18 +18,45 @@ module.exports = {
1818
},
1919
},
2020
create(context) {
21+
// `unbound` is an ambient strict-mode keyword (Ember BUILTIN_KEYWORD_HELPERS,
22+
// listed in STRICT_MODE_KEYWORDS), so `{{unbound foo}}` resolves to the
23+
// built-in helper in .gjs/.gts without an import. Flag it everywhere
24+
// unless shadowed by a JS binding or template block param —
25+
// ember-eslint-parser registers template block params in scope, so a
26+
// single getScope walk covers both.
27+
const sourceCode = context.sourceCode;
28+
29+
function isInScope(node, name) {
30+
if (!sourceCode) return false;
31+
try {
32+
let scope = sourceCode.getScope(node);
33+
while (scope) {
34+
if (scope.variables.some((v) => v.name === name)) return true;
35+
scope = scope.upper;
36+
}
37+
} catch {
38+
// getScope not available in .hbs-only mode
39+
}
40+
return false;
41+
}
42+
2143
function check(node) {
22-
if (node.path?.type === 'GlimmerPathExpression' && node.path.original === 'unbound') {
44+
if (
45+
node.path?.type === 'GlimmerPathExpression' &&
46+
node.path.original === 'unbound' &&
47+
!isInScope(node, 'unbound')
48+
) {
2349
context.report({
2450
node,
2551
messageId: 'unexpected',
2652
data: { unboundHelper: '{{unbound}}' },
2753
});
2854
}
2955
}
56+
3057
return {
31-
GlimmerMustacheStatement: check,
3258
GlimmerBlockStatement: check,
59+
GlimmerMustacheStatement: check,
3360
GlimmerSubExpression: check,
3461
};
3562
},

‎lib/rules/template-simple-unless.js‎

Lines changed: 2 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ module.exports = {
1616
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-simple-unless.md',
1717
templateMode: 'both',
1818
},
19-
fixable: 'code',
2019
schema: [
2120
{
2221
type: 'object',
@@ -59,64 +58,6 @@ module.exports = {
5958
return false;
6059
}
6160

62-
/**
63-
* Build a fixer for the _withHelper case.
64-
* Converts `unless` to `if` and wraps the first param with `(not ...)`.
65-
* Special-cases when the first param is already `(not ...)`:
66-
* - single arg: `unless (not x)` → `if x`
67-
* - multiple args: `unless (not x y)` → `if (or x y)`
68-
*/
69-
function buildWithHelperFix(node) {
70-
return function fix(fixer) {
71-
const nodeText = sourceCode.getText(node);
72-
const firstParam = node.params[0];
73-
const firstParamText = sourceCode.getText(firstParam);
74-
75-
// Replace 'unless' with 'if' in the keyword
76-
let newText = nodeText.replace(/^({{#?)unless/, '$1if');
77-
78-
if (firstParam.type === 'GlimmerSubExpression' && firstParam.path?.original === 'not') {
79-
// Special case: first param is (not ...)
80-
if (firstParam.params.length > 1) {
81-
// (not x y) → (or x y)
82-
const innerParamsText = firstParam.params.map((p) => sourceCode.getText(p)).join(' ');
83-
newText = newText.replace(firstParamText, `(or ${innerParamsText})`);
84-
} else {
85-
// (not x) → x — unwrap the not
86-
const innerText = sourceCode.getText(firstParam.params[0]);
87-
newText = newText.replace(firstParamText, innerText);
88-
}
89-
} else {
90-
// Wrap with (not ...)
91-
newText = newText.replace(firstParamText, `(not ${firstParamText})`);
92-
}
93-
94-
// Also fix the closing tag for block statements
95-
if (node.type === 'GlimmerBlockStatement') {
96-
newText = newText.replace(/{{\/unless}}$/, '{{/if}}');
97-
}
98-
99-
return fixer.replaceText(node, newText);
100-
};
101-
}
102-
103-
/**
104-
* Build a fixer for the _followingElseBlock case (simple else, no else-if).
105-
* Swaps body/inverse and changes unless→if.
106-
*/
107-
function buildFollowingElseBlockFix(node) {
108-
return function fix(fixer) {
109-
const programStart = node.program.range[0];
110-
const bodyText = sourceCode.text.slice(programStart, node.program.range[1]);
111-
const inverseText = sourceCode.text.slice(node.inverse.range[0], node.inverse.range[1]);
112-
const openingTag = sourceCode.text
113-
.slice(node.range[0], programStart)
114-
.replace(/^({{#)unless/, '$1if');
115-
// Result shape: {{#if cond}}inverse{{else}}body{{/if}}
116-
return fixer.replaceText(node, `${openingTag}${inverseText}{{else}}${bodyText}{{/if}}`);
117-
};
118-
}
119-
12061
function checkWithHelper(node) {
12162
let helperCount = 0;
12263
let nextParams = node.params || [];
@@ -137,7 +78,6 @@ module.exports = {
13778
data: {
13879
message: `Using {{unless}} in combination with other helpers should be avoided. MaxHelpers: ${maxHelpers}`,
13980
},
140-
fix: buildWithHelperFix(node),
14181
});
14282
return;
14383
}
@@ -149,7 +89,6 @@ module.exports = {
14989
data: {
15090
message: `Using {{unless}} in combination with other helpers should be avoided. Allowed helper${allowlist.length > 1 ? 's' : ''}: ${allowlist}`,
15191
},
152-
fix: buildWithHelperFix(node),
15392
});
15493
return;
15594
}
@@ -161,7 +100,6 @@ module.exports = {
161100
data: {
162101
message: `Using {{unless}} in combination with other helpers should be avoided. Restricted helper${denylist.length > 1 ? 's' : ''}: ${denylist}`,
163102
},
164-
fix: buildWithHelperFix(node),
165103
});
166104
return;
167105
}
@@ -190,21 +128,19 @@ module.exports = {
190128
if (isUnless(node)) {
191129
// Check for {{#unless}}...{{else if}}
192130
if (nodeInverse.body[0] && isIf(nodeInverse.body[0])) {
193-
// Not fixable (ETL: _followingElseIfBlock, isFixable=false)
194131
context.report({
195132
node: node.program || node,
196133
messageId: 'followingElseBlock',
197134
});
198135
} else {
199-
// {{#unless}}...{{else}} — fixable
136+
// {{#unless}}...{{else}}
200137
context.report({
201138
node: node.program || node,
202139
messageId: 'followingElseBlock',
203-
fix: buildFollowingElseBlockFix(node),
204140
});
205141
}
206142
} else if (isElseUnlessBlock(nodeInverse.body[0])) {
207-
// {{#if}}...{{else unless}} — not fixable (ETL: isFixable=false)
143+
// {{#if}}...{{else unless}}
208144
context.report({
209145
node: nodeInverse.body[0],
210146
messageId: 'asElseUnlessBlock',

0 commit comments

Comments
 (0)