-
Notifications
You must be signed in to change notification settings - Fork 235
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #1716 from ember-template-lint/add-no-dynamic-sube…
…xpression-invocations Add `no-dynamic-subexpression-invocations`.
- Loading branch information
Showing
5 changed files
with
367 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
# no-dynamic-subexpression-invocations | ||
|
||
When using Ember versions prior to 3.25 the usage of dynamic invocations for | ||
helpers and modifiers did not work. Unfortunately, some versions of Ember | ||
silently ignored additional arguments (3.16) wherease others throw a very | ||
bizarre error (3.20) when you attempt to invoke what the rendering engine knows as a | ||
dynamic value as if it were a helper. | ||
|
||
For example, in versions of Ember prior to 3.25 the helper invocation here is impossible: | ||
|
||
```hbs | ||
{{#if (this.someHelper)}} | ||
Hi! | ||
{{/if}} | ||
``` | ||
|
||
This rule helps applications using Ember versions prior to 3.25 avoid these types of invocation. | ||
|
||
## Examples | ||
|
||
This rule **forbids** the following: | ||
|
||
```hbs | ||
{{! invoking a yielded block param as if it were a helper }} | ||
{{#let anything as |blockParamValue|}} | ||
<button onclick={{blockParamValue someArgument}}></button> | ||
{{/let}} | ||
``` | ||
|
||
```hbs | ||
{{! invoking a path as if it were a helper }} | ||
<Foo data-any-attribute={{this.anything someArgument}} /> | ||
<Foo data-any-attribute={{some.other.path someArgument}} /> | ||
``` | ||
|
||
```hbs | ||
{{! invoking a yielded block param as if it were a modifier }} | ||
{{#let anything as |blockParamValue|}} | ||
<Foo {{blockParamValue}} /> | ||
{{/let}} | ||
``` | ||
|
||
```hbs | ||
{{! invoking a path as if it were a modifier }} | ||
<Foo {{this.anything}} /> | ||
<Foo {{some.other.path}} /> | ||
``` | ||
|
||
This rule **allows** the following: | ||
|
||
```hbs | ||
{{! use `fn` to wrap a function}} | ||
{{#let anything as |blockParamValue|}} | ||
<button onclick={{fn blockParamValue someArgument}}></button> | ||
{{/let}} | ||
``` | ||
|
||
## References | ||
|
||
RFC's introducing this functionality in Ember > 3.25: | ||
|
||
* [emberjs/rfcs#432](https://github.com/emberjs/rfcs/blob/master/text/0432-contextual-helpers.md) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
'use strict'; | ||
|
||
const Rule = require('./base'); | ||
|
||
module.exports = class NoDynamicSubexpressionInvocations extends Rule { | ||
logDynamicInvocation(node, path) { | ||
let isLocal = this.scope.isLocal(node.path); | ||
let isPath = node.path.parts.length > 1; | ||
let isThisPath = node.path.original.startsWith('this.'); | ||
let isNamedArgument = node.path.original.startsWith('@'); | ||
let hasArguments = node.params.length > 0 || node.hash.length > 0; | ||
let isDynamic = isLocal || isNamedArgument || isPath || isThisPath; | ||
|
||
switch (node.type) { | ||
case 'ElementModifierStatement': | ||
case 'SubExpression': | ||
if (isDynamic) { | ||
this.log({ | ||
message: `You cannot invoke a dynamic value in the ${node.type} position`, | ||
line: node.loc && node.loc.start.line, | ||
column: node.loc && node.loc.start.column, | ||
source: this.sourceForNode(node), | ||
}); | ||
} | ||
break; | ||
case 'MustacheStatement': { | ||
let parents = [...path.parents()]; | ||
let isAttr = parents.some((it) => it.node.type === 'AttrNode'); | ||
|
||
if (isAttr && isDynamic && hasArguments) { | ||
this.log({ | ||
message: 'You must use `fn` helper to invoke a function with arguments', | ||
line: node.loc && node.loc.start.line, | ||
column: node.loc && node.loc.start.column, | ||
source: this.sourceForNode(node), | ||
}); | ||
} | ||
} | ||
} | ||
} | ||
|
||
visitor() { | ||
return { | ||
MustacheStatement(node, path) { | ||
this.logDynamicInvocation(node, path); | ||
}, | ||
|
||
SubExpression(node, path) { | ||
this.logDynamicInvocation(node, path); | ||
}, | ||
|
||
ElementModifierStatement(node, path) { | ||
this.logDynamicInvocation(node, path); | ||
}, | ||
}; | ||
} | ||
}; |
241 changes: 241 additions & 0 deletions
241
test/unit/rules/no-dynamic-subexpression-invocations-test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,241 @@ | ||
'use strict'; | ||
|
||
const generateRuleTests = require('../../helpers/rule-test-harness'); | ||
|
||
generateRuleTests({ | ||
name: 'no-dynamic-subexpression-invocations', | ||
|
||
config: true, | ||
|
||
good: [ | ||
'{{something "here"}}', | ||
'{{something}}', | ||
'{{something here="goes"}}', | ||
'<button onclick={{fn something "here"}}></button>', | ||
'{{@thing "somearg"}}', | ||
], | ||
|
||
bad: [ | ||
{ | ||
template: '<Foo bar="{{@thing "some-arg"}}" />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 10, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{@thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Foo {{this.foo}} />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 5, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You cannot invoke a dynamic value in the ElementModifierStatement position", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{this.foo}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Foo {{@foo}} />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 5, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You cannot invoke a dynamic value in the ElementModifierStatement position", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{@foo}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Foo {{foo.bar}} />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 5, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You cannot invoke a dynamic value in the ElementModifierStatement position", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{foo.bar}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<button onclick={{@thing "some-arg"}}></button>', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 16, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{@thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: | ||
'{{#let "whatever" as |thing|}}<button onclick={{thing "some-arg"}}></button>{{/let}}', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 46, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<button onclick={{this.thing "some-arg"}}></button>', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 16, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{this.thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<button onclick={{lol.other.path "some-arg"}}></button>', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 16, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{lol.other.path \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '{{if (this.foo) "true" "false"}}', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 5, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You cannot invoke a dynamic value in the SubExpression position", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "(this.foo)", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Foo @bar={{@thing "some-arg"}} />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 10, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{@thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
{ | ||
template: '<Foo onclick={{@thing "some-arg"}} />', | ||
|
||
verifyResults(results) { | ||
expect(results).toMatchInlineSnapshot(` | ||
Array [ | ||
Object { | ||
"column": 13, | ||
"filePath": "layout.hbs", | ||
"line": 1, | ||
"message": "You must use \`fn\` helper to invoke a function with arguments", | ||
"moduleId": "layout", | ||
"rule": "no-dynamic-subexpression-invocations", | ||
"severity": 2, | ||
"source": "{{@thing \\"some-arg\\"}}", | ||
}, | ||
] | ||
`); | ||
}, | ||
}, | ||
], | ||
}); |