diff --git a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts index 03149452bc..2224c3723a 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -14,7 +14,7 @@ import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHa import {CycleAnalyzer, ImportGraph} from '../../../ngtsc/cycles'; import {ModuleResolver, TsReferenceResolver} from '../../../ngtsc/imports'; import {PartialEvaluator} from '../../../ngtsc/partial_evaluator'; -import {CompileResult, DecoratorHandler} from '../../../ngtsc/transform'; +import {CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../ngtsc/transform'; import {DecoratedClass} from '../host/decorated_class'; import {NgccReflectionHost} from '../host/ngcc_host'; import {isDefined} from '../utils'; @@ -26,8 +26,7 @@ export interface AnalyzedFile { export interface AnalyzedClass extends DecoratedClass { diagnostics?: ts.Diagnostic[]; - handler: DecoratorHandler; - analysis: any; + matches: {handler: DecoratorHandler; analysis: any;}[]; } export interface CompiledClass extends AnalyzedClass { compilation: CompileResult[]; } @@ -43,7 +42,7 @@ export const DecorationAnalyses = Map; export interface MatchingHandler { handler: DecoratorHandler; - match: M; + detected: M; } /** @@ -118,21 +117,52 @@ export class DecorationAnalyzer { protected analyzeClass(clazz: DecoratedClass): AnalyzedClass|null { const matchingHandlers = this.handlers .map(handler => { - const match = + const detected = handler.detect(clazz.declaration, clazz.decorators); - return {handler, match}; + return {handler, detected}; }) .filter(isMatchingHandler); - if (matchingHandlers.length > 1) { - throw new Error('TODO.Diagnostic: Class has multiple Angular decorators.'); - } if (matchingHandlers.length === 0) { return null; } - const {handler, match} = matchingHandlers[0]; - const {analysis, diagnostics} = handler.analyze(clazz.declaration, match); - return {...clazz, handler, analysis, diagnostics}; + 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(clazz.declaration, detected.metadata); + if (diagnostics !== undefined) { + allDiagnostics.push(...diagnostics); + } + matches.push({handler, analysis}); + } + return {...clazz, matches, diagnostics: allDiagnostics.length > 0 ? allDiagnostics : undefined}; } protected compileFile(analyzedFile: AnalyzedFile): CompiledFile { @@ -145,15 +175,20 @@ export class DecorationAnalyzer { } protected compileClass(clazz: AnalyzedClass, constantPool: ConstantPool): CompileResult[] { - let compilation = clazz.handler.compile(clazz.declaration, clazz.analysis, constantPool); - if (!Array.isArray(compilation)) { - compilation = [compilation]; + const compilations: CompileResult[] = []; + for (const {handler, analysis} of clazz.matches) { + const result = handler.compile(clazz.declaration, analysis, constantPool); + if (Array.isArray(result)) { + compilations.push(...result); + } else { + compilations.push(result); + } } - return compilation; + return compilations; } } function isMatchingHandler(handler: Partial>): handler is MatchingHandler { - return !!handler.match; + return !!handler.detected; } diff --git a/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts b/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts index e4a145825f..889b8dbbec 100644 --- a/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; import {Decorator} from '../../../ngtsc/reflection'; -import {DecoratorHandler} from '../../../ngtsc/transform'; +import {DecoratorHandler, DetectResult} from '../../../ngtsc/transform'; import {DecorationAnalyses, DecorationAnalyzer} from '../../src/analysis/decoration_analyzer'; import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; @@ -62,15 +62,24 @@ function createTestHandler() { 'compile', ]); // Only detect the Component decorator - handler.detect.and.callFake((node: ts.Declaration, decorators: Decorator[]) => { - if (!decorators) { - return undefined; - } - return decorators.find(d => d.name === 'Component'); - }); + handler.detect.and.callFake( + (node: ts.Declaration, decorators: Decorator[]): DetectResult| undefined => { + if (!decorators) { + return undefined; + } + const metadata = decorators.find(d => d.name === 'Component'); + if (metadata === undefined) { + return undefined; + } else { + return { + metadata, + trigger: metadata.node, + }; + } + }); // The "test" analysis is just the name of the decorator being analyzed handler.analyze.and.callFake( - ((decl: ts.Declaration, dec: Decorator) => ({analysis: dec.name, diagnostics: null}))); + ((decl: ts.Declaration, dec: Decorator) => ({analysis: dec.name, diagnostics: undefined}))); // The "test" compilation result is just the name of the decorator being compiled handler.compile.and.callFake(((decl: ts.Declaration, analysis: any) => ({analysis}))); return handler; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts index 47c64bdb61..68941ae221 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {PartialEvaluator} from '../../partial_evaluator'; import {ClassMember, Decorator, ReflectionHost} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {isAngularCore} from './util'; function containsNgTopLevelDecorator(decorators: Decorator[] | null): boolean { @@ -28,8 +28,10 @@ export class BaseDefDecoratorHandler implements DecoratorHandler { constructor(private reflector: ReflectionHost, private evaluator: PartialEvaluator) {} - detect(node: ts.ClassDeclaration, decorators: Decorator[]|null): R3BaseRefDecoratorDetection - |undefined { + readonly precedence = HandlerPrecedence.WEAK; + + detect(node: ts.ClassDeclaration, decorators: Decorator[]|null): + DetectResult|undefined { if (containsNgTopLevelDecorator(decorators)) { // If the class is already decorated by @Component or @Directive let that // DecoratorHandler handle this. BaseDef is unnecessary. @@ -56,7 +58,14 @@ export class BaseDefDecoratorHandler implements } }); - return result; + if (result !== undefined) { + return { + metadata: result, + trigger: null, + }; + } else { + return undefined; + } } analyze(node: ts.ClassDeclaration, metadata: R3BaseRefDecoratorDetection): diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index d78247cda9..019c87a685 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -12,10 +12,10 @@ import * as ts from 'typescript'; import {CycleAnalyzer} from '../../cycles'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {ModuleResolver, Reference, ResolvedReference} from '../../imports'; +import {ModuleResolver, ResolvedReference} from '../../imports'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {TypeCheckContext} from '../../typecheck'; import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300'; @@ -49,13 +49,22 @@ export class ComponentDecoratorHandler implements private literalCache = new Map(); private elementSchemaRegistry = new DomElementSchemaRegistry(); + readonly precedence = HandlerPrecedence.PRIMARY; - detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { + detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult|undefined { if (!decorators) { return undefined; } - return decorators.find( + const decorator = decorators.find( decorator => decorator.name === 'Component' && (this.isCore || isAngularCore(decorator))); + if (decorator !== undefined) { + return { + trigger: decorator.node, + metadata: decorator, + }; + } else { + return undefined; + } } preanalyze(node: ts.ClassDeclaration, decorator: Decorator): Promise|undefined { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 61f2a72e2d..6a7ddfc0c2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference, ResolvedReference} from '../../imports'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {ClassMember, ClassMemberKind, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {generateSetClassMetadataCall} from './metadata'; import {SelectorScopeRegistry} from './selector_scope'; @@ -31,12 +31,22 @@ export class DirectiveDecoratorHandler implements private reflector: ReflectionHost, private evaluator: PartialEvaluator, private scopeRegistry: SelectorScopeRegistry, private isCore: boolean) {} - detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { + readonly precedence = HandlerPrecedence.PRIMARY; + + detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult|undefined { if (!decorators) { return undefined; } - return decorators.find( + const decorator = decorators.find( decorator => decorator.name === 'Directive' && (this.isCore || isAngularCore(decorator))); + if (decorator !== undefined) { + return { + trigger: decorator.node, + metadata: decorator, + }; + } else { + return undefined; + } } analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index c3434fadce..6bd05da041 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {generateSetClassMetadataCall} from './metadata'; import {getConstructorDependencies, getValidConstructorDependencies, isAngularCore, validateConstructorDependencies} from './util'; @@ -30,12 +30,22 @@ export class InjectableDecoratorHandler implements private reflector: ReflectionHost, private isCore: boolean, private strictCtorDeps: boolean) { } - detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { + readonly precedence = HandlerPrecedence.SHARED; + + detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult|undefined { if (!decorators) { return undefined; } - return decorators.find( + const decorator = decorators.find( decorator => decorator.name === 'Injectable' && (this.isCore || isAngularCore(decorator))); + if (decorator !== undefined) { + return { + trigger: decorator.node, + metadata: decorator, + }; + } else { + return undefined; + } } analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 99c2863a76..9b5cc2d361 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -14,7 +14,7 @@ import {Reference, ResolvedReference} from '../../imports'; import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection'; import {NgModuleRouteAnalyzer} from '../../routing'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {generateSetClassMetadataCall} from './metadata'; import {ReferencesRegistry} from './references_registry'; @@ -39,12 +39,22 @@ export class NgModuleDecoratorHandler implements DecoratorHandler|undefined { if (!decorators) { return undefined; } - return decorators.find( + const decorator = decorators.find( decorator => decorator.name === 'NgModule' && (this.isCore || isAngularCore(decorator))); + if (decorator !== undefined) { + return { + trigger: decorator.node, + metadata: decorator, + }; + } else { + return undefined; + } } analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index 9ebee27f00..643a0da6e4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {PartialEvaluator} from '../../partial_evaluator'; import {Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {generateSetClassMetadataCall} from './metadata'; import {SelectorScopeRegistry} from './selector_scope'; @@ -28,12 +28,22 @@ export class PipeDecoratorHandler implements DecoratorHandler|undefined { if (!decorators) { return undefined; } - return decorators.find( + const decorator = decorators.find( decorator => decorator.name === 'Pipe' && (this.isCore || isAngularCore(decorator))); + if (decorator !== undefined) { + return { + trigger: decorator.node, + metadata: decorator, + }; + } else { + return undefined; + } } analyze(clazz: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput { diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts index 42256a0670..08abb0b890 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts @@ -59,7 +59,7 @@ describe('ComponentDecoratorHandler', () => { return fail('Failed to recognize @Component'); } try { - handler.analyze(TestCmp, detected); + handler.analyze(TestCmp, detected.metadata); return fail('Analysis should have failed'); } catch (err) { if (!(err instanceof FatalDiagnosticError)) { @@ -68,7 +68,7 @@ describe('ComponentDecoratorHandler', () => { const diag = err.toDiagnostic(); expect(diag.code).toEqual(ivyCode(ErrorCode.DECORATOR_ARG_NOT_LITERAL)); expect(diag.file.fileName.endsWith('entry.ts')).toBe(true); - expect(diag.start).toBe(detected.args ![0].getStart()); + expect(diag.start).toBe(detected.metadata.args ![0].getStart()); } }); }); diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts index 5b3338859d..0ae49e9cd6 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts @@ -13,6 +13,11 @@ export enum ErrorCode { DECORATOR_ON_ANONYMOUS_CLASS = 1004, DECORATOR_UNEXPECTED = 1005, + /** + * This error code indicates that there are incompatible decorators on a type. + */ + DECORATOR_COLLISION = 1006, + VALUE_HAS_WRONG_TYPE = 1010, VALUE_NOT_LITERAL = 1011, diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index 31f146c202..ab86acdb18 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -12,6 +12,29 @@ import * as ts from 'typescript'; import {Decorator} from '../../reflection'; import {TypeCheckContext} from '../../typecheck'; +export enum HandlerPrecedence { + /** + * Handler with PRIMARY precedence cannot overlap - there can only be one on a given class. + * + * If more than one PRIMARY handler matches a class, an error is produced. + */ + PRIMARY, + + /** + * Handlers with SHARED precedence can match any class, possibly in addition to a single PRIMARY + * handler. + * + * It is not an error for a class to have any number of SHARED handlers. + */ + SHARED, + + /** + * Handlers with WEAK precedence that match a class are ignored if any handlers with stronger + * precedence match a class. + */ + WEAK, +} + /** * Provides the interface between a decorator compiler from @angular/compiler and the Typescript @@ -22,11 +45,19 @@ import {TypeCheckContext} from '../../typecheck'; * and Typescript source, invoking the decorator compiler, and returning the result. */ export interface DecoratorHandler { + /** + * The precedence of a handler controls how it interacts with other handlers that match the same + * class. + * + * See the descriptions on `HandlerPrecedence` for an explanation of the behaviors involved. + */ + readonly precedence: HandlerPrecedence; + /** * Scan a set of reflected decorators and determine if this handler is responsible for compilation * of one of them. */ - detect(node: ts.Declaration, decorators: Decorator[]|null): M|undefined; + detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult|undefined; /** @@ -63,6 +94,11 @@ export interface DecoratorHandler { |CompileResult[]; } +export interface DetectResult { + trigger: ts.Node|null; + metadata: M; +} + /** * The output of an analysis operation, consisting of possibly an arbitrary analysis object (used as * the input to code generation) and potentially diagnostics if there were errors uncovered during diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index aff334317c..7ce20e9b12 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -9,24 +9,32 @@ import {ConstantPool} from '@angular/compiler'; import * as ts from 'typescript'; -import {FatalDiagnosticError} from '../../diagnostics'; +import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ImportRewriter} from '../../imports'; -import {Decorator, ReflectionHost, reflectNameOfDeclaration} from '../../reflection'; +import {ReflectionHost, reflectNameOfDeclaration} from '../../reflection'; import {TypeCheckContext} from '../../typecheck'; +import {getSourceFile} from '../../util/src/typescript'; -import {AnalysisOutput, CompileResult, DecoratorHandler} from './api'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from './api'; import {DtsFileTransformer} from './declaration'; - +const EMPTY_ARRAY: any = []; /** * Record of an adapter which decided to emit a static field, and the analysis it performed to * prepare for that operation. */ -interface EmitFieldOperation { - adapter: DecoratorHandler; - analysis: AnalysisOutput; - metadata: M; +interface MatchedHandler { + handler: DecoratorHandler; + analyzed: AnalysisOutput|null; + detected: DetectResult; +} + +interface IvyClass { + matchedHandlers: MatchedHandler[]; + + hasWeakHandlers: boolean; + hasPrimaryHandler: boolean; } /** @@ -40,8 +48,7 @@ export class IvyCompilation { * Tracks classes which have been analyzed and found to have an Ivy decorator, and the * information recorded about them for later compilation. */ - private analysis = new Map>(); - private typeCheckMap = new Map>(); + private ivyClasses = new Map(); /** * Tracks factory information which needs to be generated. @@ -73,6 +80,83 @@ export class IvyCompilation { analyzeAsync(sf: ts.SourceFile): Promise|undefined { return this.analyze(sf, true); } + private detectHandlersForClass(node: ts.Declaration): IvyClass|null { + // The first step is to reflect the decorators. + const classDecorators = this.reflector.getDecoratorsOfDeclaration(node); + let ivyClass: IvyClass|null = null; + + // Look through the DecoratorHandlers to see if any are relevant. + for (const handler of this.handlers) { + // An adapter is relevant if it matches one of the decorators on the class. + const detected = handler.detect(node, classDecorators); + if (detected === undefined) { + // This handler didn't match. + continue; + } + + const isPrimaryHandler = handler.precedence === HandlerPrecedence.PRIMARY; + const isWeakHandler = handler.precedence === HandlerPrecedence.WEAK; + const match = { + handler, + analyzed: null, detected, + }; + + if (ivyClass === null) { + // This is the first handler to match this class. This path is a fast path through which + // most classes will flow. + ivyClass = { + matchedHandlers: [match], + hasPrimaryHandler: isPrimaryHandler, + hasWeakHandlers: isWeakHandler, + }; + this.ivyClasses.set(node, ivyClass); + } else { + // This is at least the second handler to match this class. This is a slower path that some + // classes will go through, which validates that the set of decorators applied to the class + // is valid. + + // Validate according to rules as follows: + // + // * WEAK handlers are removed if a non-WEAK handler matches. + // * Only one PRIMARY handler can match at a time. Any other PRIMARY handler matching a + // class with an existing PRIMARY handler is an error. + + if (!isWeakHandler && ivyClass.hasWeakHandlers) { + // The current handler is not a WEAK handler, but the class has other WEAK handlers. + // Remove them. + ivyClass.matchedHandlers = ivyClass.matchedHandlers.filter( + field => field.handler.precedence !== HandlerPrecedence.WEAK); + ivyClass.hasWeakHandlers = false; + } else if (isWeakHandler && !ivyClass.hasWeakHandlers) { + // The current handler is a WEAK handler, but the class has non-WEAK handlers already. + // Drop the current one. + continue; + } + + if (isPrimaryHandler && ivyClass.hasPrimaryHandler) { + // The class already has a PRIMARY handler, and another one just matched. + this._diagnostics.push({ + category: ts.DiagnosticCategory.Error, + code: Number('-99' + ErrorCode.DECORATOR_COLLISION), + file: getSourceFile(node), + start: node.getStart(undefined, false), + length: node.getWidth(), + messageText: 'Two incompatible decorators on class', + }); + this.ivyClasses.delete(node); + return null; + } + + // Otherwise, it's safe to accept the multiple decorators here. Update some of the metadata + // regarding this class. + ivyClass.matchedHandlers.push(match); + ivyClass.hasPrimaryHandler = ivyClass.hasPrimaryHandler || isPrimaryHandler; + } + } + + return ivyClass; + } + /** * Analyze a source file and produce diagnostics for it (if any). */ @@ -82,48 +166,31 @@ export class IvyCompilation { const promises: Promise[] = []; const analyzeClass = (node: ts.Declaration): void => { - // The first step is to reflect the decorators. - const classDecorators = this.reflector.getDecoratorsOfDeclaration(node); + const ivyClass = this.detectHandlersForClass(node); - // Look through the DecoratorHandlers to see if any are relevant. - this.handlers.forEach(adapter => { + // If the class has no Ivy behavior (or had errors), skip it. + if (ivyClass === null) { + return; + } - // An adapter is relevant if it matches one of the decorators on the class. - const metadata = adapter.detect(node, classDecorators); - if (metadata === undefined) { - return; - } - - const completeAnalysis = () => { - // Check for multiple decorators on the same node. Technically speaking this - // could be supported, but right now it's an error. - if (this.analysis.has(node)) { - throw new Error('TODO.Diagnostic: Class has multiple Angular decorators.'); - } - - // Run analysis on the metadata. This will produce either diagnostics, an - // analysis result, or both. + // Loop through each matched handler that needs to be analyzed and analyze it, either + // synchronously or asynchronously. + for (const match of ivyClass.matchedHandlers) { + // The analyze() function will run the analysis phase of the handler. + const analyze = () => { try { - const analysis = adapter.analyze(node, metadata); - if (analysis.analysis !== undefined) { - this.analysis.set(node, { - adapter, - analysis: analysis.analysis, - metadata: metadata, - }); - if (!!analysis.typeCheck) { - this.typeCheckMap.set(node, adapter); - } + match.analyzed = match.handler.analyze(node, match.detected.metadata); + + if (match.analyzed.diagnostics !== undefined) { + this._diagnostics.push(...match.analyzed.diagnostics); } - if (analysis.diagnostics !== undefined) { - this._diagnostics.push(...analysis.diagnostics); - } - - if (analysis.factorySymbolName !== undefined && this.sourceToFactorySymbols !== null && + if (match.analyzed.factorySymbolName !== undefined && + this.sourceToFactorySymbols !== null && this.sourceToFactorySymbols.has(sf.fileName)) { - this.sourceToFactorySymbols.get(sf.fileName) !.add(analysis.factorySymbolName); + this.sourceToFactorySymbols.get(sf.fileName) !.add(match.analyzed.factorySymbolName); } + } catch (err) { if (err instanceof FatalDiagnosticError) { this._diagnostics.push(err.toDiagnostic()); @@ -133,17 +200,25 @@ export class IvyCompilation { } }; - if (preanalyze && adapter.preanalyze !== undefined) { - const preanalysis = adapter.preanalyze(node, metadata); + // If preanalysis was requested and a preanalysis step exists, then run preanalysis. + // Otherwise, skip directly to analysis. + if (preanalyze && match.handler.preanalyze !== undefined) { + // Preanalysis might return a Promise, indicating an async operation was necessary. Or it + // might return undefined, indicating no async step was needed and analysis can proceed + // immediately. + const preanalysis = match.handler.preanalyze(node, match.detected.metadata); if (preanalysis !== undefined) { - promises.push(preanalysis.then(() => completeAnalysis())); + // Await the results of preanalysis before running analysis. + promises.push(preanalysis.then(analyze)); } else { - completeAnalysis(); + // No async preanalysis needed, skip directly to analysis. + analyze(); } } else { - completeAnalysis(); + // Not in preanalysis mode or not needed for this handler, skip directly to analysis. + analyze(); } - }); + } }; const visit = (node: ts.Node): void => { @@ -164,17 +239,23 @@ export class IvyCompilation { } resolve(): void { - this.analysis.forEach((op, decl) => { - if (op.adapter.resolve !== undefined) { - op.adapter.resolve(decl, op.analysis); + this.ivyClasses.forEach((ivyClass, node) => { + for (const match of ivyClass.matchedHandlers) { + if (match.handler.resolve !== undefined && match.analyzed !== null && + match.analyzed.analysis !== undefined) { + match.handler.resolve(node, match.analyzed.analysis); + } } }); } typeCheck(context: TypeCheckContext): void { - this.typeCheckMap.forEach((handler, node) => { - if (handler.typeCheck !== undefined) { - handler.typeCheck(context, node, this.analysis.get(node) !.analysis); + this.ivyClasses.forEach((ivyClass, node) => { + for (const match of ivyClass.matchedHandlers) { + if (match.handler.typeCheck !== undefined && match.analyzed !== null && + match.analyzed.analysis !== undefined) { + match.handler.typeCheck(context, node, match.analyzed.analysis); + } } }); } @@ -186,37 +267,59 @@ export class IvyCompilation { compileIvyFieldFor(node: ts.Declaration, constantPool: ConstantPool): CompileResult[]|undefined { // Look to see whether the original node was analyzed. If not, there's nothing to do. const original = ts.getOriginalNode(node) as ts.Declaration; - if (!this.analysis.has(original)) { + if (!this.ivyClasses.has(original)) { return undefined; } - const op = this.analysis.get(original) !; - // Run the actual compilation, which generates an Expression for the Ivy field. - let res: CompileResult|CompileResult[] = op.adapter.compile(node, op.analysis, constantPool); - if (!Array.isArray(res)) { - res = [res]; + const ivyClass = this.ivyClasses.get(original) !; + + let res: CompileResult[] = []; + + for (const match of ivyClass.matchedHandlers) { + if (match.analyzed === null || match.analyzed.analysis === undefined) { + continue; + } + + const compileMatchRes = match.handler.compile(node, match.analyzed.analysis, constantPool); + if (!Array.isArray(compileMatchRes)) { + res.push(compileMatchRes); + } else { + res.push(...compileMatchRes); + } } - // Look up the .d.ts transformer for the input file and record that a field was generated, - // which will allow the .d.ts to be transformed later. + // Look up the .d.ts transformer for the input file and record that at least one field was + // generated, which will allow the .d.ts to be transformed later. const fileName = original.getSourceFile().fileName; const dtsTransformer = this.getDtsTransformer(fileName); dtsTransformer.recordStaticField(reflectNameOfDeclaration(node) !, res); - // Return the instruction to the transformer so the field will be added. - return res; + // Return the instruction to the transformer so the fields will be added. + return res.length > 0 ? res : undefined; } /** * Lookup the `ts.Decorator` which triggered transformation of a particular class declaration. */ - ivyDecoratorFor(node: ts.Declaration): Decorator|undefined { + ivyDecoratorsFor(node: ts.Declaration): ts.Decorator[] { const original = ts.getOriginalNode(node) as ts.Declaration; - if (!this.analysis.has(original)) { - return undefined; + + if (!this.ivyClasses.has(original)) { + return EMPTY_ARRAY; + } + const ivyClass = this.ivyClasses.get(original) !; + const decorators: ts.Decorator[] = []; + + for (const match of ivyClass.matchedHandlers) { + if (match.analyzed === null || match.analyzed.analysis === undefined) { + continue; + } + if (match.detected.trigger !== null && ts.isDecorator(match.detected.trigger)) { + decorators.push(match.detected.trigger); + } } - return this.analysis.get(original) !.metadata; + return decorators; } /** diff --git a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts index c648b29198..5223ee376a 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/transform.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/transform.ts @@ -68,8 +68,7 @@ class IvyVisitor extends Visitor { node = ts.updateClassDeclaration( node, // Remove the decorator which triggered this compilation, leaving the others alone. - maybeFilterDecorator( - node.decorators, this.compilation.ivyDecoratorFor(node) !.node as ts.Decorator), + maybeFilterDecorator(node.decorators, this.compilation.ivyDecoratorsFor(node)), node.modifiers, node.name, node.typeParameters, node.heritageClauses || [], // Map over the class members and remove any Angular decorators from them. members.map(member => this._stripAngularDecorators(member))); @@ -208,11 +207,12 @@ function transformIvySourceFile( function maybeFilterDecorator( decorators: ts.NodeArray| undefined, - toRemove: ts.Decorator): ts.NodeArray|undefined { + toRemove: ts.Decorator[]): ts.NodeArray|undefined { if (decorators === undefined) { return undefined; } - const filtered = decorators.filter(dec => ts.getOriginalNode(dec) !== toRemove); + const filtered = decorators.filter( + dec => toRemove.find(decToRemove => ts.getOriginalNode(dec) === decToRemove) === undefined); if (filtered.length === 0) { return undefined; } diff --git a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts index e0816ef3b1..3ef6e50cab 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts @@ -26,3 +26,11 @@ export function isFromDtsFile(node: ts.Node): boolean { } return sf !== undefined && D_TS.test(sf.fileName); } + +export function getSourceFile(node: ts.Node): ts.SourceFile { + // In certain transformation contexts, `ts.Node.getSourceFile()` can actually return `undefined`, + // despite the type signature not allowing it. In that event, get the `ts.SourceFile` via the + // original node instead (which works). + const directSf = node.getSourceFile() as ts.SourceFile | undefined; + return directSf !== undefined ? directSf : ts.getOriginalNode(node).getSourceFile(); +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index c68ddf492d..41f8515b79 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -624,6 +624,95 @@ describe('ngtsc behavioral tests', () => { 'i0.ɵNgModuleDefWithMeta'); }); + describe('multiple decorators on classes', () => { + it('should compile @Injectable on Components, Directives, Pipes, and Modules', () => { + env.tsconfig(); + env.write('test.ts', ` + import {Component, Directive, Injectable, NgModule, Pipe} from '@angular/core'; + + @Component({selector: 'test', template: 'test'}) + @Injectable() + export class TestCmp {} + + @Directive({selector: 'test'}) + @Injectable() + export class TestDir {} + + @Pipe({name: 'test'}) + @Injectable() + export class TestPipe {} + + @NgModule({declarations: [TestCmp, TestDir, TestPipe]}) + @Injectable() + export class TestNgModule {} + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + const dtsContents = env.getContents('test.d.ts'); + + // Validate that each class has the primary definition. + expect(jsContents).toContain('TestCmp.ngComponentDef ='); + expect(jsContents).toContain('TestDir.ngDirectiveDef ='); + expect(jsContents).toContain('TestPipe.ngPipeDef ='); + expect(jsContents).toContain('TestNgModule.ngModuleDef ='); + + // Validate that each class also has an injectable definition. + expect(jsContents).toContain('TestCmp.ngInjectableDef ='); + expect(jsContents).toContain('TestDir.ngInjectableDef ='); + expect(jsContents).toContain('TestPipe.ngInjectableDef ='); + expect(jsContents).toContain('TestNgModule.ngInjectableDef ='); + + // Validate that each class's .d.ts declaration has the primary definition. + expect(dtsContents).toContain('ComponentDefWithMeta { + env.tsconfig(); + env.write('test.ts', ` + import {Component, Directive} from '@angular/core'; + + @Component({selector: 'test', template: 'test'}) + @Directive({selector: 'test'}) + class ShouldNotCompile {} + `); + + const errors = env.driveDiagnostics(); + expect(errors.length).toBe(1); + expect(errors[0].messageText).toContain('Two incompatible decorators on class'); + }); + + + + it('should leave decorators present on jit: true directives', () => { + env.tsconfig(); + env.write('test.ts', ` + import {Directive, Inject} from '@angular/core'; + + @Directive({ + selector: 'test', + jit: true, + }) + export class Test { + constructor(@Inject('foo') foo: string) {} + } + `); + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('Directive({'); + expect(jsContents).toContain('__param(0, Inject'); + }); + }); + describe('compiling invalid @Injectables', () => { describe('with strictInjectionParameters = true', () => { it('should give a compile-time error if an invalid @Injectable is used with no arguments',