Skip to content

Commit 8952249

Browse files
NullVoxPopuliNullVoxPopuli
authored
Merge pull request #2567 from NullVoxPopuli/nvp/template-lint-extract-rule-template-no-redundant-fn
Extract rule: template-no-redundant-fn
2 parents ba63d48 + ba7a105 commit 8952249

4 files changed

Lines changed: 272 additions & 0 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ rules in templates can be disabled with eslint directives with mustache or html
261261
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
262262
| [template-no-positional-data-test-selectors](docs/rules/template-no-positional-data-test-selectors.md) | disallow positional data-test-* params in curly invocations | | | |
263263
| [template-no-potential-path-strings](docs/rules/template-no-potential-path-strings.md) | disallow potential path strings in attribute values | | | |
264+
| [template-no-redundant-fn](docs/rules/template-no-redundant-fn.md) | disallow unnecessary usage of (fn) helper | | | |
264265
| [template-no-restricted-invocations](docs/rules/template-no-restricted-invocations.md) | disallow certain components, helpers or modifiers from being used | | | |
265266
| [template-no-splattributes-with-class](docs/rules/template-no-splattributes-with-class.md) | disallow splattributes with class attribute | | | |
266267
| [template-no-this-in-template-only-components](docs/rules/template-no-this-in-template-only-components.md) | disallow this in template-only components (gjs/gts) | | 🔧 | |

‎docs/rules/template-no-redundant-fn.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# ember/template-no-redundant-fn
2+
3+
<!-- end auto-generated rule header -->
4+
5+
The `fn` helper can be used to bind arguments to another function. Using it without any arguments is redundant because then the inner function could just be used directly.
6+
7+
This rule is looking for `fn` helper usages that don't provide any additional arguments to the inner function and warns about them.
8+
9+
## Examples
10+
11+
This rule **forbids** the following:
12+
13+
```hbs
14+
<button {{on 'click' (fn this.handleClick)}}>Click Me</button>
15+
```
16+
17+
This rule **allows** the following:
18+
19+
```hbs
20+
<button {{on 'click' this.handleClick}}>Click Me</button>
21+
```
22+
23+
```hbs
24+
<button {{on 'click' (fn this.handleClick 'foo')}}>Click Me</button>
25+
```
26+
27+
## References
28+
29+
- [Ember Guides](https://guides.emberjs.com/release/components/component-state-and-actions/#toc_passing-arguments-to-actions)
30+
- [`fn` API documentation](https://api.emberjs.com/ember/3.20/classes/Ember.Templates.helpers/methods/fn?anchor=fn)

‎lib/rules/template-no-redundant-fn.js‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/** @type {import('eslint').Rule.RuleModule} */
2+
module.exports = {
3+
meta: {
4+
type: 'suggestion',
5+
docs: {
6+
description: 'disallow unnecessary usage of (fn) helper',
7+
category: 'Best Practices',
8+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-redundant-fn.md',
9+
templateMode: 'both',
10+
},
11+
fixable: null,
12+
schema: [],
13+
messages: {
14+
redundant:
15+
'Unnecessary use of (fn) helper. Pass the function directly instead: {{suggestion}}',
16+
},
17+
originallyFrom: {
18+
name: 'ember-template-lint',
19+
rule: 'lib/rules/no-redundant-fn.js',
20+
docs: 'docs/rule/no-redundant-fn.md',
21+
tests: 'test/unit/rules/no-redundant-fn-test.js',
22+
},
23+
},
24+
25+
create(context) {
26+
const sourceCode = context.sourceCode;
27+
28+
/**
29+
* Check whether `fn` in the template resolves to a local/imported binding.
30+
* If it does, the user has shadowed Ember's built-in `fn` helper and we
31+
* should not flag the usage.
32+
*/
33+
function isFnShadowed(node) {
34+
const head = node.path?.head;
35+
if (!head) {
36+
return false;
37+
}
38+
const scope = sourceCode.getScope(node);
39+
const ref = scope.references.find((r) => r.identifier === head);
40+
return ref?.resolved !== null && ref?.resolved !== undefined;
41+
}
42+
43+
function checkFnUsage(node) {
44+
// Check if this is an (fn) call with only one argument (the function itself)
45+
if (
46+
node.path &&
47+
node.path.type === 'GlimmerPathExpression' &&
48+
node.path.original === 'fn' &&
49+
node.params &&
50+
node.params.length === 1 &&
51+
!node.hash?.pairs?.length
52+
) {
53+
// In gjs/gts, `fn` may be shadowed by a local import — skip if so.
54+
if (isFnShadowed(node)) {
55+
return;
56+
}
57+
const param = node.params[0];
58+
const paramText =
59+
param.type === 'GlimmerPathExpression'
60+
? param.original
61+
: context.sourceCode.getText(param);
62+
63+
context.report({
64+
node,
65+
messageId: 'redundant',
66+
data: {
67+
suggestion: paramText,
68+
},
69+
});
70+
}
71+
}
72+
73+
return {
74+
GlimmerSubExpression(node) {
75+
checkFnUsage(node);
76+
},
77+
78+
GlimmerMustacheStatement(node) {
79+
checkFnUsage(node);
80+
},
81+
};
82+
},
83+
};

‎tests/lib/rules/template-no-redundant-fn.js‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
//------------------------------------------------------------------------------
2+
// Requirements
3+
//------------------------------------------------------------------------------
4+
5+
const rule = require('../../../lib/rules/template-no-redundant-fn');
6+
const RuleTester = require('eslint').RuleTester;
7+
8+
//------------------------------------------------------------------------------
9+
// Tests
10+
//------------------------------------------------------------------------------
11+
12+
const ruleTester = new RuleTester({
13+
parser: require.resolve('ember-eslint-parser'),
14+
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
15+
});
16+
17+
ruleTester.run('template-no-redundant-fn', rule, {
18+
valid: [
19+
`<template>
20+
<button {{on "click" this.handleClick}}>Click</button>
21+
</template>`,
22+
`<template>
23+
<button {{on "click" (fn this.handleClick arg)}}>Click</button>
24+
</template>`,
25+
`<template>
26+
<button {{on "click" (fn this.handleClick arg1 arg2)}}>Click</button>
27+
</template>`,
28+
`<template>
29+
<Component @action={{this.myAction}} />
30+
</template>`,
31+
32+
'<template><button {{on "click" this.handleClick}}>Click Me</button></template>',
33+
'<template><button {{on "click" (fn this.handleClick "foo")}}>Click Me</button></template>',
34+
'<template><SomeComponent @onClick={{this.handleClick}} /></template>',
35+
'<template><SomeComponent @onClick={{fn this.handleClick "foo"}} /></template>',
36+
'<template>{{foo bar=this.handleClick}}></template>',
37+
'<template>{{foo bar=(fn this.handleClick "foo")}}></template>',
38+
39+
// `fn` is imported from a local module — not Ember's built-in helper.
40+
// Should NOT be flagged even with a single argument.
41+
`
42+
import { fn } from './my-utils';
43+
<template>
44+
<button {{on "click" (fn this.handleClick)}}>Click</button>
45+
</template>
46+
`,
47+
],
48+
49+
invalid: [
50+
{
51+
code: `<template>
52+
<button {{on "click" (fn this.handleClick)}}>Click</button>
53+
</template>`,
54+
output: null,
55+
errors: [
56+
{
57+
message:
58+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
59+
type: 'GlimmerSubExpression',
60+
},
61+
],
62+
},
63+
{
64+
code: `<template>
65+
<Component @action={{fn this.save}} />
66+
</template>`,
67+
output: null,
68+
errors: [
69+
{
70+
message: 'Unnecessary use of (fn) helper. Pass the function directly instead: this.save',
71+
type: 'GlimmerMustacheStatement',
72+
},
73+
],
74+
},
75+
76+
{
77+
code: '<template><button {{on "click" (fn this.handleClick)}}>Click Me</button></template>',
78+
output: null,
79+
errors: [
80+
{
81+
message:
82+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
83+
},
84+
],
85+
},
86+
{
87+
code: '<template><SomeComponent @onClick={{fn this.handleClick}} /></template>',
88+
output: null,
89+
errors: [
90+
{
91+
message:
92+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
93+
},
94+
],
95+
},
96+
{
97+
code: '<template>{{foo bar=(fn this.handleClick)}}></template>',
98+
output: null,
99+
errors: [
100+
{
101+
message:
102+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
103+
},
104+
],
105+
},
106+
],
107+
});
108+
109+
const hbsRuleTester = new RuleTester({
110+
parser: require.resolve('ember-eslint-parser/hbs'),
111+
parserOptions: {
112+
ecmaVersion: 2022,
113+
sourceType: 'module',
114+
},
115+
});
116+
117+
hbsRuleTester.run('template-no-redundant-fn', rule, {
118+
valid: [
119+
'<button {{on "click" this.handleClick}}>Click Me</button>',
120+
'<button {{on "click" (fn this.handleClick "foo")}}>Click Me</button>',
121+
'<SomeComponent @onClick={{this.handleClick}} />',
122+
'<SomeComponent @onClick={{fn this.handleClick "foo"}} />',
123+
'{{foo bar=this.handleClick}}>',
124+
'{{foo bar=(fn this.handleClick "foo")}}>',
125+
],
126+
invalid: [
127+
{
128+
code: '<button {{on "click" (fn this.handleClick)}}>Click Me</button>',
129+
output: null,
130+
errors: [
131+
{
132+
message:
133+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
134+
},
135+
],
136+
},
137+
{
138+
code: '<SomeComponent @onClick={{fn this.handleClick}} />',
139+
output: null,
140+
errors: [
141+
{
142+
message:
143+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
144+
},
145+
],
146+
},
147+
{
148+
code: '{{foo bar=(fn this.handleClick)}}>',
149+
output: null,
150+
errors: [
151+
{
152+
message:
153+
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
154+
},
155+
],
156+
},
157+
],
158+
});

0 commit comments

Comments
 (0)