Skip to content

Commit

Permalink
Fix usages of isPropOfType
Browse files Browse the repository at this point in the history
  • Loading branch information
lin-ll committed May 12, 2021
1 parent 3f5ebcd commit d8eb285
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 32 deletions.
65 changes: 49 additions & 16 deletions lib/utils/ember.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,7 @@ function isEmberModule(callee, element, module) {
function isPropOfType(node, type) {
if (types.isProperty(node) && types.isCallExpression(node.value)) {
const calleeNode = node.value.callee;
return (
(types.isIdentifier(calleeNode) && calleeNode.name === type) ||
(types.isMemberExpression(calleeNode) &&
types.isIdentifier(calleeNode.property) &&
calleeNode.property.name === type)
);
return types.isIdentifier(calleeNode) && calleeNode.name === type;
} else if ((types.isClassProperty(node) || types.isMethodDefinition(node)) && node.decorators) {
return node.decorators.some((decorator) => {
const expression = decorator.expression;
Expand Down Expand Up @@ -336,12 +331,50 @@ function isInjectedServiceProp(node, importedEmberName, importedInjectName) {
);
}

function isInjectedControllerProp(node) {
return isPropOfType(node, 'controller');
/**
* Checks if a node is a controller injection. Looks for:
* * controller()
* * Ember.inject.controller()
* @param {node} node
* @param {string} importedEmberName name that `Ember` is imported under
* @returns
*/
function isInjectedControllerProp(node, importedEmberName) {
return (
isPropOfType(node, 'controller') ||
(types.isProperty(node) &&
types.isCallExpression(node.value) &&
types.isMemberExpression(node.value.callee) &&
types.isMemberExpression(node.value.callee.object) &&
types.isIdentifier(node.value.callee.object.object) &&
node.value.callee.object.object.name === importedEmberName &&
types.isIdentifier(node.value.callee.object.property) &&
node.value.callee.object.property.name === 'inject' &&
types.isIdentifier(node.value.callee.property) &&
node.value.callee.property.name === 'controller')
);
}

function isObserverProp(node) {
return isPropOfType(node, 'observer');
/**
* Checks if a node is an observer prop. Looks for:
* * observer()
* * Ember.observer()
* @param {node} node
* @param {string} importedEmberName name that `Ember` is imported under
* @returns
*/
function isObserverProp(node, importedEmberName) {
return (
isPropOfType(node, 'observer') ||
(types.isProperty(node) &&
types.isCallExpression(node.value) &&
types.isMemberExpression(node.value.callee) &&
types.isIdentifier(node.value.callee.object) &&
node.value.callee.object.name === importedEmberName &&
types.isIdentifier(node.value.callee.property) &&
types.isIdentifier(node.value.callee.property) &&
node.value.callee.property.name === 'observer')
);
}

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

function isSingleLineFn(property) {
function isSingleLineFn(property, importedEmberName) {
return (
(types.isMethodDefinition(property) && utils.getSize(property) === 1) ||
(property.value &&
types.isCallExpression(property.value) &&
utils.getSize(property.value) === 1 &&
!isObserverProp(property) &&
(isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) ||
!isObserverProp(property, importedEmberName) &&
(isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
!types.isCallWithFunctionExpression(property.value)))
);
}

function isMultiLineFn(property) {
function isMultiLineFn(property, importedEmberName) {
return (
(types.isMethodDefinition(property) && utils.getSize(property) > 1) ||
(property.value &&
types.isCallExpression(property.value) &&
utils.getSize(property.value) > 1 &&
!isObserverProp(property) &&
(isComputedProp(property.value, 'Ember', 'computed', { includeSuffix: true }) ||
!isObserverProp(property, importedEmberName) &&
(isComputedProp(property.value, importedEmberName, 'computed', { includeSuffix: true }) ||
!types.isCallWithFunctionExpression(property.value)))
);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/utils/property-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
return 'service';
}

if (ember.isInjectedControllerProp(node)) {
if (ember.isInjectedControllerProp(node, importedEmberName)) {
return 'controller';
}

Expand Down Expand Up @@ -98,19 +98,19 @@ function determinePropertyType(node, parentType, ORDER, importedEmberName, impor
}
}

if (ember.isObserverProp(node)) {
if (ember.isObserverProp(node, importedEmberName)) {
return 'observer';
}

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

if (ember.isSingleLineFn(node)) {
if (ember.isSingleLineFn(node, importedEmberName)) {
return 'single-line-function';
}

if (ember.isMultiLineFn(node)) {
if (ember.isMultiLineFn(node, importedEmberName)) {
return 'multi-line-function';
}

Expand Down
72 changes: 60 additions & 12 deletions tests/lib/utils/ember-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,30 @@ describe('isInjectedServiceProp', () => {
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeTruthy();
});

it("should check that it's not an injected service prop with foo.service", () => {
const context = new FauxContext(`
import {inject as service} from '@ember/service';
export default Controller.extend({
currentUser: foo.service()
});
`);
const importName = context.ast.body[0].specifiers[0].local.name;
const node = context.ast.body[1].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeFalsy();
});

it("should check that it's not an injected service prop with foo.service.inject", () => {
const context = new FauxContext(`
import {inject as service} from '@ember/service';
export default Controller.extend({
currentUser: foo.service.inject()
});
`);
const importName = context.ast.body[0].specifiers[0].local.name;
const node = context.ast.body[1].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedServiceProp(node, undefined, importName)).toBeFalsy();
});

it("should check that it's not an injected service prop without the renamed import", () => {
const context = new FauxContext(`
export default Controller.extend({
Expand Down Expand Up @@ -920,16 +944,6 @@ describe('isInjectedServiceProp', () => {
expect(emberUtils.isInjectedServiceProp(node, importName, undefined)).toBeFalsy();
});

it("should check that it's not an injected service prop with foo.service.inject", () => {
const context = new FauxContext(`
export default Controller.extend({
currentUser: foo.service.inject()
});
`);
const node = context.ast.body[0].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedServiceProp(node)).toBeFalsy();
});

it("should check that it's not an injected service prop", () => {
const context = new FauxContext(`
export default Controller.extend({
Expand Down Expand Up @@ -1035,13 +1049,35 @@ describe('isInjectedControllerProp', () => {
});

it("should check if it's an injected controller prop with full import", () => {
const context = new FauxContext(`
import Ember from 'ember';
export default Controller.extend({
application: Ember.inject.controller(),
});
`);
const importName = context.ast.body[0].specifiers[0].local.name;
const node = context.ast.body[1].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedControllerProp(node, importName)).toBeTruthy();
});

it("should check if it's not an injected controller prop without full import", () => {
const context = new FauxContext(`
export default Controller.extend({
application: Ember.inject.controller(),
});
`);
const node = context.ast.body[0].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedControllerProp(node)).toBeTruthy();
expect(emberUtils.isInjectedControllerProp(node)).toBeFalsy();
});

it("should check if it's not an injected controller prop with foo.controller", () => {
const context = new FauxContext(`
export default Controller.extend({
application: foo.controller(),
});
`);
const node = context.ast.body[0].declaration.arguments[0].properties[0];
expect(emberUtils.isInjectedControllerProp(node)).toBeFalsy();
});
});

Expand Down Expand Up @@ -1194,13 +1230,25 @@ describe('isObserverProp', () => {
});

it("should check if it's an observer prop with full import", () => {
const context = new FauxContext(`
import Ember from 'ember';
export default Controller.extend({
someObserver: Ember.observer(),
});
`);
const importName = context.ast.body[0].specifiers[0].local.name;
const node = context.ast.body[1].declaration.arguments[0].properties[0];
expect(emberUtils.isObserverProp(node, importName)).toBeTruthy();
});

it("should check that it's not an observer prop without full import", () => {
const context = new FauxContext(`
export default Controller.extend({
someObserver: Ember.observer(),
});
`);
const node = context.ast.body[0].declaration.arguments[0].properties[0];
expect(emberUtils.isObserverProp(node)).toBeTruthy();
expect(emberUtils.isObserverProp(node)).toBeFalsy();
});

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

0 comments on commit d8eb285

Please sign in to comment.