Merge pull request #50 from rwjblue/scoped-processing

Scope all processing to only module options and test callbacks.

Author: rwjblue

__testfixtures__/ember-qunit-codemod/custom-module-for-implementation.input.js

@@ -0,0 +1,11 @@
+import moduleForComponent from '../helpers/module-for-component';
+import { test } from 'ember-qunit';
+
+moduleForOtherComponent('foo-bar', 'Integration | Component | FooBar', {
+  integration: true
+});
+
+test('it does not get changed', function() {
+  this.render(hbs`derp`);
+});
+

__testfixtures__/ember-qunit-codemod/custom-module-for-implementation.output.js

@@ -0,0 +1,11 @@
+import moduleForComponent from '../helpers/module-for-component';
+import { test } from 'qunit';
+
+moduleForOtherComponent('foo-bar', 'Integration | Component | FooBar', {
+  integration: true
+});
+
+test('it does not get changed', function() {
+  this.render(hbs`derp`);
+});
+

__testfixtures__/ember-qunit-codemod/non-module-render-usage.input.js

@@ -0,0 +1,7 @@
+import someOtherThing from '../foo-bar/';
+
+// this example doesn't use this.render inside of a test block, so it should not be transformed
+// and there should be no new imports added
+someOtherThing(function() {
+  this.render('derp');
+});

__testfixtures__/ember-qunit-codemod/non-module-render-usage.output.js

@@ -0,0 +1,7 @@
+import someOtherThing from '../foo-bar/';
+
+// this example doesn't use this.render inside of a test block, so it should not be transformed
+// and there should be no new imports added
+someOtherThing(function() {
+  this.render('derp');
+});

ember-qunit-codemod.js

@@ -126,25 +126,6 @@ module.exports = function(file, api, options) {
     }
   }
 
-  function updateEmberTestHelperImports() {
-    let specifiers = new Set();
-
-    ['render', 'clearRender'].forEach(type => {
-      let usages = findTestHelperUsageOf(root, type);
-      if (usages.size() > 0) {
-        specifiers.add(type);
-      }
-    });
-
-    if (specifiers.size > 0) {
-      ensureImportWithSpecifiers({
-        source: 'ember-test-helpers',
-        anchor: 'ember-qunit',
-        specifiers,
-      });
-    }
-  }
-
   function findTestHelperUsageOf(collection, property) {
     return collection.find(j.ExpressionStatement, {
       expression: {
@@ -268,7 +249,14 @@ module.exports = function(file, api, options) {
         let customMethodBeforeEachBody, customMethodBeforeEachExpression;
 
         options.properties.forEach(property => {
-          updateGetOwnerThisUsage(property.value);
+          if (setupType) {
+            let expressionCollection = j(property.value);
+
+            updateGetOwnerThisUsage(expressionCollection);
+            updateLookupCalls(expressionCollection);
+            updateRegisterCalls(expressionCollection);
+            updateInjectCalls(expressionCollection);
+          }
 
           if (isLifecycleHook(property)) {
             needsHooks = true;
@@ -342,10 +330,13 @@ module.exports = function(file, api, options) {
     function processExpressionForRenderingTest(testExpression) {
       // mark the test function as an async function
       let testExpressionCollection = j(testExpression);
+      let specifiers = new Set();
 
       // Transform to await render() or await clearRender()
       ['render', 'clearRender'].forEach(type => {
         findTestHelperUsageOf(testExpressionCollection, type).forEach(p => {
+          specifiers.add(type);
+
           let expression = p.get('expression');
 
           let awaitExpression = j.awaitExpression(
@@ -356,6 +347,12 @@ module.exports = function(file, api, options) {
         });
       });
 
+      ensureImportWithSpecifiers({
+        source: 'ember-test-helpers',
+        anchor: 'ember-qunit',
+        specifiers,
+      });
+
       // Migrate `this._element` -> `this.element`
       testExpressionCollection
         .find(j.MemberExpression, {
@@ -445,39 +442,43 @@ module.exports = function(file, api, options) {
     let programPath = root.get('program');
     let bodyPath = programPath.get('body');
 
-    let bodyReplacement = [];
     let currentModuleCallbackBody, currentTestType, currentSubject, currentHasCustomSubject;
     bodyPath.each(expressionPath => {
       let expression = expressionPath.node;
       if (isModuleDefinition(expressionPath)) {
         let result = createModule(expressionPath);
-        bodyReplacement.push(result[0]);
+        expressionPath.replace(result[0]);
         currentModuleCallbackBody = result[1];
         currentTestType = result[2];
         currentSubject = result[3];
         currentHasCustomSubject = result[4];
       } else if (currentModuleCallbackBody) {
+        // calling `path.replace()` essentially just removes
+        expressionPath.replace();
         currentModuleCallbackBody.push(expression);
 
         let isTest = j.match(expression, { expression: { callee: { name: 'test' } } });
         if (isTest) {
-          updateGetOwnerThisUsage(expression.expression.arguments[1]);
+          let expressionCollection = j(expression);
+          updateLookupCalls(expressionCollection);
+          updateRegisterCalls(expressionCollection);
+          updateInjectCalls(expressionCollection);
+          // passing the specific function callback here, because `getOwner` is only
+          // transformed if the call site's scope is the same as the expression passed
+          updateGetOwnerThisUsage(j(expression.expression.arguments[1]));
+
           if (currentTestType === 'setupRenderingTest') {
             processExpressionForRenderingTest(expression);
           } else if (currentTestType === 'setupTest' && !currentHasCustomSubject) {
             processSubject(expression, currentSubject);
           }
         }
-      } else {
-        bodyReplacement.push(expression);
       }
     });
-
-    bodyPath.replace(bodyReplacement);
   }
 
-  function updateLookupCalls() {
-    root
+  function updateLookupCalls(ctx) {
+    ctx
       .find(j.MemberExpression, {
         object: {
           object: { type: 'ThisExpression' },
@@ -491,8 +492,8 @@ module.exports = function(file, api, options) {
       });
   }
 
-  function updateRegisterCalls() {
-    root
+  function updateRegisterCalls(ctx) {
+    ctx
       .find(j.MemberExpression, {
         object: {
           object: { type: 'ThisExpression' },
@@ -505,7 +506,7 @@ module.exports = function(file, api, options) {
         path.replace(j.memberExpression(thisDotOwner, path.value.property));
       });
 
-    root
+    ctx
       .find(j.MemberExpression, {
         object: { type: 'ThisExpression' },
         property: { name: 'register' },
@@ -516,8 +517,8 @@ module.exports = function(file, api, options) {
       });
   }
 
-  function updateInjectCalls() {
-    root
+  function updateInjectCalls(ctx) {
+    ctx
       .find(j.CallExpression, {
         callee: {
           type: 'MemberExpression',
@@ -565,8 +566,8 @@ module.exports = function(file, api, options) {
       });
   }
 
-  function updateGetOwnerThisUsage(expression) {
-    let expressionCollection = j(expression);
+  function updateGetOwnerThisUsage(expressionCollection) {
+    let expression = expressionCollection.get().node;
     let thisDotOwner = j.memberExpression(j.thisExpression(), j.identifier('owner'));
 
     function replacement(path) {
@@ -625,11 +626,7 @@ module.exports = function(file, api, options) {
 
   moveQUnitImportsFromEmberQUnit();
   updateToNewEmberQUnitImports();
-  updateEmberTestHelperImports();
   updateModuleForToNestedModule();
-  updateLookupCalls();
-  updateRegisterCalls();
-  updateInjectCalls();
   updateWaitUsage();
 
   return root.toSource(printOptions);