Skip to content

Commit d8eb285

Browse files
lin-lllin-ll
committed
Fix usages of isPropOfType
1 parent 3f5ebcd commit d8eb285

File tree

3 files changed

+113
-32
lines changed

3 files changed

+113
-32
lines changed

‎lib/utils/ember.js

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,7 @@ function isEmberModule(callee, element, module) {
115115
function isPropOfType(node, type) {
116116
if (types.isProperty(node) && types.isCallExpression(node.value)) {
117117
const calleeNode = node.value.callee;
118-
return (
119-
(types.isIdentifier(calleeNode) && calleeNode.name === type) ||
120-
(types.isMemberExpression(calleeNode) &&
121-
types.isIdentifier(calleeNode.property) &&
122-
calleeNode.property.name === type)
123-
);
118+
return types.isIdentifier(calleeNode) && calleeNode.name === type;
124119
} else if ((types.isClassProperty(node) || types.isMethodDefinition(node)) && node.decorators) {
125120
return node.decorators.some((decorator) => {
126121
const expression = decorator.expression;
@@ -336,12 +331,50 @@ function isInjectedServiceProp(node, importedEmberName, importedInjectName) {
336331
);
337332
}
338333

339-
function isInjectedControllerProp(node) {
340-
return isPropOfType(node, 'controller');
334+
/**
335+
* Checks if a node is a controller injection. Looks for:
336+
* * controller()
337+
* * Ember.inject.controller()
338+
* @param {node} node
339+
* @param {string} importedEmberName name that `Ember` is imported under
340+
* @returns
341+
*/
342+
function isInjectedControllerProp(node, importedEmberName) {
343+
return (
344+
isPropOfType(node, 'controller') ||
345+
(types.isProperty(node) &&
346+
types.isCallExpression(node.value) &&
347+
types.isMemberExpression(node.value.callee) &&
348+
types.isMemberExpression(node.value.callee.object) &&
349+
types.isIdentifier(node.value.callee.object.object) &&
350+
node.value.callee.object.object.name === importedEmberName &&
351+
types.isIdentifier(node.value.callee.object.property) &&
352+
node.value.callee.object.property.name === 'inject' &&
353+
types.isIdentifier(node.value.callee.property) &&
354+
node.value.callee.property.name === 'controller')
355+
);
341356
}
342357

343-
function isObserverProp(node) {
344-
return isPropOfType(node, 'observer');
358+
/**
359+
* Checks if a node is an observer prop. Looks for:
360+
* * observer()
361+
* * Ember.observer()
362+
* @param {node} node
363+
* @param {string} importedEmberName name that `Ember` is imported under
364+
* @returns
365+
*/
366+
function isObserverProp(node, importedEmberName) {
367+
return (
368+
isPropOfType(node, 'observer') ||
369+
(types.isProperty(node) &&
370+
types.isCallExpression(node.value) &&
371+
types.isMemberExpression(node.value.callee) &&
372+
types.isIdentifier(node.value.callee.object) &&
373+
node.value.callee.object.name === importedEmberName &&
374+
types.isIdentifier(node.value.callee.property) &&
375+
types.isIdentifier(node.value.callee.property) &&
376+
node.value.callee.property.name === 'observer')
377+
);
345378
}
346379

347380
const allowedMemberExpNames = new Set(['volatile', 'readOnly', 'property', 'meta']);
@@ -585,26 +618,26 @@ function getEmberImportAliasName(importDeclaration) {
585618
return importDeclaration.specifiers[0].local.name;
586619
}
587620

588-
function isSingleLineFn(property) {
621+
function isSingleLineFn(property, importedEmberName) {
589622
return (
590623
(types.isMethodDefinition(property) && utils.getSize(property) === 1) ||
591624
(property.value &&
592625
types.isCallExpression(property.value) &&
593626
utils.getSize(property.value) === 1 &&
594-
!isObserverProp(property) &&
595-
(isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) ||
627+
!isObserverProp(property, importedEmberName) &&
628+
(isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
596629
!types.isCallWithFunctionExpression(property.value)))
597630
);
598631
}
599632

600-
function isMultiLineFn(property) {
633+
function isMultiLineFn(property, importedEmberName) {
601634
return (
602635
(types.isMethodDefinition(property) && utils.getSize(property) > 1) ||
603636
(property.value &&
604637
types.isCallExpression(property.value) &&
605638
utils.getSize(property.value) > 1 &&
606-
!isObserverProp(property) &&
607-
(isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) ||
639+
!isObserverProp(property, importedEmberName) &&
640+
(isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
608641
!types.isCallWithFunctionExpression(property.value)))
609642
);
610643
}

‎lib/utils/property-order.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
5757
return 'service';
5858
}
5959

60-
if (ember.isInjectedControllerProp(node)) {
60+
if (ember.isInjectedControllerProp(node, importedEmberName)) {
6161
return 'controller';
6262
}
6363

@@ -98,19 +98,19 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
9898
}
9999
}
100100

101-
if (ember.isObserverProp(node)) {
101+
if (ember.isObserverProp(node, importedEmberName)) {
102102
return 'observer';
103103
}
104104

105105
if (parentType !== 'model' && ember.isActionsProp(node)) {
106106
return 'actions';
107107
}
108108

109-
if (ember.isSingleLineFn(node)) {
109+
if (ember.isSingleLineFn(node, importedEmberName)) {
110110
return 'single-line-function';
111111
}
112112

113-
if (ember.isMultiLineFn(node)) {
113+
if (ember.isMultiLineFn(node, importedEmberName)) {
114114
return 'multi-line-function';
115115
}
116116

‎tests/lib/utils/ember-test.js

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,30 @@ describe('isInjectedServiceProp', () => {
852852
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeTruthy();
853853
});
854854

855+
it("should check that it's not an injected service prop with foo.service", () => {
856+
const context = new FauxContext(`
857+
import {inject as service} from '@ember/service';
858+
export default Controller.extend({
859+
currentUser: foo.service()
860+
});
861+
`);
862+
const importName = context.ast.body[0].specifiers[0].local.name;
863+
const node = context.ast.body[1].declaration.arguments[0].properties[0];
864+
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeFalsy();
865+
});
866+
867+
it("should check that it's not an injected service prop with foo.service.inject", () => {
868+
const context = new FauxContext(`
869+
import {inject as service} from '@ember/service';
870+
export default Controller.extend({
871+
currentUser: foo.service.inject()
872+
});
873+
`);
874+
const importName = context.ast.body[0].specifiers[0].local.name;
875+
const node = context.ast.body[1].declaration.arguments[0].properties[0];
876+
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeFalsy();
877+
});
878+
855879
it("should check that it's not an injected service prop without the renamed import", () => {
856880
const context = new FauxContext(`
857881
export default Controller.extend({
@@ -920,16 +944,6 @@ describe('isInjectedServiceProp', () => {
920944
expect(emberUtils.isInjectedServiceProp(node, importName, undefined)).toBeFalsy();
921945
});
922946

923-
it("should check that it's not an injected service prop with foo.service.inject", () => {
924-
const context = new FauxContext(`
925-
export default Controller.extend({
926-
currentUser: foo.service.inject()
927-
});
928-
`);
929-
const node = context.ast.body[0].declaration.arguments[0].properties[0];
930-
expect(emberUtils.isInjectedServiceProp(node)).toBeFalsy();
931-
});
932-
933947
it("should check that it's not an injected service prop", () => {
934948
const context = new FauxContext(`
935949
export default Controller.extend({
@@ -1035,13 +1049,35 @@ describe('isInjectedControllerProp', () => {
10351049
});
10361050

10371051
it("should check if it's an injected controller prop with full import", () => {
1052+
const context = new FauxContext(`
1053+
import Ember from 'ember';
1054+
export default Controller.extend({
1055+
application: Ember.inject.controller(),
1056+
});
1057+
`);
1058+
const importName = context.ast.body[0].specifiers[0].local.name;
1059+
const node = context.ast.body[1].declaration.arguments[0].properties[0];
1060+
expect(emberUtils.isInjectedControllerProp(node, importName)).toBeTruthy();
1061+
});
1062+
1063+
it("should check if it's not an injected controller prop without full import", () => {
10381064
const context = new FauxContext(`
10391065
export default Controller.extend({
10401066
application: Ember.inject.controller(),
10411067
});
10421068
`);
10431069
const node = context.ast.body[0].declaration.arguments[0].properties[0];
1044-
expect(emberUtils.isInjectedControllerProp(node)).toBeTruthy();
1070+
expect(emberUtils.isInjectedControllerProp(node)).toBeFalsy();
1071+
});
1072+
1073+
it("should check if it's not an injected controller prop with foo.controller", () => {
1074+
const context = new FauxContext(`
1075+
export default Controller.extend({
1076+
application: foo.controller(),
1077+
});
1078+
`);
1079+
const node = context.ast.body[0].declaration.arguments[0].properties[0];
1080+
expect(emberUtils.isInjectedControllerProp(node)).toBeFalsy();
10451081
});
10461082
});
10471083

@@ -1194,13 +1230,25 @@ describe('isObserverProp', () => {
11941230
});
11951231

11961232
it("should check if it's an observer prop with full import", () => {
1233+
const context = new FauxContext(`
1234+
import Ember from 'ember';
1235+
export default Controller.extend({
1236+
someObserver: Ember.observer(),
1237+
});
1238+
`);
1239+
const importName = context.ast.body[0].specifiers[0].local.name;
1240+
const node = context.ast.body[1].declaration.arguments[0].properties[0];
1241+
expect(emberUtils.isObserverProp(node, importName)).toBeTruthy();
1242+
});
1243+
1244+
it("should check that it's not an observer prop without full import", () => {
11971245
const context = new FauxContext(`
11981246
export default Controller.extend({
11991247
someObserver: Ember.observer(),
12001248
});
12011249
`);
12021250
const node = context.ast.body[0].declaration.arguments[0].properties[0];
1203-
expect(emberUtils.isObserverProp(node)).toBeTruthy();
1251+
expect(emberUtils.isObserverProp(node)).toBeFalsy();
12041252
});
12051253

12061254
it("should check if it's an observer prop with multi-line observer", () => {

0 commit comments

Comments
 (0)