feat(ivy): support @Injectable on already decorated classes (#28523)

Previously, ngtsc would throw an error if two decorators were matched on
the same class simultaneously. However, @Injectable is a special case, and
it appears frequently on component, directive, and pipe classes. For pipes
in particular, it's a common pattern to treat the pipe class also as an
injectable service.

ngtsc actually lacked the capability to compile multiple matching
decorators on a class, so this commit adds support for that. Decorator
handlers (and thus the decorators they match) are classified into three
categories: PRIMARY, SHARED, and WEAK.

PRIMARY handlers compile decorators that cannot coexist with other primary
decorators. The handlers for Component, Directive, Pipe, and NgModule are
marked as PRIMARY. A class may only have one decorator from this group.

SHARED handlers compile decorators that can coexist with others. Injectable
is the only decorator in this category, meaning it's valid to put an
@Injectable decorator on a previously decorated class.

WEAK handlers behave like SHARED, but are dropped if any non-WEAK handler
matches a class. The handler which compiles ngBaseDef is WEAK, since
ngBaseDef is only needed if a class doesn't otherwise have a decorator.

Tests are added to validate that @Injectable can coexist with the other
decorators and that an error is generated when mixing the primaries.

PR Close #28523
This commit is contained in:
Alex Rickabaugh 2019-02-01 12:23:21 -08:00 committed by Misko Hevery
parent d2742cf473
commit 99d8582882
15 changed files with 466 additions and 123 deletions

View File

@ -14,7 +14,7 @@ import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHa
import {CycleAnalyzer, ImportGraph} from '../../../ngtsc/cycles'; import {CycleAnalyzer, ImportGraph} from '../../../ngtsc/cycles';
import {ModuleResolver, TsReferenceResolver} from '../../../ngtsc/imports'; import {ModuleResolver, TsReferenceResolver} from '../../../ngtsc/imports';
import {PartialEvaluator} from '../../../ngtsc/partial_evaluator'; 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 {DecoratedClass} from '../host/decorated_class';
import {NgccReflectionHost} from '../host/ngcc_host'; import {NgccReflectionHost} from '../host/ngcc_host';
import {isDefined} from '../utils'; import {isDefined} from '../utils';
@ -26,8 +26,7 @@ export interface AnalyzedFile {
export interface AnalyzedClass extends DecoratedClass { export interface AnalyzedClass extends DecoratedClass {
diagnostics?: ts.Diagnostic[]; diagnostics?: ts.Diagnostic[];
handler: DecoratorHandler<any, any>; matches: {handler: DecoratorHandler<any, any>; analysis: any;}[];
analysis: any;
} }
export interface CompiledClass extends AnalyzedClass { compilation: CompileResult[]; } export interface CompiledClass extends AnalyzedClass { compilation: CompileResult[]; }
@ -43,7 +42,7 @@ export const DecorationAnalyses = Map;
export interface MatchingHandler<A, M> { export interface MatchingHandler<A, M> {
handler: DecoratorHandler<A, M>; handler: DecoratorHandler<A, M>;
match: M; detected: M;
} }
/** /**
@ -118,21 +117,52 @@ export class DecorationAnalyzer {
protected analyzeClass(clazz: DecoratedClass): AnalyzedClass|null { protected analyzeClass(clazz: DecoratedClass): AnalyzedClass|null {
const matchingHandlers = this.handlers const matchingHandlers = this.handlers
.map(handler => { .map(handler => {
const match = const detected =
handler.detect(clazz.declaration, clazz.decorators); handler.detect(clazz.declaration, clazz.decorators);
return {handler, match}; return {handler, detected};
}) })
.filter(isMatchingHandler); .filter(isMatchingHandler);
if (matchingHandlers.length > 1) {
throw new Error('TODO.Diagnostic: Class has multiple Angular decorators.');
}
if (matchingHandlers.length === 0) { if (matchingHandlers.length === 0) {
return null; return null;
} }
const {handler, match} = matchingHandlers[0]; const detections: {handler: DecoratorHandler<any, any>, detected: DetectResult<any>}[] = [];
const {analysis, diagnostics} = handler.analyze(clazz.declaration, match); let hasWeakHandler: boolean = false;
return {...clazz, handler, analysis, diagnostics}; 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<any, any>, 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 { protected compileFile(analyzedFile: AnalyzedFile): CompiledFile {
@ -145,15 +175,20 @@ export class DecorationAnalyzer {
} }
protected compileClass(clazz: AnalyzedClass, constantPool: ConstantPool): CompileResult[] { protected compileClass(clazz: AnalyzedClass, constantPool: ConstantPool): CompileResult[] {
let compilation = clazz.handler.compile(clazz.declaration, clazz.analysis, constantPool); const compilations: CompileResult[] = [];
if (!Array.isArray(compilation)) { for (const {handler, analysis} of clazz.matches) {
compilation = [compilation]; 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<A, M>(handler: Partial<MatchingHandler<A, M>>): function isMatchingHandler<A, M>(handler: Partial<MatchingHandler<A, M>>):
handler is MatchingHandler<A, M> { handler is MatchingHandler<A, M> {
return !!handler.match; return !!handler.detected;
} }

View File

@ -8,7 +8,7 @@
import * as ts from 'typescript'; import * as ts from 'typescript';
import {Decorator} from '../../../ngtsc/reflection'; 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 {DecorationAnalyses, DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
@ -62,15 +62,24 @@ function createTestHandler() {
'compile', 'compile',
]); ]);
// Only detect the Component decorator // Only detect the Component decorator
handler.detect.and.callFake((node: ts.Declaration, decorators: Decorator[]) => { handler.detect.and.callFake(
if (!decorators) { (node: ts.Declaration, decorators: Decorator[]): DetectResult<any>| undefined => {
return undefined; if (!decorators) {
} return undefined;
return decorators.find(d => d.name === 'Component'); }
}); 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 // The "test" analysis is just the name of the decorator being analyzed
handler.analyze.and.callFake( 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 // The "test" compilation result is just the name of the decorator being compiled
handler.compile.and.callFake(((decl: ts.Declaration, analysis: any) => ({analysis}))); handler.compile.and.callFake(((decl: ts.Declaration, analysis: any) => ({analysis})));
return handler; return handler;

View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {PartialEvaluator} from '../../partial_evaluator'; import {PartialEvaluator} from '../../partial_evaluator';
import {ClassMember, Decorator, ReflectionHost} from '../../reflection'; import {ClassMember, Decorator, ReflectionHost} from '../../reflection';
import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform';
import {isAngularCore} from './util'; import {isAngularCore} from './util';
function containsNgTopLevelDecorator(decorators: Decorator[] | null): boolean { function containsNgTopLevelDecorator(decorators: Decorator[] | null): boolean {
@ -28,8 +28,10 @@ export class BaseDefDecoratorHandler implements
DecoratorHandler<R3BaseRefMetaData, R3BaseRefDecoratorDetection> { DecoratorHandler<R3BaseRefMetaData, R3BaseRefDecoratorDetection> {
constructor(private reflector: ReflectionHost, private evaluator: PartialEvaluator) {} constructor(private reflector: ReflectionHost, private evaluator: PartialEvaluator) {}
detect(node: ts.ClassDeclaration, decorators: Decorator[]|null): R3BaseRefDecoratorDetection readonly precedence = HandlerPrecedence.WEAK;
|undefined {
detect(node: ts.ClassDeclaration, decorators: Decorator[]|null):
DetectResult<R3BaseRefDecoratorDetection>|undefined {
if (containsNgTopLevelDecorator(decorators)) { if (containsNgTopLevelDecorator(decorators)) {
// If the class is already decorated by @Component or @Directive let that // If the class is already decorated by @Component or @Directive let that
// DecoratorHandler handle this. BaseDef is unnecessary. // 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): analyze(node: ts.ClassDeclaration, metadata: R3BaseRefDecoratorDetection):

View File

@ -12,10 +12,10 @@ import * as ts from 'typescript';
import {CycleAnalyzer} from '../../cycles'; import {CycleAnalyzer} from '../../cycles';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {ModuleResolver, Reference, ResolvedReference} from '../../imports'; import {ModuleResolver, ResolvedReference} from '../../imports';
import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; 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 {TypeCheckContext} from '../../typecheck';
import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300'; import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
@ -49,13 +49,22 @@ export class ComponentDecoratorHandler implements
private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>(); private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
private elementSchemaRegistry = new DomElementSchemaRegistry(); 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<Decorator>|undefined {
if (!decorators) { if (!decorators) {
return undefined; return undefined;
} }
return decorators.find( const decorator = decorators.find(
decorator => decorator.name === 'Component' && (this.isCore || isAngularCore(decorator))); 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<void>|undefined { preanalyze(node: ts.ClassDeclaration, decorator: Decorator): Promise<void>|undefined {

View File

@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {Reference, ResolvedReference} from '../../imports'; import {Reference, ResolvedReference} from '../../imports';
import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {ClassMember, ClassMemberKind, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; 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 {generateSetClassMetadataCall} from './metadata';
import {SelectorScopeRegistry} from './selector_scope'; import {SelectorScopeRegistry} from './selector_scope';
@ -31,12 +31,22 @@ export class DirectiveDecoratorHandler implements
private reflector: ReflectionHost, private evaluator: PartialEvaluator, private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private scopeRegistry: SelectorScopeRegistry, private isCore: boolean) {} 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<Decorator>|undefined {
if (!decorators) { if (!decorators) {
return undefined; return undefined;
} }
return decorators.find( const decorator = decorators.find(
decorator => decorator.name === 'Directive' && (this.isCore || isAngularCore(decorator))); 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<DirectiveHandlerData> { analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput<DirectiveHandlerData> {

View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; 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 {generateSetClassMetadataCall} from './metadata';
import {getConstructorDependencies, getValidConstructorDependencies, isAngularCore, validateConstructorDependencies} from './util'; import {getConstructorDependencies, getValidConstructorDependencies, isAngularCore, validateConstructorDependencies} from './util';
@ -30,12 +30,22 @@ export class InjectableDecoratorHandler implements
private reflector: ReflectionHost, private isCore: boolean, private strictCtorDeps: boolean) { 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<Decorator>|undefined {
if (!decorators) { if (!decorators) {
return undefined; return undefined;
} }
return decorators.find( const decorator = decorators.find(
decorator => decorator.name === 'Injectable' && (this.isCore || isAngularCore(decorator))); 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<InjectableHandlerData> { analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput<InjectableHandlerData> {

View File

@ -14,7 +14,7 @@ import {Reference, ResolvedReference} from '../../imports';
import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator';
import {Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection'; import {Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection';
import {NgModuleRouteAnalyzer} from '../../routing'; import {NgModuleRouteAnalyzer} from '../../routing';
import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform';
import {generateSetClassMetadataCall} from './metadata'; import {generateSetClassMetadataCall} from './metadata';
import {ReferencesRegistry} from './references_registry'; import {ReferencesRegistry} from './references_registry';
@ -39,12 +39,22 @@ export class NgModuleDecoratorHandler implements DecoratorHandler<NgModuleAnalys
private scopeRegistry: SelectorScopeRegistry, private referencesRegistry: ReferencesRegistry, private scopeRegistry: SelectorScopeRegistry, private referencesRegistry: ReferencesRegistry,
private isCore: boolean, private routeAnalyzer: NgModuleRouteAnalyzer|null) {} private isCore: boolean, private routeAnalyzer: NgModuleRouteAnalyzer|null) {}
detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { readonly precedence = HandlerPrecedence.PRIMARY;
detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult<Decorator>|undefined {
if (!decorators) { if (!decorators) {
return undefined; return undefined;
} }
return decorators.find( const decorator = decorators.find(
decorator => decorator.name === 'NgModule' && (this.isCore || isAngularCore(decorator))); 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<NgModuleAnalysis> { analyze(node: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput<NgModuleAnalysis> {

View File

@ -12,7 +12,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {PartialEvaluator} from '../../partial_evaluator'; import {PartialEvaluator} from '../../partial_evaluator';
import {Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; 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 {generateSetClassMetadataCall} from './metadata';
import {SelectorScopeRegistry} from './selector_scope'; import {SelectorScopeRegistry} from './selector_scope';
@ -28,12 +28,22 @@ export class PipeDecoratorHandler implements DecoratorHandler<PipeHandlerData, D
private reflector: ReflectionHost, private evaluator: PartialEvaluator, private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private scopeRegistry: SelectorScopeRegistry, private isCore: boolean) {} 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<Decorator>|undefined {
if (!decorators) { if (!decorators) {
return undefined; return undefined;
} }
return decorators.find( const decorator = decorators.find(
decorator => decorator.name === 'Pipe' && (this.isCore || isAngularCore(decorator))); 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<PipeHandlerData> { analyze(clazz: ts.ClassDeclaration, decorator: Decorator): AnalysisOutput<PipeHandlerData> {

View File

@ -59,7 +59,7 @@ describe('ComponentDecoratorHandler', () => {
return fail('Failed to recognize @Component'); return fail('Failed to recognize @Component');
} }
try { try {
handler.analyze(TestCmp, detected); handler.analyze(TestCmp, detected.metadata);
return fail('Analysis should have failed'); return fail('Analysis should have failed');
} catch (err) { } catch (err) {
if (!(err instanceof FatalDiagnosticError)) { if (!(err instanceof FatalDiagnosticError)) {
@ -68,7 +68,7 @@ describe('ComponentDecoratorHandler', () => {
const diag = err.toDiagnostic(); const diag = err.toDiagnostic();
expect(diag.code).toEqual(ivyCode(ErrorCode.DECORATOR_ARG_NOT_LITERAL)); expect(diag.code).toEqual(ivyCode(ErrorCode.DECORATOR_ARG_NOT_LITERAL));
expect(diag.file.fileName.endsWith('entry.ts')).toBe(true); 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());
} }
}); });
}); });

View File

@ -13,6 +13,11 @@ export enum ErrorCode {
DECORATOR_ON_ANONYMOUS_CLASS = 1004, DECORATOR_ON_ANONYMOUS_CLASS = 1004,
DECORATOR_UNEXPECTED = 1005, DECORATOR_UNEXPECTED = 1005,
/**
* This error code indicates that there are incompatible decorators on a type.
*/
DECORATOR_COLLISION = 1006,
VALUE_HAS_WRONG_TYPE = 1010, VALUE_HAS_WRONG_TYPE = 1010,
VALUE_NOT_LITERAL = 1011, VALUE_NOT_LITERAL = 1011,

View File

@ -12,6 +12,29 @@ import * as ts from 'typescript';
import {Decorator} from '../../reflection'; import {Decorator} from '../../reflection';
import {TypeCheckContext} from '../../typecheck'; 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 * 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. * and Typescript source, invoking the decorator compiler, and returning the result.
*/ */
export interface DecoratorHandler<A, M> { export interface DecoratorHandler<A, M> {
/**
* 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 * Scan a set of reflected decorators and determine if this handler is responsible for compilation
* of one of them. * of one of them.
*/ */
detect(node: ts.Declaration, decorators: Decorator[]|null): M|undefined; detect(node: ts.Declaration, decorators: Decorator[]|null): DetectResult<M>|undefined;
/** /**
@ -63,6 +94,11 @@ export interface DecoratorHandler<A, M> {
|CompileResult[]; |CompileResult[];
} }
export interface DetectResult<M> {
trigger: ts.Node|null;
metadata: M;
}
/** /**
* The output of an analysis operation, consisting of possibly an arbitrary analysis object (used as * 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 * the input to code generation) and potentially diagnostics if there were errors uncovered during

View File

@ -9,24 +9,32 @@
import {ConstantPool} from '@angular/compiler'; import {ConstantPool} from '@angular/compiler';
import * as ts from 'typescript'; import * as ts from 'typescript';
import {FatalDiagnosticError} from '../../diagnostics'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {ImportRewriter} from '../../imports'; import {ImportRewriter} from '../../imports';
import {Decorator, ReflectionHost, reflectNameOfDeclaration} from '../../reflection'; import {ReflectionHost, reflectNameOfDeclaration} from '../../reflection';
import {TypeCheckContext} from '../../typecheck'; 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'; 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 * Record of an adapter which decided to emit a static field, and the analysis it performed to
* prepare for that operation. * prepare for that operation.
*/ */
interface EmitFieldOperation<A, M> { interface MatchedHandler<A, M> {
adapter: DecoratorHandler<A, M>; handler: DecoratorHandler<A, M>;
analysis: AnalysisOutput<A>; analyzed: AnalysisOutput<A>|null;
metadata: M; detected: DetectResult<M>;
}
interface IvyClass {
matchedHandlers: MatchedHandler<any, any>[];
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 * Tracks classes which have been analyzed and found to have an Ivy decorator, and the
* information recorded about them for later compilation. * information recorded about them for later compilation.
*/ */
private analysis = new Map<ts.Declaration, EmitFieldOperation<any, any>>(); private ivyClasses = new Map<ts.Declaration, IvyClass>();
private typeCheckMap = new Map<ts.Declaration, DecoratorHandler<any, any>>();
/** /**
* Tracks factory information which needs to be generated. * Tracks factory information which needs to be generated.
@ -73,6 +80,83 @@ export class IvyCompilation {
analyzeAsync(sf: ts.SourceFile): Promise<void>|undefined { return this.analyze(sf, true); } analyzeAsync(sf: ts.SourceFile): Promise<void>|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). * Analyze a source file and produce diagnostics for it (if any).
*/ */
@ -82,48 +166,31 @@ export class IvyCompilation {
const promises: Promise<void>[] = []; const promises: Promise<void>[] = [];
const analyzeClass = (node: ts.Declaration): void => { const analyzeClass = (node: ts.Declaration): void => {
// The first step is to reflect the decorators. const ivyClass = this.detectHandlersForClass(node);
const classDecorators = this.reflector.getDecoratorsOfDeclaration(node);
// Look through the DecoratorHandlers to see if any are relevant. // If the class has no Ivy behavior (or had errors), skip it.
this.handlers.forEach(adapter => { if (ivyClass === null) {
return;
}
// An adapter is relevant if it matches one of the decorators on the class. // Loop through each matched handler that needs to be analyzed and analyze it, either
const metadata = adapter.detect(node, classDecorators); // synchronously or asynchronously.
if (metadata === undefined) { for (const match of ivyClass.matchedHandlers) {
return; // The analyze() function will run the analysis phase of the handler.
} const analyze = () => {
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.
try { try {
const analysis = adapter.analyze(node, metadata); match.analyzed = match.handler.analyze(node, match.detected.metadata);
if (analysis.analysis !== undefined) {
this.analysis.set(node, { if (match.analyzed.diagnostics !== undefined) {
adapter, this._diagnostics.push(...match.analyzed.diagnostics);
analysis: analysis.analysis,
metadata: metadata,
});
if (!!analysis.typeCheck) {
this.typeCheckMap.set(node, adapter);
}
} }
if (analysis.diagnostics !== undefined) { if (match.analyzed.factorySymbolName !== undefined &&
this._diagnostics.push(...analysis.diagnostics); this.sourceToFactorySymbols !== null &&
}
if (analysis.factorySymbolName !== undefined && this.sourceToFactorySymbols !== null &&
this.sourceToFactorySymbols.has(sf.fileName)) { this.sourceToFactorySymbols.has(sf.fileName)) {
this.sourceToFactorySymbols.get(sf.fileName) !.add(analysis.factorySymbolName); this.sourceToFactorySymbols.get(sf.fileName) !.add(match.analyzed.factorySymbolName);
} }
} catch (err) { } catch (err) {
if (err instanceof FatalDiagnosticError) { if (err instanceof FatalDiagnosticError) {
this._diagnostics.push(err.toDiagnostic()); this._diagnostics.push(err.toDiagnostic());
@ -133,17 +200,25 @@ export class IvyCompilation {
} }
}; };
if (preanalyze && adapter.preanalyze !== undefined) { // If preanalysis was requested and a preanalysis step exists, then run preanalysis.
const preanalysis = adapter.preanalyze(node, metadata); // 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) { if (preanalysis !== undefined) {
promises.push(preanalysis.then(() => completeAnalysis())); // Await the results of preanalysis before running analysis.
promises.push(preanalysis.then(analyze));
} else { } else {
completeAnalysis(); // No async preanalysis needed, skip directly to analysis.
analyze();
} }
} else { } else {
completeAnalysis(); // Not in preanalysis mode or not needed for this handler, skip directly to analysis.
analyze();
} }
}); }
}; };
const visit = (node: ts.Node): void => { const visit = (node: ts.Node): void => {
@ -164,17 +239,23 @@ export class IvyCompilation {
} }
resolve(): void { resolve(): void {
this.analysis.forEach((op, decl) => { this.ivyClasses.forEach((ivyClass, node) => {
if (op.adapter.resolve !== undefined) { for (const match of ivyClass.matchedHandlers) {
op.adapter.resolve(decl, op.analysis); if (match.handler.resolve !== undefined && match.analyzed !== null &&
match.analyzed.analysis !== undefined) {
match.handler.resolve(node, match.analyzed.analysis);
}
} }
}); });
} }
typeCheck(context: TypeCheckContext): void { typeCheck(context: TypeCheckContext): void {
this.typeCheckMap.forEach((handler, node) => { this.ivyClasses.forEach((ivyClass, node) => {
if (handler.typeCheck !== undefined) { for (const match of ivyClass.matchedHandlers) {
handler.typeCheck(context, node, this.analysis.get(node) !.analysis); 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 { compileIvyFieldFor(node: ts.Declaration, constantPool: ConstantPool): CompileResult[]|undefined {
// Look to see whether the original node was analyzed. If not, there's nothing to do. // Look to see whether the original node was analyzed. If not, there's nothing to do.
const original = ts.getOriginalNode(node) as ts.Declaration; const original = ts.getOriginalNode(node) as ts.Declaration;
if (!this.analysis.has(original)) { if (!this.ivyClasses.has(original)) {
return undefined; return undefined;
} }
const op = this.analysis.get(original) !;
// Run the actual compilation, which generates an Expression for the Ivy field. const ivyClass = this.ivyClasses.get(original) !;
let res: CompileResult|CompileResult[] = op.adapter.compile(node, op.analysis, constantPool);
if (!Array.isArray(res)) { let res: CompileResult[] = [];
res = [res];
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, // Look up the .d.ts transformer for the input file and record that at least one field was
// which will allow the .d.ts to be transformed later. // generated, which will allow the .d.ts to be transformed later.
const fileName = original.getSourceFile().fileName; const fileName = original.getSourceFile().fileName;
const dtsTransformer = this.getDtsTransformer(fileName); const dtsTransformer = this.getDtsTransformer(fileName);
dtsTransformer.recordStaticField(reflectNameOfDeclaration(node) !, res); dtsTransformer.recordStaticField(reflectNameOfDeclaration(node) !, res);
// Return the instruction to the transformer so the field will be added. // Return the instruction to the transformer so the fields will be added.
return res; return res.length > 0 ? res : undefined;
} }
/** /**
* Lookup the `ts.Decorator` which triggered transformation of a particular class declaration. * 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; 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;
} }
/** /**

View File

@ -68,8 +68,7 @@ class IvyVisitor extends Visitor {
node = ts.updateClassDeclaration( node = ts.updateClassDeclaration(
node, node,
// Remove the decorator which triggered this compilation, leaving the others alone. // Remove the decorator which triggered this compilation, leaving the others alone.
maybeFilterDecorator( maybeFilterDecorator(node.decorators, this.compilation.ivyDecoratorsFor(node)),
node.decorators, this.compilation.ivyDecoratorFor(node) !.node as ts.Decorator),
node.modifiers, node.name, node.typeParameters, node.heritageClauses || [], node.modifiers, node.name, node.typeParameters, node.heritageClauses || [],
// Map over the class members and remove any Angular decorators from them. // Map over the class members and remove any Angular decorators from them.
members.map(member => this._stripAngularDecorators(member))); members.map(member => this._stripAngularDecorators(member)));
@ -208,11 +207,12 @@ function transformIvySourceFile(
function maybeFilterDecorator( function maybeFilterDecorator(
decorators: ts.NodeArray<ts.Decorator>| undefined, decorators: ts.NodeArray<ts.Decorator>| undefined,
toRemove: ts.Decorator): ts.NodeArray<ts.Decorator>|undefined { toRemove: ts.Decorator[]): ts.NodeArray<ts.Decorator>|undefined {
if (decorators === undefined) { if (decorators === undefined) {
return 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) { if (filtered.length === 0) {
return undefined; return undefined;
} }

View File

@ -26,3 +26,11 @@ export function isFromDtsFile(node: ts.Node): boolean {
} }
return sf !== undefined && D_TS.test(sf.fileName); 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();
}

View File

@ -624,6 +624,95 @@ describe('ngtsc behavioral tests', () => {
'i0.ɵNgModuleDefWithMeta<TestModule, [typeof TestPipe, typeof TestCmp], never, never>'); 'i0.ɵNgModuleDefWithMeta<TestModule, [typeof TestPipe, typeof TestCmp], never, never>');
}); });
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<TestCmp');
expect(dtsContents).toContain('DirectiveDefWithMeta<TestDir');
expect(dtsContents).toContain('PipeDefWithMeta<TestPipe');
expect(dtsContents).toContain('NgModuleDefWithMeta<TestNgModule');
// Validate that each class's .d.ts declaration also has an injectable definition.
expect(dtsContents).toContain('InjectableDef<TestCmp');
expect(dtsContents).toContain('InjectableDef<TestDir');
expect(dtsContents).toContain('InjectableDef<TestPipe');
expect(dtsContents).toContain('InjectableDef<TestNgModule');
});
it('should not compile a component and a directive annotation on the same class', () => {
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('compiling invalid @Injectables', () => {
describe('with strictInjectionParameters = true', () => { describe('with strictInjectionParameters = true', () => {
it('should give a compile-time error if an invalid @Injectable is used with no arguments', it('should give a compile-time error if an invalid @Injectable is used with no arguments',