Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ rules in templates can be disabled with eslint directives with mustache or html
| [template-no-page-title-component](docs/rules/template-no-page-title-component.md) | disallow usage of ember-page-title component | | | |
| [template-no-positional-data-test-selectors](docs/rules/template-no-positional-data-test-selectors.md) | disallow positional data-test-* params in curly invocations | | | |
| [template-no-potential-path-strings](docs/rules/template-no-potential-path-strings.md) | disallow potential path strings in attribute values | | | |
| [template-no-redundant-fn](docs/rules/template-no-redundant-fn.md) | disallow unnecessary usage of (fn) helper | | | |
| [template-no-restricted-invocations](docs/rules/template-no-restricted-invocations.md) | disallow certain components, helpers or modifiers from being used | | | |
| [template-no-splattributes-with-class](docs/rules/template-no-splattributes-with-class.md) | disallow splattributes with class attribute | | | |
| [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) | | 🔧 | |
Expand Down
30 changes: 30 additions & 0 deletions docs/rules/template-no-redundant-fn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# ember/template-no-redundant-fn

<!-- end auto-generated rule header -->

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.

This rule is looking for `fn` helper usages that don't provide any additional arguments to the inner function and warns about them.

## Examples

This rule **forbids** the following:

```hbs
<button {{on 'click' (fn this.handleClick)}}>Click Me</button>
```

This rule **allows** the following:

```hbs
<button {{on 'click' this.handleClick}}>Click Me</button>
```

```hbs
<button {{on 'click' (fn this.handleClick 'foo')}}>Click Me</button>
```

## References

- [Ember Guides](https://guides.emberjs.com/release/components/component-state-and-actions/#toc_passing-arguments-to-actions)
- [`fn` API documentation](https://api.emberjs.com/ember/3.20/classes/Ember.Templates.helpers/methods/fn?anchor=fn)
83 changes: 83 additions & 0 deletions lib/rules/template-no-redundant-fn.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'disallow unnecessary usage of (fn) helper',
category: 'Best Practices',
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-redundant-fn.md',
templateMode: 'both',
},
fixable: null,
schema: [],
messages: {
redundant:
'Unnecessary use of (fn) helper. Pass the function directly instead: {{suggestion}}',
},
originallyFrom: {
name: 'ember-template-lint',
rule: 'lib/rules/no-redundant-fn.js',
docs: 'docs/rule/no-redundant-fn.md',
tests: 'test/unit/rules/no-redundant-fn-test.js',
},
},

create(context) {
const sourceCode = context.sourceCode;

/**
* Check whether `fn` in the template resolves to a local/imported binding.
* If it does, the user has shadowed Ember's built-in `fn` helper and we
* should not flag the usage.
*/
function isFnShadowed(node) {
const head = node.path?.head;
if (!head) {
return false;
}
const scope = sourceCode.getScope(node);
const ref = scope.references.find((r) => r.identifier === head);
return ref?.resolved !== null && ref?.resolved !== undefined;
}

function checkFnUsage(node) {
// Check if this is an (fn) call with only one argument (the function itself)
if (
node.path &&
node.path.type === 'GlimmerPathExpression' &&
node.path.original === 'fn' &&
node.params &&
node.params.length === 1 &&
!node.hash?.pairs?.length
) {
// In gjs/gts, `fn` may be shadowed by a local import — skip if so.
if (isFnShadowed(node)) {
return;
}
const param = node.params[0];
const paramText =
param.type === 'GlimmerPathExpression'
? param.original
: context.sourceCode.getText(param);

context.report({
node,
messageId: 'redundant',
data: {
suggestion: paramText,
},
});
}
}

return {
GlimmerSubExpression(node) {
checkFnUsage(node);
},

GlimmerMustacheStatement(node) {
checkFnUsage(node);
},
};
},
};
158 changes: 158 additions & 0 deletions tests/lib/rules/template-no-redundant-fn.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require('../../../lib/rules/template-no-redundant-fn');
const RuleTester = require('eslint').RuleTester;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parser: require.resolve('ember-eslint-parser'),
parserOptions: { ecmaVersion: 2022, sourceType: 'module' },
});

ruleTester.run('template-no-redundant-fn', rule, {
valid: [
`<template>
<button {{on "click" this.handleClick}}>Click</button>
</template>`,
`<template>
<button {{on "click" (fn this.handleClick arg)}}>Click</button>
</template>`,
`<template>
<button {{on "click" (fn this.handleClick arg1 arg2)}}>Click</button>
</template>`,
`<template>
<Component @action={{this.myAction}} />
</template>`,

'<template><button {{on "click" this.handleClick}}>Click Me</button></template>',
'<template><button {{on "click" (fn this.handleClick "foo")}}>Click Me</button></template>',
'<template><SomeComponent @onClick={{this.handleClick}} /></template>',
'<template><SomeComponent @onClick={{fn this.handleClick "foo"}} /></template>',
'<template>{{foo bar=this.handleClick}}></template>',
'<template>{{foo bar=(fn this.handleClick "foo")}}></template>',

// `fn` is imported from a local module — not Ember's built-in helper.
// Should NOT be flagged even with a single argument.
`
import { fn } from './my-utils';
<template>
<button {{on "click" (fn this.handleClick)}}>Click</button>
</template>
`,
],

invalid: [
{
code: `<template>
<button {{on "click" (fn this.handleClick)}}>Click</button>
</template>`,
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
type: 'GlimmerSubExpression',
},
],
},
{
code: `<template>
<Component @action={{fn this.save}} />
</template>`,
output: null,
errors: [
{
message: 'Unnecessary use of (fn) helper. Pass the function directly instead: this.save',
type: 'GlimmerMustacheStatement',
},
],
},

{
code: '<template><button {{on "click" (fn this.handleClick)}}>Click Me</button></template>',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
{
code: '<template><SomeComponent @onClick={{fn this.handleClick}} /></template>',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
{
code: '<template>{{foo bar=(fn this.handleClick)}}></template>',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
],
});

const hbsRuleTester = new RuleTester({
parser: require.resolve('ember-eslint-parser/hbs'),
parserOptions: {
ecmaVersion: 2022,
sourceType: 'module',
},
});

hbsRuleTester.run('template-no-redundant-fn', rule, {
valid: [
'<button {{on "click" this.handleClick}}>Click Me</button>',
'<button {{on "click" (fn this.handleClick "foo")}}>Click Me</button>',
'<SomeComponent @onClick={{this.handleClick}} />',
'<SomeComponent @onClick={{fn this.handleClick "foo"}} />',
'{{foo bar=this.handleClick}}>',
'{{foo bar=(fn this.handleClick "foo")}}>',
],
invalid: [
{
code: '<button {{on "click" (fn this.handleClick)}}>Click Me</button>',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
{
code: '<SomeComponent @onClick={{fn this.handleClick}} />',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
{
code: '{{foo bar=(fn this.handleClick)}}>',
output: null,
errors: [
{
message:
'Unnecessary use of (fn) helper. Pass the function directly instead: this.handleClick',
},
],
},
],
});
Loading