Skip to content

Commit 08325e4

Browse files
committed
Implement rule for anonymous functions passed to scheduleOnce and once
1 parent c2c9ba1 commit 08325e4

File tree

4 files changed

+243
-1
lines changed

4 files changed

+243
-1
lines changed

config/recommended.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module.exports = {
1212
rules: {
1313
// Custom rules
1414
'ember-best-practices/no-side-effect-cp': 2,
15+
'ember-best-practices/no-anonymous-once': 2,
1516
'ember-best-practices/no-attrs': 2,
1617
'ember-best-practices/no-observers': 2,
1718
'ember-best-practices/require-dependent-keys': 2,

lib/rules/no-anonymous-once.js

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/**
2+
* @fileOverview Disallow the use of anonymous functions passed to scheduleOnce and once.
3+
*/
4+
'use strict';
5+
6+
const { getCaller, cleanCaller, isCallingWithApply, isCallingWithCall } = require('../utils/caller');
7+
const { collectObjectPatternBindings } = require('../utils/destructed-binding');
8+
const { getEmberImportBinding } = require('../utils/imports');
9+
const MESSAGE
10+
= `The uniqueness once offers is based on function uniqueness, each invocation of this line will always create a new
11+
function instance, resulting in uniqueness checks, that will never be hit. Please replace with a named function.
12+
Reference: https://emberjs.com/api/ember/2.14/namespaces/Ember.run/methods/scheduleOnce?anchor=scheduleOnce`;
13+
14+
const DISALLOWED_OBJECTS = ['Ember.run', 'run'];
15+
const SCHEDULE_ONCE = 'scheduleOnce';
16+
const RUN_METHODS = [SCHEDULE_ONCE, 'once'];
17+
18+
/**
19+
* Extracts the method that we are trying to run once from the list of arguments.
20+
* An optional target parameter can be passed in as the first parameter so we need
21+
* to check the length of the array to determine where our function is being passed in.
22+
*/
23+
function getMethodToRunOnce(args) {
24+
return args.length > 1 ? args[1] : args[0];
25+
}
26+
27+
/**
28+
* scheduleOnce takes in the queue name as the first parameter. In this function we will remove
29+
* that parameter from the array to make it look the same as the arguments to once and facilitate
30+
* extracting the method that we are trying to run once out of the args.
31+
*/
32+
function normalizeArguments(caller, args) {
33+
let mut = args.slice();
34+
35+
if (isCallingWithCall(caller)) {
36+
// Whenever the action was called .call we want to remove the context parameter
37+
mut.shift();
38+
} else if (isCallingWithApply(caller)) {
39+
// Whenever the action was called with .apply we want to get the arguments with which the function
40+
// would actually get called
41+
mut = mut[1].elements.slice();
42+
}
43+
44+
// scheduleOnce takes in the queue name as the first parameter so we have to remove it have a similar
45+
// structure as "once"
46+
if (cleanCaller(caller).indexOf(SCHEDULE_ONCE) > -1) {
47+
mut.shift();
48+
}
49+
50+
return mut;
51+
}
52+
53+
function isAnonymousFunction(fn) {
54+
return fn && !fn.name;
55+
}
56+
57+
function isString(node) {
58+
return node.type === 'Literal' && typeof node.value === 'string';
59+
}
60+
61+
function mergeDisallowedCalls(objects) {
62+
return objects
63+
.reduce((calls, obj) => {
64+
RUN_METHODS.forEach((method) => {
65+
calls.push(`${obj}.${method}`);
66+
});
67+
68+
return calls;
69+
}, []);
70+
}
71+
72+
module.exports = {
73+
docs: {
74+
description: 'Disallow use of anonymous functions when use in scheduleOnce or once',
75+
category: 'Best Practices',
76+
recommended: true
77+
},
78+
meta: {
79+
message: MESSAGE
80+
},
81+
create(context) {
82+
let emberImportBinding;
83+
let disallowedCalls = mergeDisallowedCalls(DISALLOWED_OBJECTS);
84+
85+
return {
86+
ImportDefaultSpecifier(node) {
87+
emberImportBinding = getEmberImportBinding(node);
88+
},
89+
90+
ObjectPattern(node) {
91+
if (!emberImportBinding) {
92+
return;
93+
}
94+
95+
/**
96+
* Retrieves the deconstructed bindings from the Ember import, accounting for aliasing
97+
* of the import.
98+
*/
99+
disallowedCalls = disallowedCalls.concat(
100+
mergeDisallowedCalls(
101+
collectObjectPatternBindings(node, {
102+
[emberImportBinding]: ['run']
103+
})
104+
)
105+
);
106+
},
107+
108+
CallExpression(node) {
109+
const caller = getCaller(node);
110+
111+
if (!disallowedCalls.includes(cleanCaller(caller))) {
112+
return;
113+
}
114+
115+
const normalizedArguments = normalizeArguments(caller, node.arguments);
116+
const fnToRunOnce = getMethodToRunOnce(normalizedArguments);
117+
118+
// The fnToRunceOnce is a string it means that it will be resolved on the target at the time once or
119+
// scheduleOnce is invoked.
120+
if (isAnonymousFunction(fnToRunOnce) && !isString(fnToRunOnce)) {
121+
context.report(node, MESSAGE);
122+
}
123+
}
124+
};
125+
}
126+
};

lib/utils/caller.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,17 @@ function cleanCaller(caller) {
4040
return caller;
4141
}
4242

43+
function isCallingWithCall(caller) {
44+
return endsWith(caller, '.call');
45+
}
46+
47+
function isCallingWithApply(caller) {
48+
return endsWith(caller, '.apply');
49+
}
50+
4351
module.exports = {
4452
getCaller,
45-
cleanCaller
53+
cleanCaller,
54+
isCallingWithApply,
55+
isCallingWithCall
4656
};

tests/lib/rules/no-anonymous-once.js

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
const rule = require('../../../lib/rules/no-anonymous-once');
2+
const MESSAGE = rule.meta.message;
3+
const RuleTester = require('eslint').RuleTester;
4+
const ruleTester = new RuleTester({
5+
parserOptions: {
6+
ecmaVersion: 6,
7+
sourceType: 'module'
8+
}
9+
});
10+
const errors = [{
11+
message: MESSAGE
12+
}];
13+
14+
ruleTester.run('no-anonymous-once', rule, {
15+
valid: [
16+
{
17+
code: `
18+
function noop() {} ;
19+
export default Ember.Component({
20+
perform() {
21+
Ember.run.scheduleOnce('afterRender', this, noop);
22+
Ember.run.scheduleOnce('afterRender', noop);
23+
run.scheduleOnce('afterRender', this, noop);
24+
run.scheduleOnce('afterRender', noop);
25+
run.scheduleOnce.apply(this, ['afterRender', this, noop]);
26+
run.scheduleOnce.apply(this, ['afterRender', noop]);
27+
run.scheduleOnce.call(this, 'afterRender', this, noop);
28+
run.scheduleOnce.call(this, 'afterRender', noop);
29+
}
30+
});`
31+
},
32+
{
33+
code: `
34+
function noop() {} ;
35+
export default Ember.Component({
36+
perform() {
37+
Ember.run.once(this, noop);
38+
Ember.run.once(noop);
39+
run.once(this, noop);
40+
run.once(noop);
41+
run.once.apply(this, [this, noop]);
42+
run.once.apply(this, [noop]);
43+
run.once.call(this, this, noop);
44+
run.once.call(this, noop);
45+
}
46+
});`
47+
}
48+
],
49+
invalid: [
50+
{
51+
code: `
52+
export default Ember.Component({
53+
perform() {
54+
Ember.run.scheduleOnce('afterRender', () => {});
55+
}
56+
});`,
57+
errors
58+
},
59+
{
60+
code: `
61+
export default Ember.Component({
62+
perform() {
63+
Ember.run.scheduleOnce('afterRender', this, () => {});
64+
}
65+
});`,
66+
errors
67+
},
68+
{
69+
code: `
70+
export default Ember.Component({
71+
perform() {
72+
Ember.run.once(this, () => {});
73+
}
74+
});`,
75+
errors
76+
},
77+
{
78+
code: `
79+
export default Ember.Component({
80+
perform() {
81+
Ember.run.once(() => {});
82+
}
83+
});`,
84+
errors
85+
},
86+
{
87+
code: `
88+
export default Ember.Component({
89+
perform() {
90+
Ember.run.once(function () {});
91+
}
92+
});`,
93+
errors
94+
},
95+
{
96+
code: `
97+
export default Ember.Component({
98+
perform() {
99+
Ember.run.scheduleOnce('afterRender', function () {});
100+
}
101+
});`,
102+
errors
103+
}
104+
]
105+
});

0 commit comments

Comments
 (0)