Skip to content

Commit 15c6f4e

Browse files
author
Robert Jackson
committed
Scope all processing to only module options and test callbacks.
This avoids leakage when expressions are *not* included within a `test` callback or a method in the `moduleFor*` options.
1 parent 487b2e3 commit 15c6f4e

1 file changed

Lines changed: 32 additions & 35 deletions

File tree

ember-qunit-codemod.js

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -126,25 +126,6 @@ module.exports = function(file, api, options) {
126126
}
127127
}
128128

129-
function updateEmberTestHelperImports(ctx) {
130-
let specifiers = new Set();
131-
132-
['render', 'clearRender'].forEach(type => {
133-
let usages = findTestHelperUsageOf(ctx, type);
134-
if (usages.size() > 0) {
135-
specifiers.add(type);
136-
}
137-
});
138-
139-
if (specifiers.size > 0) {
140-
ensureImportWithSpecifiers({
141-
source: 'ember-test-helpers',
142-
anchor: 'ember-qunit',
143-
specifiers,
144-
});
145-
}
146-
}
147-
148129
function findTestHelperUsageOf(collection, property) {
149130
return collection.find(j.ExpressionStatement, {
150131
expression: {
@@ -242,7 +223,14 @@ module.exports = function(file, api, options) {
242223
let customMethodBeforeEachBody, customMethodBeforeEachExpression;
243224

244225
options.properties.forEach(property => {
245-
updateGetOwnerThisUsage(property.value);
226+
if (setupType) {
227+
let expressionCollection = j(property.value);
228+
229+
updateGetOwnerThisUsage(expressionCollection);
230+
updateLookupCalls(expressionCollection);
231+
updateRegisterCalls(expressionCollection);
232+
updateInjectCalls(expressionCollection);
233+
}
246234

247235
if (isLifecycleHook(property)) {
248236
needsHooks = true;
@@ -316,10 +304,13 @@ module.exports = function(file, api, options) {
316304
function processExpressionForRenderingTest(testExpression) {
317305
// mark the test function as an async function
318306
let testExpressionCollection = j(testExpression);
307+
let specifiers = new Set();
319308

320309
// Transform to await render() or await clearRender()
321310
['render', 'clearRender'].forEach(type => {
322311
findTestHelperUsageOf(testExpressionCollection, type).forEach(p => {
312+
specifiers.add(type);
313+
323314
let expression = p.get('expression');
324315

325316
let awaitExpression = j.awaitExpression(
@@ -330,6 +321,12 @@ module.exports = function(file, api, options) {
330321
});
331322
});
332323

324+
ensureImportWithSpecifiers({
325+
source: 'ember-test-helpers',
326+
anchor: 'ember-qunit',
327+
specifiers,
328+
});
329+
333330
// Migrate `this._element` -> `this.element`
334331
testExpressionCollection
335332
.find(j.MemberExpression, {
@@ -419,39 +416,43 @@ module.exports = function(file, api, options) {
419416
let programPath = root.get('program');
420417
let bodyPath = programPath.get('body');
421418

422-
let bodyReplacement = [];
423419
let currentModuleCallbackBody, currentTestType, currentSubject, currentHasCustomSubject;
424420
bodyPath.each(expressionPath => {
425421
let expression = expressionPath.node;
426422
if (isModuleDefinition(expressionPath)) {
427423
let result = createModule(expressionPath);
428-
bodyReplacement.push(result[0]);
424+
expressionPath.replace(result[0]);
429425
currentModuleCallbackBody = result[1];
430426
currentTestType = result[2];
431427
currentSubject = result[3];
432428
currentHasCustomSubject = result[4];
433429
} else if (currentModuleCallbackBody) {
430+
// calling `path.replace()` essentially just removes
431+
expressionPath.replace();
434432
currentModuleCallbackBody.push(expression);
435433

436434
let isTest = j.match(expression, { expression: { callee: { name: 'test' } } });
437435
if (isTest) {
438-
updateGetOwnerThisUsage(expression.expression.arguments[1]);
436+
let expressionCollection = j(expression);
437+
updateLookupCalls(expressionCollection);
438+
updateRegisterCalls(expressionCollection);
439+
updateInjectCalls(expressionCollection);
440+
// passing the specific function callback here, because `getOwner` is only
441+
// transformed if the call site's scope is the same as the expression passed
442+
updateGetOwnerThisUsage(j(expression.expression.arguments[1]));
443+
439444
if (currentTestType === 'setupRenderingTest') {
440445
processExpressionForRenderingTest(expression);
441446
} else if (currentTestType === 'setupTest' && !currentHasCustomSubject) {
442447
processSubject(expression, currentSubject);
443448
}
444449
}
445-
} else {
446-
bodyReplacement.push(expression);
447450
}
448451
});
449-
450-
bodyPath.replace(bodyReplacement);
451452
}
452453

453-
function updateLookupCalls() {
454-
root
454+
function updateLookupCalls(ctx) {
455+
ctx
455456
.find(j.MemberExpression, {
456457
object: {
457458
object: { type: 'ThisExpression' },
@@ -539,8 +540,8 @@ module.exports = function(file, api, options) {
539540
});
540541
}
541542

542-
function updateGetOwnerThisUsage(expression) {
543-
let expressionCollection = j(expression);
543+
function updateGetOwnerThisUsage(expressionCollection) {
544+
let expression = expressionCollection.get().node;
544545
let thisDotOwner = j.memberExpression(j.thisExpression(), j.identifier('owner'));
545546

546547
function replacement(path) {
@@ -599,11 +600,7 @@ module.exports = function(file, api, options) {
599600

600601
moveQUnitImportsFromEmberQUnit();
601602
updateToNewEmberQUnitImports();
602-
updateEmberTestHelperImports();
603603
updateModuleForToNestedModule();
604-
updateLookupCalls();
605-
updateRegisterCalls();
606-
updateInjectCalls();
607604
updateWaitUsage();
608605

609606
return root.toSource(printOptions);

0 commit comments

Comments
 (0)