From 4d93d2406ff1e71b25916fa88ea182adda91cffa Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Thu, 18 Jul 2019 21:05:32 +0100 Subject: [PATCH] feat(ivy): ngcc - support ngcc "migrations" (#31544) This commit implements support for the ngcc migrations as designed in https://hackmd.io/KhyrFV1VQHmeQsgfJq6AyQ PR Close #31544 --- packages/compiler-cli/ngcc/BUILD.bazel | 1 + .../ngcc/src/analysis/decoration_analyzer.ts | 98 ++++------ .../ngcc/src/analysis/migration_host.ts | 81 ++++++++ .../compiler-cli/ngcc/src/analysis/util.ts | 67 +++++++ .../ngcc/src/migrations/README.md | 18 ++ .../ngcc/src/migrations/migration.ts | 45 +++++ packages/compiler-cli/ngcc/test/BUILD.bazel | 1 + .../test/analysis/decoration_analyzer_spec.ts | 82 +++++++- .../ngcc/test/analysis/migration_host_spec.ts | 179 ++++++++++++++++++ .../src/ngtsc/diagnostics/src/code.ts | 5 + 10 files changed, 504 insertions(+), 73 deletions(-) create mode 100644 packages/compiler-cli/ngcc/src/analysis/migration_host.ts create mode 100644 packages/compiler-cli/ngcc/src/migrations/README.md create mode 100644 packages/compiler-cli/ngcc/src/migrations/migration.ts create mode 100644 packages/compiler-cli/ngcc/test/analysis/migration_host_spec.ts diff --git a/packages/compiler-cli/ngcc/BUILD.bazel b/packages/compiler-cli/ngcc/BUILD.bazel index 00c6a2daca..ba7e40f4eb 100644 --- a/packages/compiler-cli/ngcc/BUILD.bazel +++ b/packages/compiler-cli/ngcc/BUILD.bazel @@ -14,6 +14,7 @@ ts_library( "//packages/compiler", "//packages/compiler-cli/src/ngtsc/annotations", "//packages/compiler-cli/src/ngtsc/cycles", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/metadata", diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index 38e5910046..bfc8abc24b 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -9,17 +9,21 @@ import {ConstantPool} from '@angular/compiler'; import * as ts from 'typescript'; import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations'; import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles'; +import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics'; import {FileSystem, LogicalFileSystem, absoluteFrom, dirname, resolve} from '../../../src/ngtsc/file_system'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../../src/ngtsc/imports'; import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from '../../../src/ngtsc/metadata'; import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator'; -import {ClassDeclaration, ClassSymbol, Decorator} from '../../../src/ngtsc/reflection'; +import {ClassSymbol} from '../../../src/ngtsc/reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../src/ngtsc/scope'; import {CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform'; import {NgccReflectionHost} from '../host/ngcc_host'; +import {Migration, MigrationHost} from '../migrations/migration'; import {EntryPointBundle} from '../packages/entry_point_bundle'; import {isDefined} from '../utils'; -import {AnalyzedClass, AnalyzedFile, CompiledClass, CompiledFile, DecorationAnalyses, MatchingHandler} from './types'; +import {DefaultMigrationHost} from './migration_host'; +import {AnalyzedClass, AnalyzedFile, CompiledClass, CompiledFile, DecorationAnalyses} from './types'; +import {analyzeDecorators, isWithinPackage} from './util'; /** * Simple class that resolves and loads files directly from the filesystem. @@ -89,10 +93,12 @@ export class DecorationAnalyzer { this.reflectionHost, this.evaluator, this.metaRegistry, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore), ]; + migrations: Migration[] = []; constructor( private fs: FileSystem, private bundle: EntryPointBundle, - private reflectionHost: NgccReflectionHost, private referencesRegistry: ReferencesRegistry) {} + private reflectionHost: NgccReflectionHost, private referencesRegistry: ReferencesRegistry, + private diagnosticHandler: (error: ts.Diagnostic) => void = () => {}) {} /** * Analyze a program to find all the decorated files should be transformed. @@ -101,12 +107,15 @@ export class DecorationAnalyzer { */ analyzeProgram(): DecorationAnalyses { const decorationAnalyses = new DecorationAnalyses(); - const analysedFiles = this.program.getSourceFiles() + const analyzedFiles = this.program.getSourceFiles() .filter(sourceFile => isWithinPackage(this.packagePath, sourceFile)) .map(sourceFile => this.analyzeFile(sourceFile)) .filter(isDefined); - analysedFiles.forEach(analysedFile => this.resolveFile(analysedFile)); - const compiledFiles = analysedFiles.map(analysedFile => this.compileFile(analysedFile)); + const migrationHost = new DefaultMigrationHost( + this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers, analyzedFiles); + analyzedFiles.forEach(analyzedFile => this.migrateFile(migrationHost, analyzedFile)); + analyzedFiles.forEach(analyzedFile => this.resolveFile(analyzedFile)); + const compiledFiles = analyzedFiles.map(analyzedFile => this.compileFile(analyzedFile)); compiledFiles.forEach( compiledFile => decorationAnalyses.set(compiledFile.sourceFile, compiledFile)); return decorationAnalyses; @@ -120,61 +129,27 @@ export class DecorationAnalyzer { } protected analyzeClass(symbol: ClassSymbol): AnalyzedClass|null { - const declaration = symbol.valueDeclaration; const decorators = this.reflectionHost.getDecoratorsOfSymbol(symbol); - const matchingHandlers = this.handlers - .map(handler => { - const detected = handler.detect(declaration, decorators); - return {handler, detected}; - }) - .filter(isMatchingHandler); + return analyzeDecorators(symbol, decorators, this.handlers); + } - if (matchingHandlers.length === 0) { - return null; - } - const detections: {handler: DecoratorHandler, detected: DetectResult}[] = []; - let hasWeakHandler: boolean = false; - let hasNonWeakHandler: boolean = false; - let hasPrimaryHandler: boolean = false; - - for (const {handler, detected} of matchingHandlers) { - if (hasNonWeakHandler && handler.precedence === HandlerPrecedence.WEAK) { - continue; - } else if (hasWeakHandler && handler.precedence !== HandlerPrecedence.WEAK) { - // Clear all the WEAK handlers from the list of matches. - detections.length = 0; - } - if (hasPrimaryHandler && handler.precedence === HandlerPrecedence.PRIMARY) { - throw new Error(`TODO.Diagnostic: Class has multiple incompatible Angular decorators.`); - } - - detections.push({handler, detected}); - if (handler.precedence === HandlerPrecedence.WEAK) { - hasWeakHandler = true; - } else if (handler.precedence === HandlerPrecedence.SHARED) { - hasNonWeakHandler = true; - } else if (handler.precedence === HandlerPrecedence.PRIMARY) { - hasNonWeakHandler = true; - hasPrimaryHandler = true; - } - } - - const matches: {handler: DecoratorHandler, analysis: any}[] = []; - const allDiagnostics: ts.Diagnostic[] = []; - for (const {handler, detected} of detections) { - const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata); - if (diagnostics !== undefined) { - allDiagnostics.push(...diagnostics); - } - matches.push({handler, analysis}); - } - return { - name: symbol.name, - declaration, - decorators, - matches, - diagnostics: allDiagnostics.length > 0 ? allDiagnostics : undefined - }; + protected migrateFile(migrationHost: MigrationHost, analyzedFile: AnalyzedFile): void { + analyzedFile.analyzedClasses.forEach(({declaration}) => { + this.migrations.forEach(migration => { + try { + const result = migration.apply(declaration, migrationHost); + if (result !== null) { + this.diagnosticHandler(result); + } + } catch (e) { + if (isFatalDiagnosticError(e)) { + this.diagnosticHandler(e.toDiagnostic()); + } else { + throw e; + } + } + }); + }); } protected compileFile(analyzedFile: AnalyzedFile): CompiledFile { @@ -209,8 +184,3 @@ export class DecorationAnalyzer { }); } } - -function isMatchingHandler(handler: Partial>): - handler is MatchingHandler { - return !!handler.detected; -} diff --git a/packages/compiler-cli/ngcc/src/analysis/migration_host.ts b/packages/compiler-cli/ngcc/src/analysis/migration_host.ts new file mode 100644 index 0000000000..b575300318 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/analysis/migration_host.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import * as ts from 'typescript'; +import {ErrorCode, FatalDiagnosticError} from '../../../src/ngtsc/diagnostics'; +import {MetadataReader} from '../../../src/ngtsc/metadata'; +import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator'; +import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection'; +import {DecoratorHandler} from '../../../src/ngtsc/transform'; +import {NgccReflectionHost} from '../host/ngcc_host'; +import {MigrationHost} from '../migrations/migration'; +import {AnalyzedClass, AnalyzedFile} from './types'; +import {analyzeDecorators} from './util'; + +/** + * The standard implementation of `MigrationHost`, which is created by the + * `DecorationAnalyzer`. + */ +export class DefaultMigrationHost implements MigrationHost { + constructor( + readonly reflectionHost: NgccReflectionHost, readonly metadata: MetadataReader, + readonly evaluator: PartialEvaluator, private handlers: DecoratorHandler[], + private analyzedFiles: AnalyzedFile[]) {} + + injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void { + const classSymbol = this.reflectionHost.getClassSymbol(clazz) !; + const newAnalyzedClass = analyzeDecorators(classSymbol, [decorator], this.handlers); + if (newAnalyzedClass === null) { + return; + } + + const analyzedFile = getOrCreateAnalyzedFile(this.analyzedFiles, clazz.getSourceFile()); + const oldAnalyzedClass = analyzedFile.analyzedClasses.find(c => c.declaration === clazz); + if (oldAnalyzedClass === undefined) { + analyzedFile.analyzedClasses.push(newAnalyzedClass); + } else { + mergeAnalyzedClasses(oldAnalyzedClass, newAnalyzedClass); + } + } +} + +function getOrCreateAnalyzedFile( + analyzedFiles: AnalyzedFile[], sourceFile: ts.SourceFile): AnalyzedFile { + const analyzedFile = analyzedFiles.find(file => file.sourceFile === sourceFile); + if (analyzedFile !== undefined) { + return analyzedFile; + } else { + const newAnalyzedFile: AnalyzedFile = {sourceFile, analyzedClasses: []}; + analyzedFiles.push(newAnalyzedFile); + return newAnalyzedFile; + } +} + +function mergeAnalyzedClasses(oldClass: AnalyzedClass, newClass: AnalyzedClass) { + if (newClass.decorators !== null) { + if (oldClass.decorators === null) { + oldClass.decorators = newClass.decorators; + } else { + for (const newDecorator of newClass.decorators) { + if (oldClass.decorators.some(d => d.name === newDecorator.name)) { + throw new FatalDiagnosticError( + ErrorCode.NGCC_MIGRATION_DECORATOR_INJECTION_ERROR, newClass.declaration, + `Attempted to inject "${newDecorator.name}" decorator over a pre-existing decorator with the same name on the "${newClass.name}" class.`); + } + } + oldClass.decorators.push(...newClass.decorators); + } + } + + if (newClass.diagnostics !== undefined) { + if (oldClass.diagnostics === undefined) { + oldClass.diagnostics = newClass.diagnostics; + } else { + oldClass.diagnostics.push(...newClass.diagnostics); + } + } +} diff --git a/packages/compiler-cli/ngcc/src/analysis/util.ts b/packages/compiler-cli/ngcc/src/analysis/util.ts index b4117ad913..63e854a720 100644 --- a/packages/compiler-cli/ngcc/src/analysis/util.ts +++ b/packages/compiler-cli/ngcc/src/analysis/util.ts @@ -7,7 +7,74 @@ */ import * as ts from 'typescript'; import {AbsoluteFsPath, absoluteFromSourceFile, relative} from '../../../src/ngtsc/file_system'; +import {ClassSymbol, Decorator} from '../../../src/ngtsc/reflection'; +import {DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform'; +import {AnalyzedClass, MatchingHandler} from './types'; export function isWithinPackage(packagePath: AbsoluteFsPath, sourceFile: ts.SourceFile): boolean { return !relative(packagePath, absoluteFromSourceFile(sourceFile)).startsWith('..'); } + +export function analyzeDecorators( + symbol: ClassSymbol, decorators: Decorator[] | null, + handlers: DecoratorHandler[]): AnalyzedClass|null { + const declaration = symbol.valueDeclaration; + const matchingHandlers = handlers + .map(handler => { + const detected = handler.detect(declaration, decorators); + return {handler, detected}; + }) + .filter(isMatchingHandler); + + if (matchingHandlers.length === 0) { + return null; + } + const detections: {handler: DecoratorHandler, detected: DetectResult}[] = []; + let hasWeakHandler: boolean = false; + let hasNonWeakHandler: boolean = false; + let hasPrimaryHandler: boolean = false; + + for (const {handler, detected} of matchingHandlers) { + if (hasNonWeakHandler && handler.precedence === HandlerPrecedence.WEAK) { + continue; + } else if (hasWeakHandler && handler.precedence !== HandlerPrecedence.WEAK) { + // Clear all the WEAK handlers from the list of matches. + detections.length = 0; + } + if (hasPrimaryHandler && handler.precedence === HandlerPrecedence.PRIMARY) { + throw new Error(`TODO.Diagnostic: Class has multiple incompatible Angular decorators.`); + } + + detections.push({handler, detected}); + if (handler.precedence === HandlerPrecedence.WEAK) { + hasWeakHandler = true; + } else if (handler.precedence === HandlerPrecedence.SHARED) { + hasNonWeakHandler = true; + } else if (handler.precedence === HandlerPrecedence.PRIMARY) { + hasNonWeakHandler = true; + hasPrimaryHandler = true; + } + } + + const matches: {handler: DecoratorHandler, analysis: any}[] = []; + const allDiagnostics: ts.Diagnostic[] = []; + for (const {handler, detected} of detections) { + const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata); + if (diagnostics !== undefined) { + allDiagnostics.push(...diagnostics); + } + matches.push({handler, analysis}); + } + return { + name: symbol.name, + declaration, + decorators, + matches, + diagnostics: allDiagnostics.length > 0 ? allDiagnostics : undefined + }; +} + +function isMatchingHandler(handler: Partial>): + handler is MatchingHandler { + return !!handler.detected; +} diff --git a/packages/compiler-cli/ngcc/src/migrations/README.md b/packages/compiler-cli/ngcc/src/migrations/README.md new file mode 100644 index 0000000000..62b2a0e057 --- /dev/null +++ b/packages/compiler-cli/ngcc/src/migrations/README.md @@ -0,0 +1,18 @@ +# ngcc migrations + +There are some cases where source code needs to be migrated before ngtsc can compile it correctly. + +For example, there are cases where ngtsc expects directives need to be explicitly attached to +classes, whereas previously they were not required. + +There are two ways this can happen: + +1) in a project being developed, the code can be migrated via a CLI schematic. +2) in a package already published to npm, the code can be migrated as part of the ngcc compilation. + +To create one of these migrations for ngcc, you should implement the `Migration` interface and add +an instance of the class to the `DecorationAnalyzer.migrations` collection. + +This folder is where we keep the `Migration` interface and the implemented migrations. + +Each migration should have a unit test stored in the `../../test/migrations` directory. \ No newline at end of file diff --git a/packages/compiler-cli/ngcc/src/migrations/migration.ts b/packages/compiler-cli/ngcc/src/migrations/migration.ts new file mode 100644 index 0000000000..3854684eef --- /dev/null +++ b/packages/compiler-cli/ngcc/src/migrations/migration.ts @@ -0,0 +1,45 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import * as ts from 'typescript'; +import {MetadataReader} from '../../../src/ngtsc/metadata'; +import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator'; +import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection'; +import {NgccReflectionHost} from '../host/ngcc_host'; + +/** + * Implement this interface and add it to the `DecorationAnalyzer.migrations` collection to get ngcc + * to modify the analysis of the decorators in the program in order to migrate older code to work + * with Ivy. + * + * `Migration.apply()` is called for every class in the program being compiled by ngcc. + * + * Note that the underlying program could be in a variety of different formats, e.g. ES2015, ES5, + * UMD, CommonJS etc. This means that an author of a `Migration` should not attempt to navigate and + * manipulate the AST nodes directly. Instead, the `MigrationHost` interface, passed to the + * `Migration`, provides access to a `MetadataReader`, `ReflectionHost` and `PartialEvaluator` + * interfaces, which should be used. + */ +export interface Migration { + apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null; +} + +export interface MigrationHost { + /** Provides access to the decorator information associated with classes. */ + readonly metadata: MetadataReader; + /** Provides access to navigate the AST in a format-agnostic manner. */ + readonly reflectionHost: NgccReflectionHost; + /** Enables expressions to be statically evaluated in the context of the program. */ + readonly evaluator: PartialEvaluator; + /** + * Associate a new synthesized decorator, which did not appear in the original source, with a + * given class. + * @param clazz the class to receive the new decorator. + * @param decorator the decorator to inject. + */ + injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void; +} diff --git a/packages/compiler-cli/ngcc/test/BUILD.bazel b/packages/compiler-cli/ngcc/test/BUILD.bazel index 5e120e0871..54d2264902 100644 --- a/packages/compiler-cli/ngcc/test/BUILD.bazel +++ b/packages/compiler-cli/ngcc/test/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( deps = [ "//packages/compiler-cli/ngcc", "//packages/compiler-cli/ngcc/test/helpers", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", "//packages/compiler-cli/src/ngtsc/imports", diff --git a/packages/compiler-cli/ngcc/test/analysis/decoration_analyzer_spec.ts b/packages/compiler-cli/ngcc/test/analysis/decoration_analyzer_spec.ts index 47b71fe136..0ea23efb78 100644 --- a/packages/compiler-cli/ngcc/test/analysis/decoration_analyzer_spec.ts +++ b/packages/compiler-cli/ngcc/test/analysis/decoration_analyzer_spec.ts @@ -7,15 +7,17 @@ */ import * as ts from 'typescript'; +import {FatalDiagnosticError, makeDiagnostic} from '../../../src/ngtsc/diagnostics'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system'; import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; -import {Decorator} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection'; import {DecoratorHandler, DetectResult} from '../../../src/ngtsc/transform'; import {loadFakeCore, loadTestFiles} from '../../../test/helpers'; import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer'; import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; import {CompiledClass, DecorationAnalyses} from '../../src/analysis/types'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; +import {Migration, MigrationHost} from '../../src/migrations/migration'; import {MockLogger} from '../helpers/mock_logger'; import {getRootFiles, makeTestEntryPointBundle} from '../helpers/utils'; @@ -31,6 +33,8 @@ runInEachFileSystem(() => { describe('analyzeProgram()', () => { let logs: string[]; + let migrationLogs: string[]; + let diagnosticLogs: ts.Diagnostic[]; let program: ts.Program; let testHandler: jasmine.SpyObj; let result: DecorationAnalyses; @@ -87,7 +91,7 @@ runInEachFileSystem(() => { return handler; }; - function setUpAndAnalyzeProgram(testFiles: TestFile[]) { + function setUpAnalyzer(testFiles: TestFile[]) { logs = []; loadTestFiles(testFiles); loadFakeCore(getFileSystem()); @@ -99,11 +103,17 @@ runInEachFileSystem(() => { const reflectionHost = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); const referencesRegistry = new NgccReferencesRegistry(reflectionHost); - const analyzer = - new DecorationAnalyzer(getFileSystem(), bundle, reflectionHost, referencesRegistry); + diagnosticLogs = []; + const analyzer = new DecorationAnalyzer( + getFileSystem(), bundle, reflectionHost, referencesRegistry, + (error) => diagnosticLogs.push(error)); testHandler = createTestHandler(); analyzer.handlers = [testHandler]; - result = analyzer.analyzeProgram(); + migrationLogs = []; + const migration1 = new MockMigration('migration1', migrationLogs); + const migration2 = new MockMigration('migration2', migrationLogs); + analyzer.migrations = [migration1, migration2]; + return analyzer; } describe('basic usage', () => { @@ -144,7 +154,8 @@ runInEachFileSystem(() => { `, }, ]; - setUpAndAnalyzeProgram(TEST_PROGRAM); + const analyzer = setUpAnalyzer(TEST_PROGRAM); + result = analyzer.analyzeProgram(); }); it('should return an object containing a reference to the original source file', () => { @@ -185,6 +196,18 @@ runInEachFileSystem(() => { } as unknown as CompiledClass)); }); + it('should call `apply()` on each migration for each class', () => { + expect(migrationLogs).toEqual([ + 'migration1:MyComponent', + 'migration2:MyComponent', + 'migration1:MyDirective', + 'migration2:MyDirective', + 'migration1:MyOtherComponent', + 'migration2:MyOtherComponent', + ]); + }); + + it('should analyze, resolve and compile the classes that are detected', () => { expect(logs).toEqual([ // Classes without decorators should also be detected. @@ -238,7 +261,8 @@ runInEachFileSystem(() => { isRoot: false, } ]; - setUpAndAnalyzeProgram(INTERNAL_COMPONENT_PROGRAM); + const analyzer = setUpAnalyzer(INTERNAL_COMPONENT_PROGRAM); + result = analyzer.analyzeProgram(); }); // The problem of exposing the type of these internal components in the .d.ts typing @@ -299,7 +323,8 @@ runInEachFileSystem(() => { }, ]; - setUpAndAnalyzeProgram(EXTERNAL_COMPONENT_PROGRAM); + const analyzer = setUpAnalyzer(EXTERNAL_COMPONENT_PROGRAM); + result = analyzer.analyzeProgram(); }); it('should ignore classes from an externally imported file', () => { @@ -307,6 +332,45 @@ runInEachFileSystem(() => { expect(result.has(file)).toBe(false); }); }); + + describe('diagnostic handling', () => { + it('should report migration diagnostics to the `diagnosticHandler` callback', () => { + const analyzer = setUpAnalyzer([ + { + name: _('/node_modules/test-package/index.js'), + contents: ` + import {Component, Directive, Injectable} from '@angular/core'; + export class MyComponent {} + MyComponent.decorators = [{type: Component}]; + `, + }, + ]); + analyzer.migrations = [ + { + apply(clazz: ClassDeclaration) { + return makeDiagnostic(9999, clazz, 'normal diagnostic'); + } + }, + { + apply(clazz: ClassDeclaration) { + throw new FatalDiagnosticError(6666, clazz, 'fatal diagnostic'); + } + } + ]; + analyzer.analyzeProgram(); + expect(diagnosticLogs.length).toEqual(2); + expect(diagnosticLogs[0]).toEqual(jasmine.objectContaining({code: -999999})); + expect(diagnosticLogs[1]).toEqual(jasmine.objectContaining({code: -996666})); + }); + }); }); }); -}); \ No newline at end of file +}); + +class MockMigration implements Migration { + constructor(private name: string, private log: string[]) {} + apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null { + this.log.push(`${this.name}:${clazz.name.text}`); + return null; + } +} \ No newline at end of file diff --git a/packages/compiler-cli/ngcc/test/analysis/migration_host_spec.ts b/packages/compiler-cli/ngcc/test/analysis/migration_host_spec.ts new file mode 100644 index 0000000000..d827fb7ed8 --- /dev/null +++ b/packages/compiler-cli/ngcc/test/analysis/migration_host_spec.ts @@ -0,0 +1,179 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +import {ErrorCode} from '../../../src/ngtsc/diagnostics'; +import {ClassDeclaration, ClassSymbol, Decorator} from '../../../src/ngtsc/reflection'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform'; +import {DefaultMigrationHost} from '../../src/analysis/migration_host'; +import {AnalyzedClass, AnalyzedFile} from '../../src/analysis/types'; + +describe('DefaultMigrationHost', () => { + describe('injectSyntheticDecorator()', () => { + const mockHost: any = { + getClassSymbol: (node: any): ClassSymbol | undefined => + ({ valueDeclaration: node, name: node.name.text } as any), + }; + const mockMetadata: any = {}; + const mockEvaluator: any = {}; + const mockClazz: any = { + name: {text: 'MockClazz'}, + getSourceFile: () => { fileName: 'test-file.js'; }, + }; + const mockDecorator: any = {name: 'MockDecorator'}; + + it('should call `detect()` on each of the provided handlers', () => { + const log: string[] = []; + const handler1 = new TestHandler('handler1', log); + const handler2 = new TestHandler('handler2', log); + const host = + new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler1, handler2], []); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(log).toEqual([ + `handler1:detect:MockClazz:MockDecorator`, + `handler2:detect:MockClazz:MockDecorator`, + ]); + }); + + it('should call `analyze()` on each of the provided handlers whose `detect()` call returns a result', + () => { + const log: string[] = []; + const handler1 = new TestHandler('handler1', log); + const handler2 = new AlwaysDetectHandler('handler2', log); + const handler3 = new TestHandler('handler3', log); + const host = new DefaultMigrationHost( + mockHost, mockMetadata, mockEvaluator, [handler1, handler2, handler3], []); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(log).toEqual([ + `handler1:detect:MockClazz:MockDecorator`, + `handler2:detect:MockClazz:MockDecorator`, + `handler3:detect:MockClazz:MockDecorator`, + 'handler2:analyze:MockClazz', + ]); + }); + + it('should add a newly `AnalyzedFile` to the `analyzedFiles` object', () => { + const log: string[] = []; + const handler = new AlwaysDetectHandler('handler', log); + const analyzedFiles: AnalyzedFile[] = []; + const host = + new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(analyzedFiles.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses[0].name).toEqual('MockClazz'); + }); + + it('should add a newly `AnalyzedClass` to an existing `AnalyzedFile` object', () => { + const DUMMY_CLASS_1: any = {}; + const DUMMY_CLASS_2: any = {}; + const log: string[] = []; + const handler = new AlwaysDetectHandler('handler', log); + const analyzedFiles: AnalyzedFile[] = [{ + sourceFile: mockClazz.getSourceFile(), + analyzedClasses: [DUMMY_CLASS_1, DUMMY_CLASS_2], + }]; + const host = + new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(analyzedFiles.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses.length).toEqual(3); + expect(analyzedFiles[0].analyzedClasses[2].name).toEqual('MockClazz'); + }); + + it('should add a new decorator into an already existing `AnalyzedClass`', () => { + const analyzedClass: AnalyzedClass = { + name: 'MockClazz', + declaration: mockClazz, + matches: [], + decorators: null, + }; + const log: string[] = []; + const handler = new AlwaysDetectHandler('handler', log); + const analyzedFiles: AnalyzedFile[] = [{ + sourceFile: mockClazz.getSourceFile(), + analyzedClasses: [analyzedClass], + }]; + const host = + new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(analyzedFiles.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass); + expect(analyzedClass.decorators !.length).toEqual(1); + expect(analyzedClass.decorators ![0].name).toEqual('MockDecorator'); + }); + + it('should merge a new decorator into pre-existing decorators an already existing `AnalyzedClass`', + () => { + const analyzedClass: AnalyzedClass = { + name: 'MockClazz', + declaration: mockClazz, + matches: [], + decorators: [{name: 'OtherDecorator'} as Decorator], + }; + const log: string[] = []; + const handler = new AlwaysDetectHandler('handler', log); + const analyzedFiles: AnalyzedFile[] = [{ + sourceFile: mockClazz.getSourceFile(), + analyzedClasses: [analyzedClass], + }]; + const host = new DefaultMigrationHost( + mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles); + host.injectSyntheticDecorator(mockClazz, mockDecorator); + expect(analyzedFiles.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses.length).toEqual(1); + expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass); + expect(analyzedClass.decorators !.length).toEqual(2); + expect(analyzedClass.decorators ![1].name).toEqual('MockDecorator'); + }); + + it('should throw an error if the injected decorator already exists', () => { + const analyzedClass: AnalyzedClass = { + name: 'MockClazz', + declaration: mockClazz, + matches: [], + decorators: [{name: 'MockDecorator'} as Decorator], + }; + const log: string[] = []; + const handler = new AlwaysDetectHandler('handler', log); + const analyzedFiles: AnalyzedFile[] = [{ + sourceFile: mockClazz.getSourceFile(), + analyzedClasses: [analyzedClass], + }]; + const host = + new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles); + expect(() => host.injectSyntheticDecorator(mockClazz, mockDecorator)) + .toThrow( + jasmine.objectContaining({code: ErrorCode.NGCC_MIGRATION_DECORATOR_INJECTION_ERROR})); + }); + }); +}); + +class TestHandler implements DecoratorHandler { + constructor(protected name: string, protected log: string[]) {} + + precedence = HandlerPrecedence.PRIMARY; + detect(node: ClassDeclaration, decorators: Decorator[]|null): DetectResult|undefined { + this.log.push(`${this.name}:detect:${node.name.text}:${decorators !.map(d => d.name)}`); + return undefined; + } + analyze(node: ClassDeclaration): AnalysisOutput { + this.log.push(this.name + ':analyze:' + node.name.text); + return {}; + } + compile(node: ClassDeclaration): CompileResult|CompileResult[] { + this.log.push(this.name + ':compile:' + node.name.text); + return []; + } +} + +class AlwaysDetectHandler extends TestHandler { + detect(node: ClassDeclaration, decorators: Decorator[]|null): DetectResult|undefined { + super.detect(node, decorators); + return {trigger: node, metadata: {}}; + } +} diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts index ee2ca86614..9446a94ee3 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts @@ -56,6 +56,11 @@ export enum ErrorCode { * otherwise imported. */ NGMODULE_INVALID_REEXPORT = 6004, + + /** + * Raised when ngcc tries to inject a synthetic decorator over one that already exists. + */ + NGCC_MIGRATION_DECORATOR_INJECTION_ERROR = 7001, } export function ngErrorCode(code: ErrorCode): number {