Skip to content

Commit 163168c

Browse files
nlfurnissnlfurniss
committed
Rethink detecting global jquery var assignments
1 parent 26c87d8 commit 163168c

File tree

4 files changed

+111
-62
lines changed

4 files changed

+111
-62
lines changed

‎lib/rules/no-global-jquery.js

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@
22
* @fileOverview Disallow the use of global `$`.
33
*/
44
'use strict';
5-
65
const { getEmberImportBinding } = require('../utils/imports');
76
const {
8-
isJQueryCallee,
9-
isVariableAssigmentJquery,
7+
isVariableAssigmentGlobalJquery,
108
JQUERY_ALIASES
119
} = require('../utils/jquery');
1210
const { collectObjectPatternBindings } = require('../utils/destructed-binding');
@@ -34,7 +32,6 @@ module.exports = {
3432
create(context) {
3533
let emberImportBinding;
3634
let destructuredAssignment = null;
37-
let jQueryLocalName = null;
3835

3936
function isDestructured(node) {
4037
return node && node.callee && node.callee.name === destructuredAssignment;
@@ -54,13 +51,13 @@ module.exports = {
5451
},
5552

5653
VariableDeclarator(node) {
57-
jQueryLocalName = isVariableAssigmentJquery(node);
54+
if (isVariableAssigmentGlobalJquery(node)) {
55+
context.report(node, MESSAGE);
56+
}
5857
},
5958

6059
CallExpression(node) {
61-
if (jQueryLocalName && !isJQueryCallee(node, jQueryLocalName)) {
62-
return;
63-
} else if (!isDestructured(node) && isGlobalJquery(node)) {
60+
if (!isDestructured(node) && isGlobalJquery(node)) {
6461
context.report(node, MESSAGE);
6562
}
6663
}

‎lib/rules/no-jquery-methods.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
'use strict';
55

66
const { get } = require('../utils/get');
7-
const { isJQueryCallee, isVariableAssigmentJquery } = require('../utils/jquery');
7+
const { isJQueryCallee, isVariableAssigmentGlobalJquery } = require('../utils/jquery');
88

99
function getMessage(blackListName) {
1010
return `The use of jQuery's ${blackListName} method has been deprecated.`;
@@ -40,7 +40,7 @@ module.exports = {
4040
},
4141

4242
VariableDeclarator(node) {
43-
jQueryLocalName = isVariableAssigmentJquery(node);
43+
jQueryLocalName = isVariableAssigmentGlobalJquery(node);
4444
},
4545

4646
CallExpression(node) {

‎lib/utils/jquery.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ const { get } = require('../utils/get');
33
const JQUERY_ALIASES = ['$', 'jQuery'];
44

55
function isJQueryCallee(node, name = null) {
6-
const calleePropertyNameisJquery = get(node, 'object.callee.property.name') === '$';
7-
const calleeNameIsJquery = get(node, 'object.callee.name') === '$';
6+
const calleePropertyNameisJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.property.name'));
7+
const calleeNameIsJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.name'));
88
const calleeNameIsLocalJquery = get(node, 'object.callee.name') === name;
99
const ObjectNameisLocalJquery = get(node, 'object.name') === name;
1010

@@ -14,20 +14,19 @@ function isJQueryCallee(node, name = null) {
1414
|| ObjectNameisLocalJquery;
1515
}
1616

17-
function isVariableAssigmentJquery(node) {
17+
function isVariableAssigmentGlobalJquery(node) {
1818
const varDecIDName = get(node, 'id.name');
1919
// Get the name of what was assigned to the variable.
20-
const varAssignment = get(node, 'init.callee.property.name')
21-
|| get(node, 'init.property.name')
22-
|| get(node, 'init.name')
23-
|| '';
24-
const varAssignmentIsJquery = JQUERY_ALIASES.includes(varAssignment);
20+
const varAssignment = get(node, 'init.callee.property.name') || get(node, 'init.name');
21+
// A variable's callee object's name tells us whether this is a usage of Ember.$ or global $.
22+
const varCalleeObjName = get(node, 'init.callee.object.name');
23+
const varAssignmentIsGlobalJquery = JQUERY_ALIASES.includes(varAssignment) && varCalleeObjName !== 'Ember';
2524

26-
return varAssignmentIsJquery ? varDecIDName : null;
25+
return varAssignmentIsGlobalJquery ? varDecIDName : null;
2726
}
2827

2928
module.exports = {
3029
isJQueryCallee,
31-
isVariableAssigmentJquery,
30+
isVariableAssigmentGlobalJquery,
3231
JQUERY_ALIASES
3332
};

‎tests/lib/rules/no-global-jquery.js

Lines changed: 95 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,13 @@ ruleTester.run('no-global-jquery', rule, {
1111
valid: [
1212
{
1313
code: `
14-
import Ember from 'ember';
15-
const $ = Ember.$;
16-
17-
export default Ember.Component({
18-
func() {
19-
$('.class').focus();
14+
export default Component.extend({
15+
focusWithJQuery() {
16+
const find = Ember.$;
17+
find('.class').focus();
2018
}
21-
});`,
22-
parserOptions,
23-
errors: [{
24-
message: MESSAGE
25-
}]
26-
},
27-
{
28-
code: `
29-
import Ember from 'ember';
30-
const jQuery = Ember.$;
31-
32-
export default Ember.Component({
33-
func() {
34-
jQuery('.class').focus();
35-
}
36-
});`,
37-
parserOptions,
38-
errors: [{
39-
message: MESSAGE
40-
}]
19+
})`,
20+
parserOptions
4121
},
4222
{
4323
code: `
@@ -90,10 +70,7 @@ ruleTester.run('no-global-jquery', rule, {
9070
this.el = $('.test');
9171
}
9272
});`,
93-
parserOptions,
94-
errors: [{
95-
message: MESSAGE
96-
}]
73+
parserOptions
9774
},
9875
{
9976
code: `
@@ -108,10 +85,7 @@ ruleTester.run('no-global-jquery', rule, {
10885
}
10986
}
11087
});`,
111-
parserOptions,
112-
errors: [{
113-
message: MESSAGE
114-
}]
88+
parserOptions
11589
},
11690
{
11791
code: `
@@ -124,10 +98,7 @@ ruleTester.run('no-global-jquery', rule, {
12498
this.el = foo('.test');
12599
}
126100
});`,
127-
parserOptions,
128-
errors: [{
129-
message: MESSAGE
130-
}]
101+
parserOptions
131102
},
132103
{
133104
code: `
@@ -142,10 +113,7 @@ ruleTester.run('no-global-jquery', rule, {
142113
}
143114
}
144115
});`,
145-
parserOptions,
146-
errors: [{
147-
message: MESSAGE
148-
}]
116+
parserOptions
149117
}
150118
],
151119
invalid: [
@@ -200,6 +168,91 @@ ruleTester.run('no-global-jquery', rule, {
200168
errors: [{
201169
message: MESSAGE
202170
}]
171+
},
172+
{
173+
code: `
174+
const jayQuery = $;
175+
export default Ember.Component({
176+
badFun() {
177+
jayQuery('.class');
178+
}
179+
});`,
180+
parserOptions,
181+
errors: [{
182+
message: MESSAGE
183+
}]
184+
},
185+
{
186+
code: `
187+
const jayQuery = $;
188+
export default Ember.Component({
189+
badFun() {
190+
jayQuery.ajax();
191+
}
192+
});`,
193+
parserOptions,
194+
errors: [{
195+
message: MESSAGE
196+
}]
197+
},
198+
{
199+
code: `
200+
const jayQuery = jQuery;
201+
export default Ember.Component({
202+
badFun() {
203+
jayQuery('.class');
204+
}
205+
});`,
206+
parserOptions,
207+
errors: [{
208+
message: MESSAGE
209+
}]
210+
},
211+
{
212+
code: `
213+
const jayQuery = jQuery;
214+
export default Ember.Component({
215+
badFun() {
216+
jayQuery.ajax();
217+
}
218+
});`,
219+
parserOptions,
220+
errors: [{
221+
message: MESSAGE
222+
}]
223+
},
224+
{
225+
code: `
226+
export default Ember.Component({
227+
func() {
228+
let a = jQuery;
229+
let b = 'notDollarSign';
230+
231+
a('.find-me').click();
232+
}
233+
});`,
234+
parserOptions,
235+
errors: [{
236+
message: MESSAGE
237+
}]
238+
},
239+
{
240+
code: `
241+
export default Component.extend({
242+
doStuffWithJQuery() {
243+
let find = jQuery;
244+
find('.class').focus();
245+
},
246+
247+
nonJQueryStuff() {
248+
let find = document.querySelector;
249+
find('.class').focus();
250+
}
251+
})`,
252+
parserOptions,
253+
errors: [{
254+
message: MESSAGE
255+
}]
203256
}
204257
]
205258
});

0 commit comments

Comments
 (0)