Skip to content

Commit 294c350

Browse files
nlfurnissnlfurniss
committed
Add fuctionality and tests for more global jquery cases
1 parent 163168c commit 294c350

File tree

4 files changed

+210
-13
lines changed

4 files changed

+210
-13
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @fileOverview Disallow the use of global `$`.
33
*/
44
'use strict';
5+
const { get } = require('../utils/get');
56
const { getEmberImportBinding } = require('../utils/imports');
67
const {
78
isVariableAssigmentGlobalJquery,
@@ -32,6 +33,7 @@ module.exports = {
3233
create(context) {
3334
let emberImportBinding;
3435
let destructuredAssignment = null;
36+
let emberJqueryVariables = [];
3537

3638
function isDestructured(node) {
3739
return node && node.callee && node.callee.name === destructuredAssignment;
@@ -53,11 +55,15 @@ module.exports = {
5355
VariableDeclarator(node) {
5456
if (isVariableAssigmentGlobalJquery(node)) {
5557
context.report(node, MESSAGE);
58+
} else {
59+
const varName = get(node, 'id.name');
60+
emberJqueryVariables.push(varName);
5661
}
5762
},
5863

5964
CallExpression(node) {
60-
if (!isDestructured(node) && isGlobalJquery(node)) {
65+
const calleeName = get(node, 'callee.name');
66+
if (!isDestructured(node) && isGlobalJquery(node) && !emberJqueryVariables.includes(calleeName)) {
6167
context.report(node, MESSAGE);
6268
}
6369
}

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

Lines changed: 4 additions & 3 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, isVariableAssigmentGlobalJquery } = require('../utils/jquery');
7+
const { isJQueryCallee, isVariableAssigmentJquery } = require('../utils/jquery');
88

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

4242
VariableDeclarator(node) {
43-
jQueryLocalName = isVariableAssigmentGlobalJquery(node);
43+
const varName = get(node, 'id.name');
44+
jQueryLocalName = isVariableAssigmentJquery(node) ? varName : null;
4445
},
4546

4647
CallExpression(node) {
4748
const calleeName = get(node, 'callee.property.name');
4849
const parentCalleeName = get(node, 'callee.object.property.name');
49-
if (BLACKLIST.includes(calleeName) && parentCalleeName === '$') {
50+
if (BLACKLIST.includes(calleeName) && (parentCalleeName === '$' || isJQueryCallee(node, jQueryLocalName))) {
5051
context.report({ node: node.callee.property, message: getMessage(`${parentCalleeName}.${calleeName}()`) });
5152
}
5253
},

‎lib/utils/jquery.js

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,69 @@ function isJQueryCallee(node, name = null) {
66
const calleePropertyNameisJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.property.name'));
77
const calleeNameIsJquery = JQUERY_ALIASES.includes(get(node, 'object.callee.name'));
88
const calleeNameIsLocalJquery = get(node, 'object.callee.name') === name;
9-
const ObjectNameisLocalJquery = get(node, 'object.name') === name;
9+
const objectNameisLocalJquery = get(node, 'object.name') === name;
1010

1111
return calleePropertyNameisJquery
1212
|| calleeNameIsJquery
1313
|| calleeNameIsLocalJquery
14-
|| ObjectNameisLocalJquery;
14+
|| objectNameisLocalJquery;
1515
}
1616

17-
function isVariableAssigmentGlobalJquery(node) {
18-
const varDecIDName = get(node, 'id.name');
17+
/**
18+
* Helper function that returns the name of what was assigned to the variable.
19+
* @param {ASTNode} node
20+
* @return {String}
21+
*/
22+
function getVarAssignmentName(node) {
1923
// Get the name of what was assigned to the variable.
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';
24+
return get(node, 'init.callee.property.name') || get(node, 'init.name');
25+
}
26+
27+
/**
28+
* Determines whether a variable assignment's callee is Ember/this.
29+
* @param {ASTNode} node
30+
* @return {Boolean}
31+
*/
32+
function isVarAssignmentCalleeEmber(node) {
33+
// A variable's callee object's name tells us whether this is a usage of Ember.$ or global $.
34+
const isVarCalleeObjNameEmber = get(node, 'init.callee.object.name') === 'Ember';
35+
// Whether or not the callee oject is `this.$`.
36+
const isVarCalleeObjThis = get(node, 'init.callee.object.type') === 'ThisExpression';
37+
38+
return isVarCalleeObjNameEmber || isVarCalleeObjThis;
39+
}
40+
41+
/**
42+
* Determines whether a variable's assignment is global jQuery.
43+
* @param {ASTNode} node
44+
* @return {Boolean}
45+
*/
46+
function isVariableAssigmentGlobalJquery(node) {
47+
return isVariableAssigmentJquery(node) && !isVarAssignmentCalleeEmber(node);
48+
}
49+
50+
/**
51+
* Determines whether a variable's assignment is Ember jQuery.
52+
* @param {ASTNode} node
53+
* @return {Boolean}
54+
*/
55+
function isVariableAssigmentEmberJquery(node) {
56+
return isVariableAssigmentJquery(node) && isVarAssignmentCalleeEmber(node);
57+
}
2458

25-
return varAssignmentIsGlobalJquery ? varDecIDName : null;
59+
/**
60+
* Determines whether a variable's assignment is Ember jQuery.
61+
* @param {ASTNode} node
62+
* @return {Boolean}
63+
*/
64+
function isVariableAssigmentJquery(node) {
65+
return JQUERY_ALIASES.includes(getVarAssignmentName(node));
2666
}
2767

2868
module.exports = {
2969
isJQueryCallee,
3070
isVariableAssigmentGlobalJquery,
71+
isVariableAssigmentEmberJquery,
72+
isVariableAssigmentJquery,
3173
JQUERY_ALIASES
3274
};

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,26 @@ const parserOptions = {
99

1010
ruleTester.run('no-global-jquery', rule, {
1111
valid: [
12+
{
13+
code: `
14+
export default Component.extend({
15+
focusWithJQuery() {
16+
const $ = Ember.$;
17+
$('.class').focus();
18+
}
19+
})`,
20+
parserOptions
21+
},
22+
{
23+
code: `
24+
export default Component.extend({
25+
focusWithJQuery() {
26+
const Jquery = Ember.$;
27+
Jquery('.class').focus();
28+
}
29+
})`,
30+
parserOptions
31+
},
1232
{
1333
code: `
1434
export default Component.extend({
@@ -19,6 +39,26 @@ ruleTester.run('no-global-jquery', rule, {
1939
})`,
2040
parserOptions
2141
},
42+
{
43+
code: `
44+
const $ = Ember.$;
45+
export default Component.extend({
46+
focusWithJQuery() {
47+
$('.class').focus();
48+
}
49+
})`,
50+
parserOptions
51+
},
52+
{
53+
code: `
54+
const jQuery = Ember.$;
55+
export default Component.extend({
56+
focusWithJQuery() {
57+
jQuery('.class').focus();
58+
}
59+
})`,
60+
parserOptions
61+
},
2262
{
2363
code: `
2464
export default Ember.Component({
@@ -59,6 +99,28 @@ ruleTester.run('no-global-jquery', rule, {
5999
});`,
60100
parserOptions
61101
},
102+
{
103+
code: `
104+
export default Ember.Component({
105+
actions: {
106+
valid4() {
107+
const elem = this.$();
108+
}
109+
}
110+
});`,
111+
parserOptions
112+
},
113+
{
114+
code: `
115+
export default Ember.Component({
116+
actions: {
117+
valid4() {
118+
const classElem = this.$('.class');
119+
}
120+
}
121+
});`,
122+
parserOptions
123+
},
62124
{
63125
code: `
64126
import Ember from 'ember';
@@ -253,6 +315,92 @@ ruleTester.run('no-global-jquery', rule, {
253315
errors: [{
254316
message: MESSAGE
255317
}]
318+
},
319+
{
320+
code: `
321+
export default Component.extend({
322+
nonJQueryStuff() {
323+
let find = document.querySelector;
324+
find('.class').focus();
325+
326+
},
327+
328+
doStuffWithJQuery() {
329+
let find = jQuery;
330+
find('.class').focus();
331+
}
332+
})`,
333+
parserOptions,
334+
errors: [{
335+
message: MESSAGE
336+
}]
337+
},
338+
{
339+
code: `
340+
let find = Ember.$;
341+
export default Component.extend({
342+
nonJQueryStuff() {
343+
let find = document.querySelector;
344+
find('.class').focus();
345+
346+
},
347+
348+
doStuffWithJQuery() {
349+
let find = jQuery;
350+
find('.class').focus();
351+
}
352+
})`,
353+
parserOptions,
354+
errors: [{
355+
message: MESSAGE
356+
}]
357+
},
358+
{
359+
code: `
360+
let find = $;
361+
export default Component.extend({
362+
nonJQueryStuff() {
363+
let find = document.querySelector;
364+
find('.class').focus();
365+
366+
},
367+
368+
doStuffWithJQuery() {
369+
find('.class').focus();
370+
}
371+
})`,
372+
parserOptions,
373+
errors: [{
374+
message: MESSAGE
375+
}]
376+
},
377+
{
378+
code: `
379+
const $ = jQuery;
380+
export default Component.extend({
381+
focusWithJQuery() {
382+
let $ = Ember.$;
383+
$('.class').focus();
384+
}
385+
})`,
386+
parserOptions,
387+
errors: [{
388+
message: MESSAGE
389+
}]
390+
},
391+
{
392+
code: `
393+
const $ = Ember.$;
394+
export default Component.extend({
395+
focusWithJQuery() {
396+
let $ = $;
397+
$('.class').focus();
398+
}
399+
})`,
400+
parserOptions,
401+
errors: [{
402+
message: MESSAGE
403+
}]
256404
}
257405
]
258406
});

0 commit comments

Comments
 (0)