From a46e0e48a3e470934319a0c3144d35aaae316288 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 27 Aug 2020 11:32:33 -0700 Subject: [PATCH] refactor(compiler-cli): Adjust output of TCB to support `TemplateTypeChecker` Symbol retrieval (#38618) The statements generated in the TCB are optimized for performance and producing diagnostics. These optimizations can result in generating a TCB that does not have all the information needed by the `TemplateTypeChecker` for retrieving `Symbol`s. For example, as an optimization, the TCB will not generate variable declaration statements for directives that have no references, inputs, or outputs. However, the `TemplateTypeChecker` always needs these statements to be present in order to provide `ts.Symbol`s and `ts.Type`s for the directives. This commit adds logic to the TCB generation to ensure the required information is available in a form that the `TemplateTypeChecker` can consume. It also adds an option to the `NgCompiler` that makes this generation configurable. PR Close #38618 --- .../src/ngtsc/core/src/compiler.ts | 16 +- .../src/ngtsc/core/test/compiler_test.ts | 2 +- packages/compiler-cli/src/ngtsc/program.ts | 2 +- packages/compiler-cli/src/ngtsc/tsc_plugin.ts | 3 +- .../src/ngtsc/typecheck/api/api.ts | 16 ++ .../src/ngtsc/typecheck/src/comments.ts | 74 ++++++ .../src/ngtsc/typecheck/src/diagnostics.ts | 44 +--- .../src/ngtsc/typecheck/src/expression.ts | 14 +- .../ngtsc/typecheck/src/type_check_block.ts | 214 ++++++++++++------ .../typecheck/test/span_comments_spec.ts | 10 +- .../src/ngtsc/typecheck/test/test_utils.ts | 4 +- .../typecheck/test/type_check_block_spec.ts | 99 +++++--- .../language-service/ivy/language_service.ts | 2 + 13 files changed, 346 insertions(+), 154 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 97b676c209..c4a6637d91 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -99,11 +99,15 @@ export class NgCompiler { readonly ignoreForEmit: Set; constructor( - private adapter: NgCompilerAdapter, private options: NgCompilerOptions, + private adapter: NgCompilerAdapter, + private options: NgCompilerOptions, private tsProgram: ts.Program, private typeCheckingProgramStrategy: TypeCheckingProgramStrategy, - private incrementalStrategy: IncrementalBuildStrategy, oldProgram: ts.Program|null = null, - private perfRecorder: PerfRecorder = NOOP_PERF_RECORDER) { + private incrementalStrategy: IncrementalBuildStrategy, + private enableTemplateTypeChecker: boolean, + oldProgram: ts.Program|null = null, + private perfRecorder: PerfRecorder = NOOP_PERF_RECORDER, + ) { this.constructionDiagnostics.push(...this.adapter.constructionDiagnostics); const incompatibleTypeCheckOptionsDiagnostic = verifyCompatibleTypeCheckOptions(this.options); if (incompatibleTypeCheckOptionsDiagnostic !== null) { @@ -212,6 +216,10 @@ export class NgCompiler { } getTemplateTypeChecker(): TemplateTypeChecker { + if (!this.enableTemplateTypeChecker) { + throw new Error( + 'The `TemplateTypeChecker` does not work without `enableTemplateTypeChecker`.'); + } return this.ensureAnalyzed().templateTypeChecker; } @@ -436,6 +444,7 @@ export class NgCompiler { strictSafeNavigationTypes: strictTemplates, useContextGenericType: strictTemplates, strictLiteralTypes: true, + enableTemplateTypeChecker: this.enableTemplateTypeChecker, }; } else { typeCheckingConfig = { @@ -456,6 +465,7 @@ export class NgCompiler { strictSafeNavigationTypes: false, useContextGenericType: false, strictLiteralTypes: false, + enableTemplateTypeChecker: this.enableTemplateTypeChecker, }; } diff --git a/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts b/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts index cb4589494b..16e9fea106 100644 --- a/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts +++ b/packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts @@ -49,7 +49,7 @@ runInEachFileSystem(() => { const program = ts.createProgram({host, options, rootNames: host.inputFiles}); const compiler = new NgCompiler( host, options, program, new ReusedProgramStrategy(program, host, options, []), - new NoopIncrementalBuildStrategy()); + new NoopIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false); const diags = compiler.getDiagnostics(getSourceFileOrError(program, COMPONENT)); expect(diags.length).toBe(1); diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 2f11883953..cc277e0d97 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -99,7 +99,7 @@ export class NgtscProgram implements api.Program { // Create the NgCompiler which will drive the rest of the compilation. this.compiler = new NgCompiler( this.host, options, this.tsProgram, reusedProgramStrategy, this.incrementalStrategy, - reuseProgram, this.perfRecorder); + /** enableTemplateTypeChecker */ false, reuseProgram, this.perfRecorder); } getTsProgram(): ts.Program { diff --git a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts index 54c6238521..dcd62d5093 100644 --- a/packages/compiler-cli/src/ngtsc/tsc_plugin.ts +++ b/packages/compiler-cli/src/ngtsc/tsc_plugin.ts @@ -102,7 +102,8 @@ export class NgTscPlugin implements TscPlugin { program, this.host, this.options, this.host.shimExtensionPrefixes); this._compiler = new NgCompiler( this.host, this.options, program, typeCheckStrategy, - new PatchedProgramIncrementalBuildStrategy(), oldProgram, NOOP_PERF_RECORDER); + new PatchedProgramIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false, + oldProgram, NOOP_PERF_RECORDER); return { ignoreForDiagnostics: this._compiler.ignoreForDiagnostics, ignoreForEmit: this._compiler.ignoreForEmit, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index 594475009a..e9b3a609d1 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -185,6 +185,22 @@ export interface TypeCheckingConfig { */ checkTypeOfNonDomReferences: boolean; + /** + * Whether to adjust the output of the TCB to ensure compatibility with the `TemplateTypeChecker`. + * + * The statements generated in the TCB are optimized for performance and producing diagnostics. + * These optimizations can result in generating a TCB that does not have all the information + * needed by the `TemplateTypeChecker` for retrieving `Symbol`s. For example, as an optimization, + * the TCB will not generate variable declaration statements for directives that have no + * references, inputs, or outputs. However, the `TemplateTypeChecker` always needs these + * statements to be present in order to provide `ts.Symbol`s and `ts.Type`s for the directives. + * + * When set to `false`, enables TCB optimizations for template diagnostics. + * When set to `true`, ensures all information required by `TemplateTypeChecker` to + * retrieve symbols for template nodes is available in the TCB. + */ + enableTemplateTypeChecker: boolean; + /** * Whether to include type information from pipes in the type-checking operation. * diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts new file mode 100644 index 0000000000..65bc50a7a0 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright Google LLC 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 {AbsoluteSourceSpan} from '@angular/compiler'; +import * as ts from 'typescript'; + +const parseSpanComment = /^(\d+),(\d+)$/; + +/** + * Reads the trailing comments and finds the first match which is a span comment (i.e. 4,10) on a + * node and returns it as an `AbsoluteSourceSpan`. + * + * Will return `null` if no trailing comments on the node match the expected form of a source span. + */ +export function readSpanComment(sourceFile: ts.SourceFile, node: ts.Node): AbsoluteSourceSpan|null { + return ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { + if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { + return null; + } + const commentText = sourceFile.text.substring(pos + 2, end - 2); + const match = commentText.match(parseSpanComment); + if (match === null) { + return null; + } + + return new AbsoluteSourceSpan(+match[1], +match[2]); + }) || null; +} + +/** Used to identify what type the comment is. */ +export enum CommentTriviaType { + DIAGNOSTIC = 'D', + EXPRESSION_TYPE_IDENTIFIER = 'T', +} + +/** Identifies what the TCB expression is for (for example, a directive declaration). */ +export enum ExpressionIdentifier { + DIRECTIVE = 'DIR', +} + +/** Tags the node with the given expression identifier. */ +export function addExpressionIdentifier(node: ts.Node, identifier: ExpressionIdentifier) { + ts.addSyntheticTrailingComment( + node, ts.SyntaxKind.MultiLineCommentTrivia, + `${CommentTriviaType.EXPRESSION_TYPE_IDENTIFIER}:${identifier}`, + /* hasTrailingNewLine */ false); +} + +export const IGNORE_MARKER = `${CommentTriviaType.DIAGNOSTIC}:ignore`; + +/** + * Tag the `ts.Node` with an indication that any errors arising from the evaluation of the node + * should be ignored. + */ +export function markIgnoreDiagnostics(node: ts.Node): void { + ts.addSyntheticTrailingComment( + node, ts.SyntaxKind.MultiLineCommentTrivia, IGNORE_MARKER, /* hasTrailingNewLine */ false); +} + +/** Returns true if the node has a marker that indicates diagnostics errors should be ignored. */ +export function hasIgnoreMarker(node: ts.Node, sourceFile: ts.SourceFile): boolean { + return ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { + if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { + return null; + } + const commentText = sourceFile.text.substring(pos + 2, end - 2); + return commentText === IGNORE_MARKER; + }) === true; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 99476d31e2..f80155bf21 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -9,9 +9,12 @@ import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler'; import * as ts from 'typescript'; import {getTokenAtPosition} from '../../util/src/typescript'; -import {ExternalTemplateSourceMapping, TemplateId, TemplateSourceMapping} from '../api'; +import {TemplateId, TemplateSourceMapping} from '../api'; import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics'; +import {hasIgnoreMarker, readSpanComment} from './comments'; + + /** * Adapter interface which allows the template type-checking diagnostics code to interpret offsets * in a TCB and map them back to original locations in the template. @@ -47,14 +50,14 @@ export function wrapForDiagnostics(expr: ts.Expression): ts.Expression { return ts.createParen(expr); } -const IGNORE_MARKER = 'ignore'; - /** - * Adds a marker to the node that signifies that any errors within the node should not be reported. + * Wraps the node in parenthesis such that inserted span comments become attached to the proper + * node. This is an alias for `ts.createParen` with the benefit that it signifies that the + * inserted parenthesis are for use by the type checker, not for correctness of the rendered TCB + * code. */ -export function ignoreDiagnostics(node: ts.Node): void { - ts.addSyntheticTrailingComment( - node, ts.SyntaxKind.MultiLineCommentTrivia, IGNORE_MARKER, /* hasTrailingNewLine */ false); +export function wrapForTypeChecker(expr: ts.Expression): ts.Expression { + return ts.createParen(expr); } /** @@ -200,30 +203,3 @@ function getTemplateId(node: ts.Node, sourceFile: ts.SourceFile): TemplateId|nul return commentText; }) as TemplateId || null; } - -const parseSpanComment = /^(\d+),(\d+)$/; - -function readSpanComment(sourceFile: ts.SourceFile, node: ts.Node): AbsoluteSourceSpan|null { - return ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { - if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { - return null; - } - const commentText = sourceFile.text.substring(pos + 2, end - 2); - const match = commentText.match(parseSpanComment); - if (match === null) { - return null; - } - - return new AbsoluteSourceSpan(+match[1], +match[2]); - }) || null; -} - -function hasIgnoreMarker(node: ts.Node, sourceFile: ts.SourceFile): boolean { - return ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { - if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { - return null; - } - const commentText = sourceFile.text.substring(pos + 2, end - 2); - return commentText === IGNORE_MARKER; - }) === true; -} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 708ac522a1..6001896b5e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -10,7 +10,7 @@ import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, import * as ts from 'typescript'; import {TypeCheckingConfig} from '../api'; -import {addParseSpanInfo, wrapForDiagnostics} from './diagnostics'; +import {addParseSpanInfo, wrapForDiagnostics, wrapForTypeChecker} from './diagnostics'; import {tsCastToAny} from './ts_util'; export const NULL_AS_ANY = @@ -112,7 +112,14 @@ class AstTranslator implements AstVisitor { visitConditional(ast: Conditional): ts.Expression { const condExpr = this.translate(ast.condition); const trueExpr = this.translate(ast.trueExp); - const falseExpr = this.translate(ast.falseExp); + // Wrap `falseExpr` in parens so that the trailing parse span info is not attributed to the + // whole conditional. + // In the following example, the last source span comment (5,6) could be seen as the + // trailing comment for _either_ the whole conditional expression _or_ just the `falseExpr` that + // is immediately before it: + // `conditional /*1,2*/ ? trueExpr /*3,4*/ : falseExpr /*5,6*/` + // This should be instead be `conditional /*1,2*/ ? trueExpr /*3,4*/ : (falseExpr /*5,6*/)` + const falseExpr = wrapForTypeChecker(this.translate(ast.falseExp)); const node = ts.createParen(ts.createConditional(condExpr, trueExpr, falseExpr)); addParseSpanInfo(node, ast.sourceSpan); return node; @@ -135,7 +142,8 @@ class AstTranslator implements AstVisitor { // interpolation's expressions. The chain is started using an actual string literal to ensure // the type is inferred as 'string'. return ast.expressions.reduce( - (lhs, ast) => ts.createBinary(lhs, ts.SyntaxKind.PlusToken, this.translate(ast)), + (lhs, ast) => + ts.createBinary(lhs, ts.SyntaxKind.PlusToken, wrapForTypeChecker(this.translate(ast))), ts.createLiteral('')); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index e295889c7c..4229892fa7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -14,7 +14,8 @@ import {ClassPropertyName} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata} from '../api'; -import {addParseSpanInfo, addTemplateId, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics'; +import {addExpressionIdentifier, ExpressionIdentifier, markIgnoreDiagnostics} from './comments'; +import {addParseSpanInfo, addTemplateId, wrapForDiagnostics} from './diagnostics'; import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {astToTypescript, NULL_AS_ANY} from './expression'; @@ -22,8 +23,6 @@ import {OutOfBandDiagnosticRecorder} from './oob'; import {ExpressionSemanticVisitor} from './template_semantics'; import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util'; - - /** * Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a * "type check block" function. @@ -259,7 +258,7 @@ class TcbTemplateBodyOp extends TcbOp { // The expression has already been checked in the type constructor invocation, so // it should be ignored when used within a template guard. - ignoreDiagnostics(expr); + markIgnoreDiagnostics(expr); if (guard.type === 'binding') { // Use the binding expression itself as guard. @@ -377,11 +376,80 @@ class TcbDirectiveTypeOp extends TcbOp { const id = this.tcb.allocateId(); const type = this.tcb.env.referenceType(this.dir.ref); + addParseSpanInfo(type, this.node.startSourceSpan || this.node.sourceSpan); + addExpressionIdentifier(type, ExpressionIdentifier.DIRECTIVE); this.scope.addStatement(tsDeclareVariable(id, type)); return id; } } +/** + * A `TcbOp` which creates a variable for a local ref in a template. + * The initializer for the variable is the variable expression for the directive, template, or + * element the ref refers to. When the reference is used in the template, those TCB statements will + * access this variable as well. For example: + * ``` + * var _t1 = document.createElement('div'); + * var _t2 = _t1; + * _t2.value + * ``` + * This operation supports more fluent lookups for the `TemplateTypeChecker` when getting a symbol + * for a reference. In most cases, this isn't essential; that is, the information for the symbol + * could be gathered without this operation using the `BoundTarget`. However, for the case of + * ng-template references, we will need this reference variable to not only provide a location in + * the shim file, but also to narrow the variable to the correct `TemplateRef` type rather than + * `TemplateRef` (this work is still TODO). + * + * Executing this operation returns a reference to the directive instance variable with its inferred + * type. + */ +class TcbReferenceOp extends TcbOp { + constructor( + private readonly tcb: Context, private readonly scope: Scope, + private readonly node: TmplAstReference, + private readonly host: TmplAstElement|TmplAstTemplate, + private readonly target: TypeCheckableDirectiveMeta|TmplAstTemplate|TmplAstElement) { + super(); + } + + // The statement generated by this operation is only used to for the Type Checker + // so it can map a reference variable in the template directly to a node in the TCB. + readonly optional = true; + + execute(): ts.Identifier { + const id = this.tcb.allocateId(); + let initializer = ts.getMutableClone( + this.target instanceof TmplAstTemplate || this.target instanceof TmplAstElement ? + this.scope.resolve(this.target) : + this.scope.resolve(this.host, this.target)); + + // The reference is either to an element, an node, or to a directive on an + // element or template. + if ((this.target instanceof TmplAstElement && !this.tcb.env.config.checkTypeOfDomReferences) || + !this.tcb.env.config.checkTypeOfNonDomReferences) { + // References to DOM nodes are pinned to 'any' when `checkTypeOfDomReferences` is `false`. + // References to `TemplateRef`s and directives are pinned to 'any' when + // `checkTypeOfNonDomReferences` is `false`. + initializer = + ts.createAsExpression(initializer, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + } else if (this.target instanceof TmplAstTemplate) { + // Direct references to an node simply require a value of type + // `TemplateRef`. To get this, an expression of the form + // `(_t1 as any as TemplateRef)` is constructed. + initializer = + ts.createAsExpression(initializer, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); + initializer = ts.createAsExpression( + initializer, + this.tcb.env.referenceExternalType('@angular/core', 'TemplateRef', [DYNAMIC_TYPE])); + initializer = ts.createParen(initializer); + } + addParseSpanInfo(initializer, this.node.sourceSpan); + + this.scope.addStatement(tsCreateVariable(id, initializer)); + return id; + } +} + /** * A `TcbOp` which constructs an instance of a directive with types inferred from its inputs. The * inputs themselves are not checked here; checking of inputs is achieved in `TcbDirectiveInputsOp`. @@ -441,7 +509,7 @@ class TcbDirectiveCtorOp extends TcbOp { // Call the type constructor of the directive to infer a type, and assign the directive // instance. const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, Array.from(genericInputs.values())); - ignoreDiagnostics(typeCtor); + markIgnoreDiagnostics(typeCtor); this.scope.addStatement(tsCreateVariable(id, typeCtor)); return id; } @@ -731,7 +799,7 @@ class TcbUnclaimedInputsOp extends TcbOp { * * Executing this operation returns nothing. */ -class TcbDirectiveOutputsOp extends TcbOp { +export class TcbDirectiveOutputsOp extends TcbOp { constructor( private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement, private dir: TypeCheckableDirectiveMeta) { @@ -789,6 +857,39 @@ class TcbDirectiveOutputsOp extends TcbOp { return null; } + + /** + * Outputs are a `ts.CallExpression` that look like one of the two: + * - `_outputHelper(_t1["outputField"]).subscribe(handler);` + * - `_t1.addEventListener(handler);` + * This method reverses the operations to create a call expression for a directive output. + * It unpacks the given call expression and returns the original element access (i.e. + * `_t1["outputField"]` in the example above). Returns `null` if the given call expression is not + * the expected structure of an output binding + */ + static decodeOutputCallExpression(node: ts.CallExpression): ts.ElementAccessExpression|null { + // `node.expression` === `_outputHelper(_t1["outputField"]).subscribe` or `_t1.addEventListener` + if (!ts.isPropertyAccessExpression(node.expression) || + node.expression.name.text === 'addEventListener') { + // `addEventListener` outputs do not have an `ElementAccessExpression` for the output field. + return null; + } + + if (!ts.isCallExpression(node.expression.expression)) { + return null; + } + + // `node.expression.expression` === `_outputHelper(_t1["outputField"])` + if (node.expression.expression.arguments.length === 0) { + return null; + } + + const [outputFieldAccess] = node.expression.expression.arguments; + if (!ts.isElementAccessExpression(outputFieldAccess)) { + return null; + } + return outputFieldAccess; + } } /** @@ -943,6 +1044,11 @@ class Scope { private directiveOpMap = new Map>(); + /** + * A map of `TmplAstReference`s to the index of their `TcbReferenceOp` in the `opQueue` + */ + private referenceOpMap = new Map(); + /** * Map of immediately nested s (within this `Scope`) represented by `TmplAstTemplate` * nodes to the index of their `TcbTemplateContextOp`s in the `opQueue`. @@ -1023,12 +1129,13 @@ class Scope { * * `TmplAstElement` - retrieve the expression for the element DOM node * * `TmplAstTemplate` - retrieve the template context variable * * `TmplAstVariable` - retrieve a template let- variable + * * `TmplAstReference` - retrieve variable created for the local ref * * @param directive if present, a directive type on a `TmplAstElement` or `TmplAstTemplate` to * look up instead of the default for an element or template node. */ resolve( - node: TmplAstElement|TmplAstTemplate|TmplAstVariable, + node: TmplAstElement|TmplAstTemplate|TmplAstVariable|TmplAstReference, directive?: TypeCheckableDirectiveMeta): ts.Expression { // Attempt to resolve the operation locally. const res = this.resolveLocal(node, directive); @@ -1054,7 +1161,10 @@ class Scope { */ render(): ts.Statement[] { for (let i = 0; i < this.opQueue.length; i++) { - this.executeOp(i, /* skipOptional */ true); + // Optional statements cannot be skipped when we are generating the TCB for use + // by the TemplateTypeChecker. + const skipOptional = !this.tcb.env.config.enableTemplateTypeChecker; + this.executeOp(i, skipOptional); } return this.statements; } @@ -1086,9 +1196,11 @@ class Scope { } private resolveLocal( - ref: TmplAstElement|TmplAstTemplate|TmplAstVariable, + ref: TmplAstElement|TmplAstTemplate|TmplAstVariable|TmplAstReference, directive?: TypeCheckableDirectiveMeta): ts.Expression|null { - if (ref instanceof TmplAstVariable && this.varMap.has(ref)) { + if (ref instanceof TmplAstReference && this.referenceOpMap.has(ref)) { + return this.resolveOp(this.referenceOpMap.get(ref)!); + } else if (ref instanceof TmplAstVariable && this.varMap.has(ref)) { // Resolving a context variable for this template. // Execute the `TcbVariableOp` associated with the `TmplAstVariable`. return this.resolveOp(this.varMap.get(ref)!); @@ -1163,27 +1275,38 @@ class Scope { for (const child of node.children) { this.appendNode(child); } - this.checkReferencesOfNode(node); + this.checkAndAppendReferencesOfNode(node); } else if (node instanceof TmplAstTemplate) { // Template children are rendered in a child scope. this.appendDirectivesAndInputsOfNode(node); this.appendOutputsOfNode(node); + const ctxIndex = this.opQueue.push(new TcbTemplateContextOp(this.tcb, this)) - 1; + this.templateCtxOpMap.set(node, ctxIndex); if (this.tcb.env.config.checkTemplateBodies) { - const ctxIndex = this.opQueue.push(new TcbTemplateContextOp(this.tcb, this)) - 1; - this.templateCtxOpMap.set(node, ctxIndex); this.opQueue.push(new TcbTemplateBodyOp(this.tcb, this, node)); } - this.checkReferencesOfNode(node); + this.checkAndAppendReferencesOfNode(node); } else if (node instanceof TmplAstBoundText) { this.opQueue.push(new TcbTextInterpolationOp(this.tcb, this, node)); } } - private checkReferencesOfNode(node: TmplAstElement|TmplAstTemplate): void { + private checkAndAppendReferencesOfNode(node: TmplAstElement|TmplAstTemplate): void { for (const ref of node.references) { - if (this.tcb.boundTarget.getReferenceTarget(ref) === null) { + const target = this.tcb.boundTarget.getReferenceTarget(ref); + if (target === null) { this.tcb.oobRecorder.missingReferenceTarget(this.tcb.id, ref); + continue; } + + let ctxIndex: number; + if (target instanceof TmplAstTemplate || target instanceof TmplAstElement) { + ctxIndex = this.opQueue.push(new TcbReferenceOp(this.tcb, this, ref, node, target)) - 1; + } else { + ctxIndex = + this.opQueue.push(new TcbReferenceOp(this.tcb, this, ref, node, target.directive)) - 1; + } + this.referenceOpMap.set(ref, ctxIndex); } } @@ -1422,62 +1545,9 @@ class TcbExpressionTranslator { return null; } - // This expression has a binding to some variable or reference in the template. Resolve it. - if (binding instanceof TmplAstVariable) { - const expr = ts.getMutableClone(this.scope.resolve(binding)); - addParseSpanInfo(expr, ast.sourceSpan); - return expr; - } else if (binding instanceof TmplAstReference) { - const target = this.tcb.boundTarget.getReferenceTarget(binding); - if (target === null) { - // This reference is unbound. Traversal of the `TmplAstReference` itself should have - // recorded the error in the `OutOfBandDiagnosticRecorder`. - // Still check the rest of the expression if possible by using an `any` value. - return NULL_AS_ANY; - } - - // The reference is either to an element, an node, or to a directive on an - // element or template. - - if (target instanceof TmplAstElement) { - if (!this.tcb.env.config.checkTypeOfDomReferences) { - // References to DOM nodes are pinned to 'any'. - return NULL_AS_ANY; - } - - const expr = ts.getMutableClone(this.scope.resolve(target)); - addParseSpanInfo(expr, ast.sourceSpan); - return expr; - } else if (target instanceof TmplAstTemplate) { - if (!this.tcb.env.config.checkTypeOfNonDomReferences) { - // References to `TemplateRef`s pinned to 'any'. - return NULL_AS_ANY; - } - - // Direct references to an node simply require a value of type - // `TemplateRef`. To get this, an expression of the form - // `(null as any as TemplateRef)` is constructed. - let value: ts.Expression = ts.createNull(); - value = ts.createAsExpression(value, ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); - value = ts.createAsExpression( - value, - this.tcb.env.referenceExternalType('@angular/core', 'TemplateRef', [DYNAMIC_TYPE])); - value = ts.createParen(value); - addParseSpanInfo(value, ast.sourceSpan); - return value; - } else { - if (!this.tcb.env.config.checkTypeOfNonDomReferences) { - // References to directives are pinned to 'any'. - return NULL_AS_ANY; - } - - const expr = ts.getMutableClone(this.scope.resolve(target.node, target.directive)); - addParseSpanInfo(expr, ast.sourceSpan); - return expr; - } - } else { - throw new Error(`Unreachable: ${binding}`); - } + const expr = ts.getMutableClone(this.scope.resolve(binding)); + addParseSpanInfo(expr, ast.sourceSpan); + return expr; } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts index 1a87da6987..ad868292b2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts @@ -22,12 +22,12 @@ describe('type check blocks diagnostics', () => { it('should annotate conditions', () => { expect(tcbWithSpans('{{ a ? b : c }}')) .toContain( - '(((ctx).a /*3,4*/) /*3,4*/ ? ((ctx).b /*7,8*/) /*7,8*/ : ((ctx).c /*11,12*/) /*11,12*/) /*3,12*/'); + '(((ctx).a /*3,4*/) /*3,4*/ ? ((ctx).b /*7,8*/) /*7,8*/ : (((ctx).c /*11,12*/) /*11,12*/)) /*3,12*/'); }); it('should annotate interpolations', () => { expect(tcbWithSpans('{{ hello }} {{ world }}')) - .toContain('"" + ((ctx).hello /*3,8*/) /*3,8*/ + ((ctx).world /*15,20*/) /*15,20*/'); + .toContain('"" + (((ctx).hello /*3,8*/) /*3,8*/) + (((ctx).world /*15,20*/) /*15,20*/)'); }); it('should annotate literal map expressions', () => { @@ -47,7 +47,7 @@ describe('type check blocks diagnostics', () => { it('should annotate literals', () => { const TEMPLATE = '{{ 123 }}'; - expect(tcbWithSpans(TEMPLATE)).toContain('123 /*3,6*/;'); + expect(tcbWithSpans(TEMPLATE)).toContain('123 /*3,6*/'); }); it('should annotate non-null assertions', () => { @@ -57,7 +57,7 @@ describe('type check blocks diagnostics', () => { it('should annotate prefix not', () => { const TEMPLATE = `{{ !a }}`; - expect(tcbWithSpans(TEMPLATE)).toContain('!(((ctx).a /*4,5*/) /*4,5*/) /*3,5*/;'); + expect(tcbWithSpans(TEMPLATE)).toContain('!(((ctx).a /*4,5*/) /*4,5*/) /*3,5*/'); }); it('should annotate method calls', () => { @@ -141,7 +141,7 @@ describe('type check blocks diagnostics', () => { }]; const block = tcbWithSpans(TEMPLATE, PIPES); expect(block).toContain( - '(null as TestPipe).transform(((ctx).a /*3,4*/) /*3,4*/, ((ctx).b /*12,13*/) /*12,13*/) /*3,13*/;'); + '((null as TestPipe).transform(((ctx).a /*3,4*/) /*3,4*/, ((ctx).b /*12,13*/) /*12,13*/) /*3,13*/);'); }); describe('attaching multiple comments for multiple references', () => { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index d87a8141e3..88bb3be900 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {CssSelector, ParseSourceFile, ParseSourceSpan, parseTemplate, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, TmplAstReference, Type} from '@angular/compiler'; +import {CssSelector, ParseSourceFile, ParseSourceSpan, parseTemplate, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, Type} from '@angular/compiler'; import * as ts from 'typescript'; import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError, LogicalFileSystem} from '../../file_system'; @@ -173,6 +173,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { strictSafeNavigationTypes: true, useContextGenericType: true, strictLiteralTypes: true, + enableTemplateTypeChecker: false, }; // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead. @@ -230,6 +231,7 @@ export function tcb( strictSafeNavigationTypes: true, useContextGenericType: true, strictLiteralTypes: true, + enableTemplateTypeChecker: false, }; options = options || { emitSpans: false, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index fb168922b4..023503fb74 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -13,38 +13,38 @@ import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from './test_ut describe('type check blocks', () => { it('should generate a basic block for a binding', () => { - expect(tcb('{{hello}} {{world}}')).toContain('"" + ((ctx).hello) + ((ctx).world);'); + expect(tcb('{{hello}} {{world}}')).toContain('"" + (((ctx).hello)) + (((ctx).world));'); }); it('should generate literal map expressions', () => { const TEMPLATE = '{{ method({foo: a, bar: b}) }}'; - expect(tcb(TEMPLATE)).toContain('(ctx).method({ "foo": ((ctx).a), "bar": ((ctx).b) });'); + expect(tcb(TEMPLATE)).toContain('(ctx).method({ "foo": ((ctx).a), "bar": ((ctx).b) })'); }); it('should generate literal array expressions', () => { const TEMPLATE = '{{ method([a, b]) }}'; - expect(tcb(TEMPLATE)).toContain('(ctx).method([((ctx).a), ((ctx).b)]);'); + expect(tcb(TEMPLATE)).toContain('(ctx).method([((ctx).a), ((ctx).b)])'); }); it('should handle non-null assertions', () => { const TEMPLATE = `{{a!}}`; - expect(tcb(TEMPLATE)).toContain('((((ctx).a))!);'); + expect(tcb(TEMPLATE)).toContain('((((ctx).a))!)'); }); it('should handle unary - operator', () => { const TEMPLATE = `{{-1}}`; - expect(tcb(TEMPLATE)).toContain('(-1);'); + expect(tcb(TEMPLATE)).toContain('(-1)'); }); it('should handle keyed property access', () => { const TEMPLATE = `{{a[b]}}`; - expect(tcb(TEMPLATE)).toContain('(((ctx).a))[((ctx).b)];'); + expect(tcb(TEMPLATE)).toContain('(((ctx).a))[((ctx).b)]'); }); it('should handle nested ternary expressions', () => { const TEMPLATE = `{{a ? b : c ? d : e}}`; expect(tcb(TEMPLATE)) - .toContain('(((ctx).a) ? ((ctx).b) : (((ctx).c) ? ((ctx).d) : ((ctx).e)))'); + .toContain('(((ctx).a) ? ((ctx).b) : ((((ctx).c) ? ((ctx).d) : (((ctx).e)))))'); }); it('should handle quote expressions as any type', () => { @@ -106,7 +106,7 @@ describe('type check blocks', () => { it('should handle method calls of template variables', () => { const TEMPLATE = `{{a(1)}}`; expect(tcb(TEMPLATE)).toContain('var _t2 = _t1.$implicit;'); - expect(tcb(TEMPLATE)).toContain('(_t2).a(1);'); + expect(tcb(TEMPLATE)).toContain('(_t2).a(1)'); }); it('should handle implicit vars when using microsyntax', () => { @@ -179,8 +179,9 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t2 = Dir.ngTypeCtor((null!)); ' + - 'var _t1 = Dir.ngTypeCtor({ "input": (_t2) });'); + 'var _t2 = Dir.ngTypeCtor({ "input": (null!) }); ' + + 'var _t1 = _t2; ' + + '_t2.input = (_t1);'); }); it('should generate circular references between two directives correctly', () => { @@ -208,9 +209,12 @@ describe('type check blocks', () => { ]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t3 = DirB.ngTypeCtor((null!)); ' + - 'var _t2 = DirA.ngTypeCtor({ "inputA": (_t3) }); ' + - 'var _t1 = DirB.ngTypeCtor({ "inputB": (_t2) });'); + 'var _t4 = DirA.ngTypeCtor({ "inputA": (null!) }); ' + + 'var _t3 = _t4; ' + + 'var _t2 = DirB.ngTypeCtor({ "inputB": (_t3) }); ' + + 'var _t1 = _t2; ' + + '_t4.inputA = (_t1); ' + + '_t2.inputB = (_t3);'); }); it('should handle empty bindings', () => { @@ -263,8 +267,10 @@ describe('type check blocks', () => { `; const block = tcb(TEMPLATE); expect(block).not.toContain('"div"'); - expect(block).toContain('var _t1 = document.createElement("button");'); - expect(block).toContain('(ctx).handle(_t1);'); + expect(block).toContain( + 'var _t2 = document.createElement("button"); ' + + 'var _t1 = _t2; ' + + '_t2.addEventListener'); }); it('should only generate directive declarations that have bindings or are referenced', () => { @@ -313,7 +319,8 @@ describe('type check blocks', () => { expect(block).toContain('_t1.input = (((ctx).value));'); expect(block).toContain('var _t2: HasOutput = (null!)'); expect(block).toContain('_t2["output"]'); - expect(block).toContain('var _t3: HasReference = (null!)'); + expect(block).toContain('var _t4: HasReference = (null!)'); + expect(block).toContain('var _t3 = _t4;'); expect(block).toContain('(_t3).a'); expect(block).not.toContain('NoBindings'); expect(block).not.toContain('NoReference'); @@ -325,7 +332,8 @@ describe('type check blocks', () => { `; expect(tcb(TEMPLATE)) - .toContain('var _t1 = document.createElement("input"); "" + ((_t1).value);'); + .toContain( + 'var _t2 = document.createElement("input"); var _t1 = _t2; "" + (((_t1).value));'); }); it('should generate a forward directive reference correctly', () => { @@ -339,7 +347,11 @@ describe('type check blocks', () => { selector: '[dir]', exportAs: ['dir'], }]; - expect(tcb(TEMPLATE, DIRECTIVES)).toContain('var _t1: Dir = (null!); "" + ((_t1).value);'); + expect(tcb(TEMPLATE, DIRECTIVES)) + .toContain( + 'var _t2: Dir = (null!); ' + + 'var _t1 = _t2; ' + + '"" + (((_t1).value));'); }); it('should handle style and class bindings specially', () => { @@ -385,8 +397,9 @@ describe('type check blocks', () => { }]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t1: Dir = (null!); ' + - '_t1.input = (_t1);'); + 'var _t2: Dir = (null!); ' + + 'var _t1 = _t2; ' + + '_t2.input = (_t1);'); }); it('should generate circular references between two directives correctly', () => { @@ -412,10 +425,12 @@ describe('type check blocks', () => { ]; expect(tcb(TEMPLATE, DIRECTIVES)) .toContain( - 'var _t1: DirB = (null!); ' + - 'var _t2: DirA = (null!); ' + - '_t2.inputA = (_t1); ' + - '_t1.inputA = (_t2);'); + 'var _t2: DirB = (null!); ' + + 'var _t1 = _t2; ' + + 'var _t3: DirA = (null!); ' + + '_t3.inputA = (_t1); ' + + 'var _t4 = _t3; ' + + '_t2.inputA = (_t4);'); }); it('should handle undeclared properties', () => { @@ -566,7 +581,7 @@ describe('type check blocks', () => { it('should handle $any casts', () => { const TEMPLATE = `{{$any(a)}}`; const block = tcb(TEMPLATE); - expect(block).toContain('(((ctx).a) as any);'); + expect(block).toContain('(((ctx).a) as any)'); }); describe('experimental DOM checking via lib.dom.d.ts', () => { @@ -699,6 +714,7 @@ describe('type check blocks', () => { strictSafeNavigationTypes: true, useContextGenericType: true, strictLiteralTypes: true, + enableTemplateTypeChecker: false, }; describe('config.applyTemplateContextGuards', () => { @@ -718,16 +734,27 @@ describe('type check blocks', () => { }); describe('config.checkTemplateBodies', () => { - const TEMPLATE = `{{a}}`; + const TEMPLATE = `{{a}}{{ref}}`; it('should descend into template bodies when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('((ctx).a);'); + expect(block).toContain('((ctx).a)'); }); it('should not descend into template bodies when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).not.toContain('((ctx).a);'); + expect(block).not.toContain('((ctx).a)'); + }); + + it('generates a references var when enabled', () => { + const block = tcb(TEMPLATE, DIRECTIVES); + expect(block).toContain('var _t2 = (_t1 as any as core.TemplateRef);'); + }); + + it('generates a reference var when disabled', () => { + const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false}; + const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); + expect(block).toContain('var _t2 = (_t1 as any as core.TemplateRef);'); }); }); @@ -844,7 +871,9 @@ describe('type check blocks', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfDomReferences: false}; const block = tcb(TEMPLATE, [], DISABLED_CONFIG); - expect(block).toContain('(null as any).value'); + expect(block).toContain( + 'var _t1 = (_t2 as any); ' + + '"" + (((_t1).value));'); }); }); @@ -868,14 +897,18 @@ describe('type check blocks', () => { it('should trace references to an when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); - expect(block).toContain('((null as any as core.TemplateRef)).value2'); + expect(block).toContain( + 'var _t4 = (_t3 as any as core.TemplateRef); ' + + '"" + (((_t4).value2));'); }); it('should use any for reference types when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfNonDomReferences: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); - expect(block).toContain('(null as any).value'); + expect(block).toContain( + 'var _t1 = (_t2 as any); ' + + '"" + (((_t1).value));'); }); }); @@ -914,12 +947,12 @@ describe('type check blocks', () => { it('should check types of pipes when enabled', () => { const block = tcb(TEMPLATE, PIPES); - expect(block).toContain('(null as TestPipe).transform(((ctx).a), ((ctx).b), ((ctx).c));'); + expect(block).toContain('(null as TestPipe).transform(((ctx).a), ((ctx).b), ((ctx).c))'); }); it('should not check types of pipes when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false}; const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG); - expect(block).toContain('(null as any).transform(((ctx).a), ((ctx).b), ((ctx).c));'); + expect(block).toContain('(null as any).transform(((ctx).a), ((ctx).b), ((ctx).c))'); }); }); diff --git a/packages/language-service/ivy/language_service.ts b/packages/language-service/ivy/language_service.ts index 128c1ed1a9..88b7edfd03 100644 --- a/packages/language-service/ivy/language_service.ts +++ b/packages/language-service/ivy/language_service.ts @@ -52,7 +52,9 @@ export class LanguageService { program, this.strategy, new PatchedProgramIncrementalBuildStrategy(), + /** enableTemplateTypeChecker */ true, this.lastKnownProgram, + /** perfRecorder (use default) */ undefined, ); }