Remove remaining traces of wrapComputed. (#152)

* Remove remaining traces of wrapComputed.

* Moves `setRuntimeData` _before_ `setDecorators` so that we can more
  easily setup local state in `setDecorators`
* Removes remaining references to `wrapComputed`
* Adds `.isComputed` boolean to `EOProp` class to allow us to track when
  the runtime data indicates that a given property was a computed
  property.
* Update decorator building code to avoid creating a call expression for
  decorators that are returning computed properties without arguments
  (e.g. use `@someComputedMacro` vs `@someComputedMacro()`)

* add/correct tests, fix bug where all objects/functions get turned into computeds

* Ensure @service('something') works properly.

* Process all functions but only object expressions for computed

* Get things working again, fixup some tests

Author: Robert Jackson

test/fixtures/output/app/components/test-component.js

@@ -10,6 +10,6 @@ function fullNameMacro() {
 
 @classic
 export default class TestComponentComponent extends Component {
-  @fullNameMacro
+  @fullNameMacro()
   fullName;
 }

transforms/ember-object/__testfixtures__/-mock-telemetry.json

@@ -1,6 +1,6 @@
 {
   "runtime.input": {
-    "computedProperties": ["computedMacro", "numPlusOne", "numPlusPlus"],
+    "computedProperties": ["computedMacro", "anotherMacro", "numPlusOne", "numPlusPlus", "error", "errorService"],
     "observedProperties": [],
     "observerProperties": {},
     "offProperties": { "offProp": ["prop1", "prop2"] },
@@ -9,5 +9,16 @@
     "ownProperties": [],
     "type": "EmberObject",
     "unobservedProperties": { "unobservedProp": ["prop3", "prop4"] }
+  },
+  "injecting-service.input": {
+    "computedProperties": ["something", "otherThing"],
+    "observedProperties": [],
+    "observerProperties": {},
+    "offProperties": {},
+    "overriddenActions": [],
+    "overriddenProperties": [],
+    "ownProperties": [],
+    "type": "Service",
+    "unobservedProperties": {}
   }
 }

transforms/ember-object/__testfixtures__/injecting-service.input.js

@@ -0,0 +1,6 @@
+import Service, { service as injectService } from '@ember/service';
+
+export default Service.extend({
+  something: injectService(),
+  otherThing: injectService('some-thing'),
+});

transforms/ember-object/__testfixtures__/injecting-service.output.js

@@ -0,0 +1,11 @@
+import classic from 'ember-classic-decorator';
+import Service, { service as injectService } from '@ember/service';
+
+@classic
+export default class InjectingServiceInputService extends Service {
+  @injectService()
+  something;
+
+  @injectService('some-thing')
+  otherThing;
+}

transforms/ember-object/__testfixtures__/runtime.input.js

@@ -1,6 +1,7 @@
 import RuntimeInput from 'common/runtime/input';
 import { alias } from '@ember/object/computed';
 import { computed } from '@ember/object';
+import { service } from '@ember/service';
 
 /**
  * Program comments
@@ -15,6 +16,9 @@ export default RuntimeInput.extend(MyMixin, {
   [MY_VAL]: 'val',
   queryParams: {},
 
+  error: service(),
+  errorService: service('error'),
+
   unobservedProp: null,
   offProp: null,
 
@@ -26,6 +30,11 @@ export default RuntimeInput.extend(MyMixin, {
 
   computedMacro: customMacro(),
 
+  anotherMacro: customMacroWithInput({
+    foo: 123,
+    bar: 'baz'
+  }),
+
   /**
    * Method comments
    */

transforms/ember-object/__testfixtures__/runtime.output.js

@@ -3,6 +3,7 @@ import { off, unobserves } from '@ember-decorators/object';
 import { action, computed } from '@ember/object';
 import { alias } from '@ember/object/computed';
 import RuntimeInput from 'common/runtime/input';
+import { service } from '@ember/service';
 
 /**
  * Program comments
@@ -19,6 +20,12 @@ export default class RuntimeInputEmberObject extends RuntimeInput.extend(MyMixin
   [MY_VAL] = 'val';
   queryParams = {};
 
+  @service
+  error;
+
+  @service('error')
+  errorService;
+
   @unobserves('prop3', 'prop4')
   unobservedProp;
 
@@ -33,9 +40,15 @@ export default class RuntimeInputEmberObject extends RuntimeInput.extend(MyMixin
   @alias('numPlusOne')
   numPlusPlus;
 
-  @customMacro
+  @customMacro()
   computedMacro;
 
+  @customMacroWithInput({
+    foo: 123,
+    bar: 'baz'
+  })
+  anotherMacro;
+
   /**
    * Method comments
    */

transforms/helpers/EOProp.js

@@ -16,7 +16,7 @@ const {
 class EOProp {
   constructor(eoProp) {
     this._prop = eoProp;
-    this.decoratorNames = [];
+    this.decorators = [];
     this.modifiers = [];
     this.decoratorArgs = {};
   }
@@ -27,9 +27,20 @@ class EOProp {
 
   get kind() {
     let kind = get(this._prop, 'kind');
-    if (kind === 'init' && this.hasDecorators && !this.hasMethodDecorator) {
+    let method = get(this._prop, 'method');
+
+    if (
+      kind === 'init' &&
+      this.hasDecorators &&
+      this.decorators.find(d => d.importedName === 'computed')
+    ) {
       kind = 'get';
     }
+
+    if (method || this.hasMethodDecorator) {
+      kind = 'method';
+    }
+
     return kind;
   }
 
@@ -61,6 +72,10 @@ class EOProp {
     return isClassDecoratorProp(this.name);
   }
 
+  get decoratorNames() {
+    return this.decorators.map(d => d.name);
+  }
+
   get classDecoratorName() {
     if (this.name === LAYOUT_DECORATOR_NAME && this.value.name === LAYOUT_DECORATOR_NAME) {
       return LAYOUT_DECORATOR_LOCAL_NAME;
@@ -81,20 +96,15 @@ class EOProp {
   }
 
   get hasDecorators() {
-    return this.decoratorNames.length;
+    return this.decorators.length;
   }
 
   get callExprArgs() {
     return get(this.calleeObject, 'arguments') || [];
   }
 
   get shouldRemoveLastArg() {
-    const lastArg = this.callExprArgs.slice(-1) || [];
-
-    return (
-      lastArg.length > 0 &&
-      (lastArg[0].type === 'FunctionExpression' || lastArg[0].type === 'ObjectExpression')
-    );
+    return this.kind === 'method' || this.kind === 'get';
   }
 
   get hasModifierWithArgs() {
@@ -141,14 +151,18 @@ class EOProp {
     return this.decoratorNames.includes('off');
   }
 
-  get hasWrapComputedDecorator() {
-    return this.decoratorNames.includes('wrapComputed');
-  }
-
   get hasRuntimeData() {
     return !!this.runtimeType;
   }
 
+  get hasMethodDecorator() {
+    return this.decorators.find(d => d.isMethodDecorator);
+  }
+
+  get hasMetaDecorator() {
+    return this.decorators.find(d => d.isMetaDecorator);
+  }
+
   setCallExpressionProps() {
     let calleeObject = get(this._prop, 'value');
     const modifiers = [getModifier(calleeObject)];
@@ -164,25 +178,21 @@ class EOProp {
   setDecorators(importedDecoratedProps) {
     if (this.isCallExpression) {
       this.setCallExpressionProps();
-      const { decoratorName, isMethodDecorator, isMetaDecorator, importedName } =
-        importedDecoratedProps[this.calleeName] || {};
-      if (decoratorName) {
-        this.hasMapDecorator = importedName === 'map';
-        this.hasFilterDecorator = importedName === 'filter';
-        this.hasComputedDecorator = importedName === 'computed';
-        this.hasMethodDecorator = isMethodDecorator;
-        this.hasMetaDecorator = isMetaDecorator;
-        this.decoratorNames.push(decoratorName);
+
+      if (importedDecoratedProps[this.calleeName]) {
+        this.decorators.push(importedDecoratedProps[this.calleeName]);
+      } else if (this.isComputed) {
+        this.decorators.push({ name: this.calleeName });
       }
     }
   }
 
   addBindingProps(attributeBindingsProps, classNameBindingsProps) {
     if (attributeBindingsProps[this.name]) {
-      this.decoratorNames.push('attribute');
+      this.decorators.push({ name: 'attribute' });
       this.propList = attributeBindingsProps[this.name];
     } else if (classNameBindingsProps[this.name]) {
-      this.decoratorNames.push('className');
+      this.decorators.push({ name: 'className' });
       this.propList = classNameBindingsProps[this.name];
     }
   }
@@ -201,17 +211,18 @@ class EOProp {
     if (!type) {
       return;
     }
+
     const name = this.name;
     if (Object.keys(unobservedProperties).includes(name)) {
-      this.decoratorNames.push('unobserves');
+      this.decorators.push({ name: 'unobserves' });
       this.decoratorArgs['unobserves'] = unobservedProperties[name];
     }
     if (Object.keys(offProperties).includes(name)) {
-      this.decoratorNames.push('off');
+      this.decorators.push({ name: 'off' });
       this.decoratorArgs['off'] = offProperties[name];
     }
-    if (computedProperties.includes(name) && !this.hasComputedDecorator && !this.hasMetaDecorator) {
-      this.decoratorNames.push('wrapComputed');
+    if (computedProperties.includes(name)) {
+      this.isComputed = true;
     }
     if (this.isAction) {
       this.overriddenActions = overriddenActions;

transforms/helpers/decorator-helper.js

@@ -46,30 +46,28 @@ function createCallExpressionDecorators(j, decoratorName, instanceProp) {
     return [];
   }
 
-  if (instanceProp.hasWrapComputedDecorator) {
-    return j.decorator(j.identifier(instanceProp.calleeName));
-  }
+  const decoratorArgs = instanceProp.shouldRemoveLastArg
+    ? instanceProp.callExprArgs.slice(0, -1)
+    : instanceProp.callExprArgs.slice(0);
 
-  const decoratorArgs =
-    !instanceProp.hasMapDecorator &&
-    !instanceProp.hasFilterDecorator &&
-    instanceProp.shouldRemoveLastArg
-      ? instanceProp.callExprArgs.slice(0, -1)
-      : instanceProp.callExprArgs.slice(0);
+  let decoratorExpression =
+    ['computed', 'service', 'controller'].includes(decoratorName) && decoratorArgs.length === 0
+      ? j.identifier(decoratorName)
+      : j.callExpression(j.identifier(decoratorName), decoratorArgs);
 
-  const decoratorExpr = instanceProp.modifiers.reduce(
+  decoratorExpression = instanceProp.modifiers.reduce(
     (callExpr, modifier) =>
       j.callExpression(j.memberExpression(callExpr, modifier.prop), modifier.args),
-    j.callExpression(j.identifier(decoratorName), decoratorArgs)
+    decoratorExpression
   );
 
   if (!instanceProp.modifiers.length) {
-    return j.decorator(decoratorExpr);
+    return j.decorator(decoratorExpression);
   }
 
   // If has modifiers wrap decorators in anonymous call expression
   // it transforms @computed('').readOnly() => @(computed('').readOnly())
-  return j.decorator(j.callExpression(j.identifier(''), [decoratorExpr]));
+  return j.decorator(j.callExpression(j.identifier(''), [decoratorExpression]));
 }
 
 /**

transforms/helpers/import-helper.js

@@ -21,20 +21,20 @@ function getDecoratorInfo(specifier, importPropDecoratorMap) {
   const importedName = get(specifier, 'imported.name');
   const isImportedAs = importedName !== localName;
   const isMetaDecorator = !importPropDecoratorMap;
-  let decoratorName;
+  let name;
   if (isImportedAs) {
-    decoratorName = localName;
+    name = localName;
   } else {
     if (isMetaDecorator) {
-      decoratorName = localName;
+      name = localName;
     } else {
-      decoratorName = importPropDecoratorMap[importedName];
+      name = importPropDecoratorMap[importedName];
     }
   }
 
   const isMethodDecorator = METHOD_DECORATORS.includes(importedName);
   return {
-    decoratorName,
+    name,
     importedName,
     isImportedAs,
     isMetaDecorator,

transforms/helpers/parse-helper.js

@@ -35,8 +35,8 @@ function getEmberObjectProps(j, eoExpression, importedDecoratedProps = {}, runti
     } else if (prop.name === 'attributeBindings') {
       Object.assign(attributeBindingsProps, parseBindingProps(prop.value.elements));
     } else {
-      prop.setDecorators(importedDecoratedProps);
       prop.setRuntimeData(runtimeData);
+      prop.setDecorators(importedDecoratedProps);
       instanceProps.push(prop);
     }
   });
@@ -83,7 +83,6 @@ function getDecoratorsToImportMap(instanceProps, decoratorsMap = {}) {
   return instanceProps.reduce((specs, prop) => {
     return {
       action: specs.action || prop.isAction,
-      wrapComputed: specs.wrapComputed || prop.hasWrapComputedDecorator,
       attribute: specs.attribute || prop.hasAttributeDecorator,
       className: specs.className || prop.hasClassNameDecorator,
       classNames: specs.classNames || prop.isClassNames,