Skip to content

Commit f271c55

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2590 from NullVoxPopuli/nvp/template-lint-extract-rule-template-no-unused-block-params
Extract rule: template-no-unused-block-params
2 parents 506418a + d2b64e0 commit f271c55

4 files changed

Lines changed: 368 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ rules in templates can be disabled with eslint directives with mustache or html
255255
| [template-no-obsolete-elements](docs/rules/template-no-obsolete-elements.md) | disallow obsolete HTML elements | | | |
256256
| [template-no-outlet-outside-routes](docs/rules/template-no-outlet-outside-routes.md) | disallow {{outlet}} outside of route templates | | | |
257257
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
258+
| [template-no-unused-block-params](docs/rules/template-no-unused-block-params.md) | disallow unused block parameters in templates | | | |
258259
| [template-no-valueless-arguments](docs/rules/template-no-valueless-arguments.md) | disallow valueless named arguments | | | |
259260
| [template-no-whitespace-for-layout](docs/rules/template-no-whitespace-for-layout.md) | disallow using whitespace for layout purposes | | | |
260261
| [template-no-yield-block-params-to-else-inverse](docs/rules/template-no-yield-block-params-to-else-inverse.md) | disallow yielding block params to else or inverse block | | | |

‎docs/rules/template-no-unused-block-params.md‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# ember/template-no-unused-block-params
2+
3+
<!-- end auto-generated rule header -->
4+
5+
This rule forbids unused block parameters except when they are needed to access a later parameter.
6+
7+
## Examples
8+
9+
This rule **forbids** the following (unused parameters):
10+
11+
```gjs
12+
<template>
13+
{{#each users as |user index|}}
14+
{{user.name}}
15+
{{/each}}
16+
</template>
17+
```
18+
19+
This rule **allows** the following:
20+
21+
Allowed (used parameters):
22+
23+
```gjs
24+
<template>
25+
{{#each users as |user|}}
26+
{{user.name}}
27+
{{/each}}
28+
</template>
29+
```
30+
31+
```gjs
32+
<template>
33+
{{#each users as |user index|}}
34+
{{index}} {{user.name}}
35+
{{/each}}
36+
</template>
37+
```
38+
39+
Allowed (later parameter used):
40+
41+
```gjs
42+
<template>
43+
{{#each users as |user index|}}
44+
{{index}}
45+
{{/each}}
46+
</template>
47+
```
48+
49+
## Related rules
50+
51+
- [eslint/no-unused-vars](https://eslint.org/docs/rules/no-unused-vars)
52+
53+
## References
54+
55+
- [Ember guides/block content](https://guides.emberjs.com/release/components/block-content/)
56+
- [rfcs/angle bracket invocation](https://emberjs.github.io/rfcs/0311-angle-bracket-invocation.html)
57+
- [rfcs/named blocks](https://emberjs.github.io/rfcs/0226-named-blocks.html)

‎lib/rules/template-no-unused-block-params.js‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
function collectChildNodes(n) {
2+
const children = [];
3+
if (n.program) {
4+
children.push(n.program);
5+
}
6+
if (n.inverse) {
7+
children.push(n.inverse);
8+
}
9+
if (n.params) {
10+
children.push(...n.params);
11+
}
12+
if (n.hash?.pairs) {
13+
children.push(...n.hash.pairs.map((p) => p.value));
14+
}
15+
if (n.body) {
16+
children.push(...n.body);
17+
}
18+
if (n.path) {
19+
children.push(n.path);
20+
}
21+
if (n.attributes) {
22+
children.push(...n.attributes.map((a) => a.value));
23+
}
24+
if (n.children) {
25+
children.push(...n.children);
26+
}
27+
return children;
28+
}
29+
30+
function markParamIfUsed(name, blockParams, usedParams, shadowedParams) {
31+
const firstPart = name.split('.')[0];
32+
if (blockParams.includes(firstPart) && !shadowedParams.has(firstPart)) {
33+
usedParams.add(firstPart);
34+
}
35+
}
36+
37+
function isPartialStatement(n) {
38+
return (
39+
(n.type === 'GlimmerMustacheStatement' || n.type === 'GlimmerBlockStatement') &&
40+
n.path?.original === 'partial'
41+
);
42+
}
43+
44+
function buildShadowedSet(shadowedParams, innerBlockParams, outerBlockParams) {
45+
const newShadowed = new Set(shadowedParams);
46+
for (const p of innerBlockParams) {
47+
if (outerBlockParams.includes(p)) {
48+
newShadowed.add(p);
49+
}
50+
}
51+
return newShadowed;
52+
}
53+
54+
/** @type {import('eslint').Rule.RuleModule} */
55+
module.exports = {
56+
meta: {
57+
type: 'suggestion',
58+
docs: {
59+
description: 'disallow unused block parameters in templates',
60+
category: 'Best Practices',
61+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-unused-block-params.md',
62+
templateMode: 'both',
63+
},
64+
schema: [],
65+
messages: {
66+
unusedBlockParam: "'{{param}}' is defined but never used",
67+
},
68+
originallyFrom: {
69+
name: 'ember-template-lint',
70+
rule: 'lib/rules/no-unused-block-params.js',
71+
docs: 'docs/rule/no-unused-block-params.md',
72+
tests: 'test/unit/rules/no-unused-block-params-test.js',
73+
},
74+
},
75+
76+
create(context) {
77+
return {
78+
GlimmerBlockStatement(node) {
79+
const blockParams = node.program?.blockParams || [];
80+
if (blockParams.length === 0) {
81+
return;
82+
}
83+
84+
const usedParams = new Set();
85+
86+
function checkNode(n, shadowedParams) {
87+
if (!n) {
88+
return;
89+
}
90+
91+
if (n.type === 'GlimmerPathExpression') {
92+
markParamIfUsed(n.original, blockParams, usedParams, shadowedParams);
93+
}
94+
95+
if (n.type === 'GlimmerElementNode') {
96+
markParamIfUsed(n.tag, blockParams, usedParams, shadowedParams);
97+
}
98+
99+
if (isPartialStatement(n)) {
100+
for (const p of blockParams) {
101+
if (!shadowedParams.has(p)) {
102+
usedParams.add(p);
103+
}
104+
}
105+
}
106+
107+
// When entering a nested block, add its blockParams to the shadowed set
108+
if (n.type === 'GlimmerBlockStatement' && n.program?.blockParams?.length > 0) {
109+
const newShadowed = buildShadowedSet(
110+
shadowedParams,
111+
n.program.blockParams,
112+
blockParams
113+
);
114+
checkBlockParts(n, blockParams, usedParams, shadowedParams, newShadowed, checkNode);
115+
return;
116+
}
117+
118+
// Recursively check children
119+
for (const child of collectChildNodes(n)) {
120+
checkNode(child, shadowedParams);
121+
}
122+
}
123+
124+
checkNode(node.program, new Set());
125+
126+
// Find the last index of a used param
127+
let lastUsedIndex = -1;
128+
for (let i = blockParams.length - 1; i >= 0; i--) {
129+
if (usedParams.has(blockParams[i])) {
130+
lastUsedIndex = i;
131+
break;
132+
}
133+
}
134+
135+
// Only report trailing unused params (after the last used one)
136+
const unusedTrailing = blockParams.slice(lastUsedIndex + 1);
137+
const firstUnusedTrailing = unusedTrailing[0];
138+
139+
if (firstUnusedTrailing) {
140+
context.report({
141+
node,
142+
messageId: 'unusedBlockParam',
143+
data: { param: firstUnusedTrailing },
144+
});
145+
}
146+
},
147+
};
148+
},
149+
};
150+
151+
function checkBlockParts(n, blockParams, usedParams, shadowedParams, newShadowed, checkNodeFn) {
152+
// Check the path/params of the block statement itself with current scope
153+
if (n.path) {
154+
checkNodeFn(n.path, shadowedParams);
155+
}
156+
if (n.params) {
157+
for (const param of n.params) {
158+
checkNodeFn(param, shadowedParams);
159+
}
160+
}
161+
if (n.hash?.pairs) {
162+
for (const pair of n.hash.pairs) {
163+
checkNodeFn(pair.value, shadowedParams);
164+
}
165+
}
166+
167+
// Check the program body with the updated shadowed set
168+
if (n.program) {
169+
checkNodeFn(n.program, newShadowed);
170+
}
171+
if (n.inverse) {
172+
checkNodeFn(n.inverse, newShadowed);
173+
}
174+
}

‎tests/lib/rules/template-no-unused-block-params.js‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
const eslint = require('eslint');
2+
const rule = require('../../../lib/rules/template-no-unused-block-params');
3+
4+
const { RuleTester } = eslint;
5+
6+
const validHbs = [
7+
'{{cat}}',
8+
'{{#each cats as |cat|}}{{cat}}{{/each}}',
9+
'{{#each cats as |cat|}}{{partial "cat"}}{{/each}}',
10+
'{{#each cats as |cat|}}{{cat.name}}{{/each}}',
11+
'{{#each cats as |cat|}}{{meow cat}}{{/each}}',
12+
'{{#each cats as |cat index|}}{{index}}{{/each}}',
13+
'{{#each cats as |cat index|}}{{#each cat.lives as |life|}}{{index}}: {{life}}{{/each}}{{/each}}',
14+
`
15+
<MyComponent @model={{this.model}} as |param|>
16+
{{! template-lint-disable }}
17+
<MyOtherComponent .... @param={{param}} />
18+
{{! template-lint-enable }}
19+
</MyComponent>
20+
`,
21+
`
22+
<MyComponent @model={{this.model}} as |param|>
23+
{{! template-lint-disable }}
24+
{{foo-bar param}}
25+
{{! template-lint-enable }}
26+
</MyComponent>
27+
`,
28+
`
29+
<MyComponent @model={{this.model}} as |param|>
30+
{{! template-lint-disable }}
31+
{{param}}
32+
{{! template-lint-enable }}
33+
</MyComponent>
34+
`,
35+
`
36+
<MyComponent @model={{this.model}} as |param|>
37+
{{! template-lint-disable }}
38+
{{foo-bar prop=param}}
39+
{{! template-lint-enable }}
40+
</MyComponent>
41+
`,
42+
`
43+
{{#my-component as |param|}}
44+
{{! template-lint-disable }}
45+
<MyOtherComponent .... @param={{param}} />
46+
{{! template-lint-enable }}
47+
{{/my-component}}
48+
`,
49+
`
50+
{{#my-component as |param|}}
51+
{{! template-lint-disable }}
52+
{{foo-bar param}}
53+
{{! template-lint-enable }}
54+
{{/my-component}}
55+
`,
56+
`
57+
{{#my-component as |param|}}
58+
{{! template-lint-disable }}
59+
{{param}}
60+
{{! template-lint-enable }}
61+
{{/my-component}}
62+
`,
63+
`
64+
{{#my-component as |param bar baz|}}
65+
{{! template-lint-disable }}
66+
{{foo-bar prop=param}}
67+
{{! template-lint-enable }}
68+
{{bar}}
69+
{{! template-lint-disable }}
70+
{{foo-bar prop=baz}}
71+
{{! template-lint-enable }}
72+
{{/my-component}}
73+
`,
74+
'{{#each cats as |cat|}}{{#meow-meow cat as |cat|}}{{cat}}{{/meow-meow}}{{/each}}',
75+
'{{#with (component "foo-bar") as |FooBar|}}<FooBar />{{/with}}',
76+
'<BurgerMenu as |menu|><header>Something</header><menu.item>Text</menu.item></BurgerMenu>',
77+
'{{#burger-menu as |menu|}}<header>Something</header>{{#menu.item}}Text{{/menu.item}}{{/burger-menu}}',
78+
];
79+
80+
const invalidHbs = [
81+
{
82+
code: '{{#each cats as |cat|}}Dogs{{/each}}',
83+
output: null,
84+
errors: [{ messageId: 'unusedBlockParam', data: { param: 'cat' } }],
85+
},
86+
{
87+
code: '{{#each cats as |cat index|}}{{cat}}{{/each}}',
88+
output: null,
89+
errors: [{ messageId: 'unusedBlockParam', data: { param: 'index' } }],
90+
},
91+
{
92+
code: '{{#each cats as |cat index|}}{{#each cat.lives as |life index|}}{{index}}: {{life}}{{/each}}{{/each}}',
93+
output: null,
94+
errors: [{ messageId: 'unusedBlockParam', data: { param: 'index' } }],
95+
},
96+
{
97+
code: '{{#each cats as |cat index|}}{{partial "cat"}}{{#each cat.lives as |life|}}Life{{/each}}{{/each}}',
98+
output: null,
99+
errors: [{ messageId: 'unusedBlockParam', data: { param: 'life' } }],
100+
},
101+
];
102+
103+
function wrapTemplate(entry) {
104+
if (typeof entry === 'string') {
105+
return `<template>${entry}</template>`;
106+
}
107+
108+
return {
109+
...entry,
110+
code: `<template>${entry.code}</template>`,
111+
output: entry.output ? `<template>${entry.output}</template>` : entry.output,
112+
};
113+
}
114+
115+
const gjsRuleTester = new RuleTester({
116+
parser: require.resolve('ember-eslint-parser'),
117+
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
118+
});
119+
120+
gjsRuleTester.run('template-no-unused-block-params', rule, {
121+
valid: validHbs.map(wrapTemplate),
122+
invalid: invalidHbs.map(wrapTemplate),
123+
});
124+
125+
const hbsRuleTester = new RuleTester({
126+
parser: require.resolve('ember-eslint-parser/hbs'),
127+
parserOptions: {
128+
ecmaVersion: 2022,
129+
sourceType: 'module',
130+
},
131+
});
132+
133+
hbsRuleTester.run('template-no-unused-block-params', rule, {
134+
valid: validHbs,
135+
invalid: invalidHbs,
136+
});

0 commit comments

Comments
 (0)