diff --git a/packages/core/schematics/migrations/google3/testbedTeardownRule.ts b/packages/core/schematics/migrations/google3/testbedTeardownRule.ts index b30da384d84..4277395da47 100644 --- a/packages/core/schematics/migrations/google3/testbedTeardownRule.ts +++ b/packages/core/schematics/migrations/google3/testbedTeardownRule.ts @@ -8,7 +8,7 @@ import {Replacement, RuleFailure, Rules} from 'tslint'; import ts from 'typescript'; -import {findInitTestEnvironmentCalls, findTestModuleMetadataNodes, InitTestEnvironmentAnalysis, migrateInitTestEnvironment, migrateTestModuleMetadataLiteral} from '../testbed-teardown/util'; +import {findInitTestEnvironmentCalls, findTestModuleMetadataNodes, getInitTestEnvironmentLiteralReplacement, InitTestEnvironmentAnalysis, migrateTestModuleMetadataLiteral} from '../testbed-teardown/util'; /** TSLint rule that adds the `teardown` flag to `TestBed` calls. */ export class Rule extends Rules.TypedRule { @@ -42,31 +42,31 @@ export class Rule extends Rules.TypedRule { if (initTestEnvironmentResult.totalCalls > 0) { // Migrate all of the unmigrated calls `initTestEnvironment` in this file. This could be zero // if the user has already opted into the new teardown behavior themselves. - initTestEnvironmentResult.callsToMigrate.forEach(call => { + initTestEnvironmentResult.callsToMigrate.forEach(node => { // This analysis is global so we need to check that the call is within this file. - if (call.getSourceFile() === sourceFile) { - failures.push(this._getFailure(call, migrateInitTestEnvironment, printer)); + if (node.getSourceFile() === sourceFile) { + const {span, text} = getInitTestEnvironmentLiteralReplacement(node, printer); + + failures.push(new RuleFailure( + sourceFile, span.start, span.end, 'Teardown behavior has to be configured.', + this.ruleName, new Replacement(span.start, span.length, text))); } }); } else { // Otherwise migrate the metadata passed into the `configureTestingModule` and `withModule` // calls. This scenario is less likely, but it could happen if `initTestEnvironment` has been // abstracted away or is inside a .js file. - findTestModuleMetadataNodes(typeChecker, sourceFile).forEach(literal => { - failures.push(this._getFailure(literal, migrateTestModuleMetadataLiteral, printer)); + findTestModuleMetadataNodes(typeChecker, sourceFile).forEach(node => { + const sourceFile = node.getSourceFile(); + const migrated = migrateTestModuleMetadataLiteral(node); + const replacementText = printer.printNode(ts.EmitHint.Unspecified, migrated, sourceFile); + + failures.push(new RuleFailure( + sourceFile, node.getStart(), node.getEnd(), 'Teardown behavior has to be configured.', + this.ruleName, new Replacement(node.getStart(), node.getWidth(), replacementText))); }); } return failures; } - - private _getFailure(node: T, migrator: (node: T) => T, printer: ts.Printer) { - const sourceFile = node.getSourceFile(); - const migrated = migrator(node); - const replacementText = printer.printNode(ts.EmitHint.Unspecified, migrated, sourceFile); - - return new RuleFailure( - sourceFile, node.getStart(), node.getEnd(), 'Teardown behavior has to be configured.', - this.ruleName, new Replacement(node.getStart(), node.getWidth(), replacementText)); - } } diff --git a/packages/core/schematics/migrations/testbed-teardown/index.ts b/packages/core/schematics/migrations/testbed-teardown/index.ts index 898ae1613ac..e741614e7b6 100644 --- a/packages/core/schematics/migrations/testbed-teardown/index.ts +++ b/packages/core/schematics/migrations/testbed-teardown/index.ts @@ -12,7 +12,7 @@ import ts from 'typescript'; import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; import {canMigrateFile, createMigrationProgram} from '../../utils/typescript/compiler_host'; -import {findInitTestEnvironmentCalls, findTestModuleMetadataNodes, migrateInitTestEnvironment, migrateTestModuleMetadataLiteral} from './util'; +import {findInitTestEnvironmentCalls, findTestModuleMetadataNodes, getInitTestEnvironmentLiteralReplacement, migrateTestModuleMetadataLiteral} from './util'; /** Migration that adds the `teardown` flag to `TestBed` calls. */ @@ -48,28 +48,30 @@ function runTestbedTeardownMigration(tree: Tree, tsconfigPath: string, basePath: if (initTestEnvironmentResult.totalCalls > 0) { // Migrate all of the unmigrated calls `initTestEnvironment`. This could be zero // if the user has already opted into the new teardown behavior themselves. - initTestEnvironmentResult.callsToMigrate.forEach(call => { - migrate(call, migrateInitTestEnvironment, tree, basePath, printer); + initTestEnvironmentResult.callsToMigrate.forEach(node => { + const {span, text} = getInitTestEnvironmentLiteralReplacement(node, printer); + const update = tree.beginUpdate(relative(basePath, node.getSourceFile().fileName)); + // The update appears to break if we try to call `remove` with a zero length. + if (span.length > 0) { + update.remove(span.start, span.length); + } + update.insertRight(span.start, text); + tree.commitUpdate(update); }); } else { // Otherwise migrate the metadata passed into the `configureTestingModule` and `withModule` // calls. This scenario is less likely, but it could happen if `initTestEnvironment` has been // abstracted away or is inside a .js file. sourceFiles.forEach(sourceFile => { - findTestModuleMetadataNodes(typeChecker, sourceFile).forEach(literal => { - migrate(literal, migrateTestModuleMetadataLiteral, tree, basePath, printer); + findTestModuleMetadataNodes(typeChecker, sourceFile).forEach(node => { + const migrated = migrateTestModuleMetadataLiteral(node); + const update = tree.beginUpdate(relative(basePath, node.getSourceFile().fileName)); + update.remove(node.getStart(), node.getWidth()); + update.insertRight( + node.getStart(), + printer.printNode(ts.EmitHint.Unspecified, migrated, node.getSourceFile())); + tree.commitUpdate(update); }); }); } } - - -function migrate( - node: T, migrator: (node: T) => T, tree: Tree, basePath: string, printer: ts.Printer) { - const migrated = migrator(node); - const update = tree.beginUpdate(relative(basePath, node.getSourceFile().fileName)); - update.remove(node.getStart(), node.getWidth()); - update.insertRight( - node.getStart(), printer.printNode(ts.EmitHint.Unspecified, migrated, node.getSourceFile())); - tree.commitUpdate(update); -} diff --git a/packages/core/schematics/migrations/testbed-teardown/util.ts b/packages/core/schematics/migrations/testbed-teardown/util.ts index 3a062afcb98..ac59492f07c 100644 --- a/packages/core/schematics/migrations/testbed-teardown/util.ts +++ b/packages/core/schematics/migrations/testbed-teardown/util.ts @@ -82,26 +82,45 @@ export function findTestModuleMetadataNodes( return sortInReverseSourceOrder(Array.from(testModuleMetadataLiterals)); } -/** Migrates a call to `TestBed.initTestEnvironment`. */ -export function migrateInitTestEnvironment(node: ts.CallExpression): ts.CallExpression { +/** + * Gets data that can be used to migrate a call to `TestBed.initTestEnvironment`. + * The returned `span` is used to mark the text that should be replaced while the `text` + * is the code that should be inserted instead. + */ +export function getInitTestEnvironmentLiteralReplacement( + node: ts.CallExpression, printer: ts.Printer) { const literalProperties: ts.ObjectLiteralElementLike[] = []; + const lastArg = node.arguments[node.arguments.length - 1]; + let span: {start: number, end: number, length: number}; + let prefix: string; if (node.arguments.length > 2) { - if (isFunction(node.arguments[2])) { + if (isFunction(lastArg)) { // If the last argument is a function, add the function as the `aotSummaries` property. - literalProperties.push(ts.createPropertyAssignment('aotSummaries', node.arguments[2])); - } else if (ts.isObjectLiteralExpression(node.arguments[2])) { + literalProperties.push(ts.createPropertyAssignment('aotSummaries', lastArg)); + } else if (ts.isObjectLiteralExpression(lastArg)) { // If the property is an object literal, copy over all the properties. - literalProperties.push(...node.arguments[2].properties); + literalProperties.push(...lastArg.properties); } + + prefix = ''; + span = {start: lastArg.getStart(), end: lastArg.getEnd(), length: lastArg.getWidth()}; + } else { + const start = lastArg.getEnd(); + prefix = ', '; + span = {start, end: start, length: 0}; } // Finally push the teardown object so that it appears last. literalProperties.push(createTeardownAssignment()); - return ts.createCall( - node.expression, node.typeArguments, - [...node.arguments.slice(0, 2), ts.createObjectLiteral(literalProperties, true)]); + return { + span, + text: prefix + + printer.printNode( + ts.EmitHint.Unspecified, ts.createObjectLiteral(literalProperties, true), + node.getSourceFile()) + }; } /** Migrates an object literal that is passed into `configureTestingModule` or `withModule`. */ diff --git a/packages/core/schematics/test/google3/testbed_teardown_spec.ts b/packages/core/schematics/test/google3/testbed_teardown_spec.ts index 79d45f84b75..876efece807 100644 --- a/packages/core/schematics/test/google3/testbed_teardown_spec.ts +++ b/packages/core/schematics/test/google3/testbed_teardown_spec.ts @@ -704,4 +704,32 @@ describe('Google3 TestBed teardown TSLint rule', () => { })); `)); }); + + it('should not duplicate comments on initTestEnvironment calls', () => { + writeFile('/index.ts', ` + import { TestBed } from '@angular/core/testing'; + import { + BrowserDynamicTestingModule, + platformBrowserDynamicTesting + } from '@angular/platform-browser-dynamic/testing'; + + // Hello + TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting()); + `); + + runTSLint(true); + + expect(stripWhitespace(getFile('/index.ts'))).toContain(stripWhitespace(` + import { TestBed } from '@angular/core/testing'; + import { + BrowserDynamicTestingModule, + platformBrowserDynamicTesting + } from '@angular/platform-browser-dynamic/testing'; + + // Hello + TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), { + teardown: { destroyAfterEach: false } + }); + `)); + }); }); diff --git a/packages/core/schematics/test/testbed_teardown_spec.ts b/packages/core/schematics/test/testbed_teardown_spec.ts index 4448fcc2581..7048aceef2a 100644 --- a/packages/core/schematics/test/testbed_teardown_spec.ts +++ b/packages/core/schematics/test/testbed_teardown_spec.ts @@ -649,6 +649,33 @@ describe('TestBed teardown migration', () => { `)); }); + it('should not duplicate comments on initTestEnvironment calls', async () => { + writeFile('/index.ts', ` + import { TestBed } from '@angular/core/testing'; + import { + BrowserDynamicTestingModule, + platformBrowserDynamicTesting + } from '@angular/platform-browser-dynamic/testing'; + + // Hello + TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting()); + `); + + await runMigration(); + + expect(stripWhitespace(tree.readContent('/index.ts'))).toContain(stripWhitespace(` + import { TestBed } from '@angular/core/testing'; + import { + BrowserDynamicTestingModule, + platformBrowserDynamicTesting + } from '@angular/platform-browser-dynamic/testing'; + + // Hello + TestBed.initTestEnvironment(BrowserDynamicTestingModule, platformBrowserDynamicTesting(), { + teardown: { destroyAfterEach: false } + }); + `)); + }); function writeFile(filePath: string, contents: string) { host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents));