Merge pull request #29 from rwjblue/avoid-rewriting-get-owner-deeply

Ensure `getOwner(this)` is only rewritten in test context.

Author: rwjblue

__testfixtures__/ember-qunit-codemod/get-owner-this.input.js

@@ -1,3 +1,4 @@
+import Service from '@ember/service';
 import { moduleFor, test } from 'ember-qunit';
 
 moduleFor('service:flash', 'Unit | Service | Flash', {
@@ -26,3 +27,28 @@ moduleFor('service:flash', 'Unit | Service | Flash', {
 test('can use Ember.getOwner(this) also', function (assert) {
   let owner = Ember.getOwner(this);
 });
+
+test('objects registered can continue to use `getOwner(this)`', function(assert) {
+  this.register('service:foo', Service.extend({
+    someMethod() {
+      let owner = getOwner(this);
+      return owner.lookup('other:thing').someMethod();
+    }
+  }));
+});
+
+moduleFor('service:flash', {
+  beforeEach() {
+    this.blah = getOwner(this).lookup('service:blah');
+    this.register('service:foo', Service.extend({
+      someMethod() {
+        let owner = getOwner(this);
+        return owner.lookup('other:thing').someMethod();
+      }
+    }));
+  }
+});
+
+test('can use getOwner(this) in beforeEach for each context', function (assert) {
+  // stuff
+});

__testfixtures__/ember-qunit-codemod/get-owner-this.output.js

@@ -1,3 +1,4 @@
+import Service from '@ember/service';
 import { module, test } from 'qunit';
 import { setupTest } from 'ember-qunit';
 
@@ -27,4 +28,31 @@ module('Unit | Service | Flash', function(hooks) {
   test('can use Ember.getOwner(this) also', function (assert) {
     let owner = this.owner;
   });
+
+  test('objects registered can continue to use `getOwner(this)`', function(assert) {
+    this.owner.register('service:foo', Service.extend({
+      someMethod() {
+        let owner = getOwner(this);
+        return owner.lookup('other:thing').someMethod();
+      }
+    }));
+  });
+});
+
+module('service:flash', function(hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(function() {
+    this.blah = this.owner.lookup('service:blah');
+    this.owner.register('service:foo', Service.extend({
+      someMethod() {
+        let owner = getOwner(this);
+        return owner.lookup('other:thing').someMethod();
+      }
+    }));
+  });
+
+  test('can use getOwner(this) in beforeEach for each context', function (assert) {
+    // stuff
+  });
 });

ember-qunit-codemod.js

@@ -179,6 +179,8 @@ module.exports = function(file, api, options) {
         let customMethodBeforeEachBody, customMethodBeforeEachExpression;
 
         options.properties.forEach(property => {
+          updateGetOwnerThisUsage(property.value);
+
           if (isLifecycleHook(property)) {
             let lifecycleStatement = j.expressionStatement(
               j.callExpression(j.memberExpression(j.identifier('hooks'), property.key), [
@@ -357,10 +359,13 @@ module.exports = function(file, api, options) {
         currentModuleCallbackBody.push(expression);
 
         let isTest = j.match(expression, { expression: { callee: { name: 'test' } } });
-        if (isTest && currentTestType === 'setupRenderingTest') {
-          processExpressionForRenderingTest(expression);
-        } else if (currentTestType === 'setupTest' && !currentHasCustomSubject) {
-          processSubject(expression, currentSubject);
+        if (isTest) {
+          updateGetOwnerThisUsage(expression.expression.arguments[1]);
+          if (currentTestType === 'setupRenderingTest') {
+            processExpressionForRenderingTest(expression);
+          } else if (currentTestType === 'setupTest' && !currentHasCustomSubject) {
+            processSubject(expression, currentSubject);
+          }
         }
       } else {
         bodyReplacement.push(expression);
@@ -452,20 +457,25 @@ module.exports = function(file, api, options) {
       });
   }
 
-  function updateGetOwnerThisUsage() {
+  function updateGetOwnerThisUsage(expression) {
+    let expressionCollection = j(expression);
     let thisDotOwner = j.memberExpression(j.thisExpression(), j.identifier('owner'));
 
-    root
+    function replacement(path) {
+      if (path.scope.node === expression) {
+        path.replace(thisDotOwner);
+      }
+    }
+
+    expressionCollection
       .find(j.CallExpression, {
         callee: {
           name: 'getOwner',
         },
       })
-      .forEach(path => {
-        path.replace(thisDotOwner);
-      });
+      .forEach(replacement);
 
-    root
+    expressionCollection
       .find(j.CallExpression, {
         callee: {
           type: 'MemberExpression',
@@ -477,9 +487,7 @@ module.exports = function(file, api, options) {
           },
         },
       })
-      .forEach(path => {
-        path.replace(thisDotOwner);
-      });
+      .forEach(replacement);
   }
 
   const printOptions = options.printOptions || { quote: 'single' };
@@ -496,7 +504,6 @@ module.exports = function(file, api, options) {
   updateLookupCalls();
   updateRegisterCalls();
   updateInjectCalls();
-  updateGetOwnerThisUsage();
 
   return root.toSource(printOptions);
 };