From 0677cf0cbebca42e0441b9596c0fc4faa6da44b8 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 16 Aug 2019 16:45:33 -0700 Subject: [PATCH] feat(ivy): use the schema registry to check DOM bindings (#32171) Previously, ngtsc attempted to use the .d.ts schema for HTML elements to check bindings to DOM properties. However, the TypeScript lib.dom.d.ts schema does not perfectly align with the Angular DomElementSchemaRegistry, and these inconsistencies would cause issues in apps. There is also the concern of supporting both CUSTOM_ELEMENTS_SCHEMA and NO_ERRORS_SCHEMA which would have been very difficult to do in the existing system. With this commit, the DomElementSchemaRegistry is employed in ngtsc to check bindings to the DOM. Previous work on producing template diagnostics is used to support generation of this different kind of error with the same high quality of error message. PR Close #32171 --- .../src/ngtsc/annotations/src/component.ts | 7 +- .../src/ngtsc/annotations/src/ng_module.ts | 44 ++++++- .../src/ngtsc/diagnostics/src/code.ts | 10 ++ .../src/ngtsc/metadata/src/api.ts | 3 +- .../src/ngtsc/metadata/src/dts.ts | 1 + packages/compiler-cli/src/ngtsc/program.ts | 7 +- .../compiler-cli/src/ngtsc/scope/src/local.ts | 4 +- .../src/ngtsc/scope/test/local_spec.ts | 18 ++- .../src/ngtsc/typecheck/BUILD.bazel | 1 + .../src/ngtsc/typecheck/src/api.ts | 26 +++- .../src/ngtsc/typecheck/src/context.ts | 88 +++---------- .../src/ngtsc/typecheck/src/diagnostics.ts | 21 ++- .../src/ngtsc/typecheck/src/dom.ts | 92 +++++++++++++ .../src/ngtsc/typecheck/src/source.ts | 78 +++++++++++ .../ngtsc/typecheck/src/type_check_block.ts | 74 +++++++++-- .../ngtsc/typecheck/src/type_check_file.ts | 7 +- .../ngtsc/typecheck/test/diagnostics_spec.ts | 4 +- .../src/ngtsc/typecheck/test/test_utils.ts | 27 +++- .../typecheck/test/type_check_block_spec.ts | 57 +++++--- .../test/ngtsc/fake_core/index.ts | 3 + .../test/ngtsc/template_typecheck_spec.ts | 123 ++++++++++++++++++ packages/compiler/src/compiler.ts | 1 + 22 files changed, 574 insertions(+), 122 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/src/source.ts diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 55c511ee2b..2b7ba63c5a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler'; +import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler'; import * as ts from 'typescript'; import {CycleAnalyzer} from '../../cycles'; @@ -396,6 +396,7 @@ export class ComponentDecoratorHandler implements const matcher = new SelectorMatcher(); const pipes = new Map>>(); + let schemas: SchemaMetadata[] = []; const scope = this.scopeReader.getScopeForComponent(node); if (scope !== null) { @@ -410,10 +411,12 @@ export class ComponentDecoratorHandler implements } pipes.set(name, ref as Reference>); } + schemas = scope.schemas; } const bound = new R3TargetBinder(matcher).bind({template: template.nodes}); - ctx.addTemplate(new Reference(node), bound, pipes, meta.templateSourceMapping, template.file); + ctx.addTemplate( + new Reference(node), bound, pipes, schemas, meta.templateSourceMapping, template.file); } resolve(node: ClassDeclaration, analysis: ComponentHandlerData): ResolveResult { 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 84671854dd..95b485a337 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, R3Identifiers, R3InjectorMetadata, R3NgModuleMetadata, R3Reference, STRING_TYPE, Statement, WrappedNodeExpr, compileInjector, compileNgModule} from '@angular/compiler'; +import {CUSTOM_ELEMENTS_SCHEMA, Expression, ExternalExpr, InvokeFunctionExpr, LiteralArrayExpr, LiteralExpr, NO_ERRORS_SCHEMA, R3Identifiers, R3InjectorMetadata, R3NgModuleMetadata, R3Reference, STRING_TYPE, SchemaMetadata, Statement, WrappedNodeExpr, compileInjector, compileNgModule} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; @@ -122,6 +122,45 @@ export class NgModuleDecoratorHandler implements DecoratorHandler[]; imports: Reference[]; exports: Reference[]; + schemas: SchemaMetadata[]; } /** diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 76d4d83cac..0e843cd776 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -59,6 +59,7 @@ export class DtsMetadataReader implements MetadataReader { this.checker, exportMetadata, ref.ownedByModuleGuess, resolutionContext), imports: extractReferencesFromType( this.checker, importMetadata, ref.ownedByModuleGuess, resolutionContext), + schemas: [], }; } diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index bc74db24b5..57e0e706fe 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -400,7 +400,9 @@ export class NgtscProgram implements api.Program { applyTemplateContextGuards: true, checkQueries: false, checkTemplateBodies: true, - checkTypeOfBindings: true, + checkTypeOfInputBindings: true, + // Even in full template type-checking mode, DOM binding checks are not quite ready yet. + checkTypeOfDomBindings: false, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -409,7 +411,8 @@ export class NgtscProgram implements api.Program { applyTemplateContextGuards: false, checkQueries: false, checkTemplateBodies: false, - checkTypeOfBindings: false, + checkTypeOfInputBindings: false, + checkTypeOfDomBindings: false, checkTypeOfPipes: false, strictSafeNavigationTypes: false, }; diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 6b1ca3a797..48318a607b 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ExternalExpr} from '@angular/compiler'; +import {ExternalExpr, SchemaMetadata} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, makeDiagnostic} from '../../diagnostics'; @@ -28,6 +28,7 @@ export interface LocalNgModuleData { export interface LocalModuleScope extends ExportScope { compilation: ScopeData; reexports: Reexport[]|null; + schemas: SchemaMetadata[]; } /** @@ -375,6 +376,7 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop }, exported, reexports, + schemas: ngModule.schemas, }; this.cache.set(ref.node, scope); return scope; diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 37fbff8e0c..008f17199b 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -52,6 +52,7 @@ describe('LocalModuleScopeRegistry', () => { imports: [], declarations: [Dir1, Dir2, Pipe1], exports: [Dir1, Pipe1], + schemas: [], }); const scope = scopeRegistry.getScopeOfModule(Module.node) !; @@ -67,18 +68,21 @@ describe('LocalModuleScopeRegistry', () => { imports: [ModuleB], declarations: [DirA], exports: [], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleB.node), exports: [ModuleC, DirB], declarations: [DirB], - imports: [] + imports: [], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleC.node), declarations: [DirCI, DirCE], exports: [DirCE], - imports: [] + imports: [], + schemas: [], }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !; @@ -94,12 +98,14 @@ describe('LocalModuleScopeRegistry', () => { exports: [ModuleB], imports: [], declarations: [], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleB.node), declarations: [Dir], exports: [Dir], imports: [], + schemas: [], }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !; @@ -115,18 +121,21 @@ describe('LocalModuleScopeRegistry', () => { declarations: [DirA, DirA], imports: [ModuleB, ModuleC], exports: [DirA, DirA, DirB, ModuleB], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleB.node), declarations: [DirB], imports: [], exports: [DirB], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleC.node), declarations: [], imports: [], exports: [ModuleB], + schemas: [], }); const scope = scopeRegistry.getScopeOfModule(ModuleA.node) !; @@ -149,6 +158,7 @@ describe('LocalModuleScopeRegistry', () => { exports: [], imports: [], declarations: [DirInModule], + schemas: [], }); const scope = scopeRegistry.getScopeOfModule(Module.node) !; @@ -163,12 +173,14 @@ describe('LocalModuleScopeRegistry', () => { exports: [Dir], imports: [ModuleB], declarations: [], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleB.node), declarations: [Dir], exports: [Dir], imports: [], + schemas: [], }); const scopeA = scopeRegistry.getScopeOfModule(ModuleA.node) !; @@ -183,12 +195,14 @@ describe('LocalModuleScopeRegistry', () => { exports: [Dir], imports: [], declarations: [], + schemas: [], }); metaRegistry.registerNgModuleMetadata({ ref: new Reference(ModuleB.node), declarations: [Dir], exports: [Dir], imports: [], + schemas: [], }); expect(scopeRegistry.getScopeOfModule(ModuleA.node)).toBe(null); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel index 133b590583..7c3d05ba9a 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel @@ -8,6 +8,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/metadata", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index ce0ae95d33..1658237e0d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {BoundTarget, DirectiveMeta} from '@angular/compiler'; +import {BoundTarget, DirectiveMeta, SchemaMetadata} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; @@ -45,6 +45,11 @@ export interface TypeCheckBlockMetadata { * Pipes used in the template of the component. */ pipes: Map>>; + + /** + * Schemas that apply to this template. + */ + schemas: SchemaMetadata[]; } export interface TypeCtorMetadata { @@ -72,8 +77,23 @@ export interface TypeCheckingConfig { * checked, but not the assignment of the resulting type to the `input` property of whichever * directive or component is receiving the binding. If set to `true`, both sides of the assignment * are checked. + * + * This flag only affects bindings to components/directives. Bindings to the DOM are checked if + * `checkTypeOfDomBindings` is set. */ - checkTypeOfBindings: boolean; + checkTypeOfInputBindings: boolean; + + /** + * Whether to check the left-hand side type of binding operations to DOM properties. + * + * As `checkTypeOfBindings`, but only applies to bindings to DOM properties. + * + * This does not affect the use of the `DomSchemaChecker` to validate the template against the DOM + * schema. Rather, this flag is an experimental, not yet complete feature which uses the + * lib.dom.d.ts DOM typings in TypeScript to validate that DOM bindings are of the correct type + * for assignability to the underlying DOM element properties. + */ + checkTypeOfDomBindings: boolean; /** * Whether to include type information from pipes in the type-checking operation. @@ -154,4 +174,4 @@ export interface ExternalTemplateSourceMapping { node: ts.Expression; template: string; templateUrl: string; -} +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index cd8c6ca513..3aa4e3dd4b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {BoundTarget, ParseLocation, ParseSourceFile, ParseSourceSpan} from '@angular/compiler'; +import {BoundTarget, ParseSourceFile, SchemaMetadata} from '@angular/compiler'; import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; @@ -15,12 +15,13 @@ import {ClassDeclaration} from '../../reflection'; import {ImportManager} from '../../translator'; import {TemplateSourceMapping, TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCheckingConfig, TypeCtorMetadata} from './api'; -import {SourceLocation, TcbSourceResolver, shouldReportDiagnostic, translateDiagnostic} from './diagnostics'; +import {shouldReportDiagnostic, translateDiagnostic} from './diagnostics'; +import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom'; import {Environment} from './environment'; import {TypeCheckProgramHost} from './host'; -import {computeLineStartsMap, getLineAndCharacterFromPosition} from './line_mappings'; +import {TcbSourceManager} from './source'; import {generateTypeCheckBlock, requiresInlineTypeCheckBlock} from './type_check_block'; -import {TypeCheckFile, typeCheckFilePath} from './type_check_file'; +import {TypeCheckFile} from './type_check_file'; import {generateInlineTypeCtor, requiresInlineTypeCtor} from './type_constructor'; @@ -53,14 +54,9 @@ export class TypeCheckContext { */ private typeCtorPending = new Set(); + private sourceManager = new TcbSourceManager(); - private nextTcbId: number = 1; - /** - * This map keeps track of all template sources that have been type-checked by the id that is - * attached to a TCB's function declaration as leading trivia. This enables translation of - * diagnostics produced for TCB code to their source location in the template. - */ - private templateSources = new Map(); + private domSchemaChecker = new RegistryDomSchemaChecker(this.sourceManager); /** * Record a template for the given component `node`, with a `SelectorMatcher` for directive @@ -74,10 +70,9 @@ export class TypeCheckContext { ref: Reference>, boundTarget: BoundTarget, pipes: Map>>, - sourceMapping: TemplateSourceMapping, file: ParseSourceFile): void { - const id = `tcb${this.nextTcbId++}`; - this.templateSources.set(id, new TemplateSource(sourceMapping, file)); - + schemas: SchemaMetadata[], sourceMapping: TemplateSourceMapping, + file: ParseSourceFile): void { + const id = this.sourceManager.captureSource(sourceMapping, file); // Get all of the directives used in the template and record type constructors for all of them. for (const dir of boundTarget.getUsedDirectives()) { const dirRef = dir.ref as Reference>; @@ -99,14 +94,14 @@ export class TypeCheckContext { } } - + const tcbMetadata: TypeCheckBlockMetadata = {id, boundTarget, pipes, schemas}; if (requiresInlineTypeCheckBlock(ref.node)) { // This class didn't meet the requirements for external type checking, so generate an inline // TCB for the class. - this.addInlineTypeCheckBlock(ref, {id, boundTarget, pipes}); + this.addInlineTypeCheckBlock(ref, tcbMetadata); } else { // The class can be type-checked externally as normal. - this.typeCheckFile.addTypeCheckBlock(ref, {id, boundTarget, pipes}); + this.typeCheckFile.addTypeCheckBlock(ref, tcbMetadata, this.domSchemaChecker); } } @@ -205,27 +200,11 @@ export class TypeCheckContext { rootNames: originalProgram.getRootFileNames(), }); - const tcbResolver: TcbSourceResolver = { - getSourceMapping: (id: string): TemplateSourceMapping => { - if (!this.templateSources.has(id)) { - throw new Error(`Unexpected unknown TCB ID: ${id}`); - } - return this.templateSources.get(id) !.mapping; - }, - sourceLocationToSpan: (location: SourceLocation): ParseSourceSpan | null => { - if (!this.templateSources.has(location.id)) { - return null; - } - const templateSource = this.templateSources.get(location.id) !; - return templateSource.toParseSourceSpan(location.start, location.end); - }, - }; - const diagnostics: ts.Diagnostic[] = []; const collectDiagnostics = (diags: readonly ts.Diagnostic[]): void => { for (const diagnostic of diags) { if (shouldReportDiagnostic(diagnostic)) { - const translated = translateDiagnostic(diagnostic, tcbResolver); + const translated = translateDiagnostic(diagnostic, this.sourceManager); if (translated !== null) { diagnostics.push(translated); @@ -238,6 +217,8 @@ export class TypeCheckContext { collectDiagnostics(typeCheckProgram.getSemanticDiagnostics(sf)); } + diagnostics.push(...this.domSchemaChecker.diagnostics); + return { diagnostics, program: typeCheckProgram, @@ -252,37 +233,7 @@ export class TypeCheckContext { this.opMap.set(sf, []); } const ops = this.opMap.get(sf) !; - ops.push(new TcbOp(ref, tcbMeta, this.config)); - } -} - - -/** - * Represents the source of a template that was processed during type-checking. This information is - * used when translating parse offsets in diagnostics back to their original line/column location. - */ -class TemplateSource { - private lineStarts: number[]|null = null; - - constructor(readonly mapping: TemplateSourceMapping, private file: ParseSourceFile) {} - - toParseSourceSpan(start: number, end: number): ParseSourceSpan { - const startLoc = this.toParseLocation(start); - const endLoc = this.toParseLocation(end); - return new ParseSourceSpan(startLoc, endLoc); - } - - private toParseLocation(position: number) { - const lineStarts = this.acquireLineStarts(); - const {line, character} = getLineAndCharacterFromPosition(lineStarts, position); - return new ParseLocation(this.file, position, line, character); - } - - private acquireLineStarts(): number[] { - if (this.lineStarts === null) { - this.lineStarts = computeLineStartsMap(this.file.content); - } - return this.lineStarts; + ops.push(new TcbOp(ref, tcbMeta, this.config, this.domSchemaChecker)); } } @@ -313,7 +264,8 @@ interface Op { class TcbOp implements Op { constructor( readonly ref: Reference>, - readonly meta: TypeCheckBlockMetadata, readonly config: TypeCheckingConfig) {} + readonly meta: TypeCheckBlockMetadata, readonly config: TypeCheckingConfig, + readonly domSchemaChecker: DomSchemaChecker) {} /** * Type check blocks are inserted immediately after the end of the component class. @@ -324,7 +276,7 @@ class TcbOp implements Op { string { const env = new Environment(this.config, im, refEmitter, sf); const fnName = ts.createIdentifier(`_tcb_${this.ref.node.pos}`); - const fn = generateTypeCheckBlock(env, this.ref, fnName, this.meta); + const fn = generateTypeCheckBlock(env, this.ref, fnName, this.meta, this.domSchemaChecker); return printer.printNode(ts.EmitHint.Unspecified, fn, sf); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 08b0a5e650..7e94aca70b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -149,17 +149,28 @@ export function translateDiagnostic( } const mapping = resolver.getSourceMapping(sourceLocation.id); + return makeTemplateDiagnostic( + mapping, span, diagnostic.category, diagnostic.code, diagnostic.messageText); +} + +/** + * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template. + */ +export function makeTemplateDiagnostic( + mapping: TemplateSourceMapping, span: ParseSourceSpan, category: ts.DiagnosticCategory, + code: number, messageText: string | ts.DiagnosticMessageChain): ts.Diagnostic { if (mapping.type === 'direct') { // For direct mappings, the error is shown inline as ngtsc was able to pinpoint a string // constant within the `@Component` decorator for the template. This allows us to map the error // directly into the bytes of the source file. return { source: 'ngtsc', + code, + category, + messageText, file: mapping.node.getSourceFile(), start: span.start.offset, length: span.end.offset - span.start.offset, - code: diagnostic.code, messageText, - category: diagnostic.category, }; } else if (mapping.type === 'indirect' || mapping.type === 'external') { // For indirect mappings (template was declared inline, but ngtsc couldn't map it directly @@ -180,12 +191,12 @@ export function translateDiagnostic( return { source: 'ngtsc', + category, + code, + messageText, file: sf, start: span.start.offset, length: span.end.offset - span.start.offset, - messageText: diagnostic.messageText, - category: diagnostic.category, - code: diagnostic.code, // Show a secondary message indicating the component whose template contains the error. relatedInformation: [{ category: ts.DiagnosticCategory.Message, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts new file mode 100644 index 0000000000..798a3e8288 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts @@ -0,0 +1,92 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {DomElementSchemaRegistry, ParseSourceSpan, SchemaMetadata, TmplAstElement} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {ErrorCode} from '../../diagnostics'; + +import {TcbSourceResolver, makeTemplateDiagnostic} from './diagnostics'; + +const REGISTRY = new DomElementSchemaRegistry(); + +/** + * Checks every non-Angular element/property processed in a template and potentially produces + * `ts.Diagnostic`s related to improper usage. + * + * A `DomSchemaChecker`'s job is to check DOM nodes and their attributes written used in templates + * and produce `ts.Diagnostic`s if the nodes don't conform to the DOM specification. It acts as a + * collector for these diagnostics, and can be queried later to retrieve the list of any that have + * been generated. + */ +export interface DomSchemaChecker { + /** + * Get the `ts.Diagnostic`s that have been generated via `checkElement` and `checkProperty` calls + * thus far. + */ + readonly diagnostics: ReadonlyArray; + + /** + * Check a non-Angular element and record any diagnostics about it. + * + * @param id the template ID, suitable for resolution with a `TcbSourceResolver`. + * @param element the element node in question. + * @param schemas any active schemas for the template, which might affect the validity of the + * element. + */ + checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void; + + /** + * Check a property binding on an element and record any diagnostics about it. + * + * @param id the template ID, suitable for resolution with a `TcbSourceResolver`. + * @param element the element node in question. + * @param name the name of the property being checked. + * @param span the source span of the binding. This is redundant with `element.attributes` but is + * passed separately to avoid having to look up the particular property name. + * @param schemas any active schemas for the template, which might affect the validity of the + * property. + */ + checkProperty( + id: string, element: TmplAstElement, name: string, span: ParseSourceSpan, + schemas: SchemaMetadata[]): void; +} + +/** + * Checks non-Angular elements and properties against the `DomElementSchemaRegistry`, a schema + * maintained by the Angular team via extraction from a browser IDL. + */ +export class RegistryDomSchemaChecker { + private _diagnostics: ts.Diagnostic[] = []; + + get diagnostics(): ReadonlyArray { return this._diagnostics; } + + constructor(private resolver: TcbSourceResolver) {} + + checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void { + if (!REGISTRY.hasElement(element.name, schemas)) { + const mapping = this.resolver.getSourceMapping(id); + const diag = makeTemplateDiagnostic( + mapping, element.sourceSpan, ts.DiagnosticCategory.Error, + ErrorCode.SCHEMA_INVALID_ELEMENT, `'${element.name}' is not a valid HTML element.`); + this._diagnostics.push(diag); + } + } + + checkProperty( + id: string, element: TmplAstElement, name: string, span: ParseSourceSpan, + schemas: SchemaMetadata[]): void { + if (!REGISTRY.hasProperty(element.name, name, schemas)) { + const mapping = this.resolver.getSourceMapping(id); + const diag = makeTemplateDiagnostic( + mapping, span, ts.DiagnosticCategory.Error, ErrorCode.SCHEMA_INVALID_ATTRIBUTE, + `'${name}' is not a valid property of <${element.name}>.`); + this._diagnostics.push(diag); + } + } +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/source.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/source.ts new file mode 100644 index 0000000000..fa2f8d4e0e --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/source.ts @@ -0,0 +1,78 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '@angular/compiler'; + +import {TemplateSourceMapping} from './api'; +import {SourceLocation, TcbSourceResolver} from './diagnostics'; +import {computeLineStartsMap, getLineAndCharacterFromPosition} from './line_mappings'; + +/** + * Represents the source of a template that was processed during type-checking. This information is + * used when translating parse offsets in diagnostics back to their original line/column location. + */ +export class TemplateSource { + private lineStarts: number[]|null = null; + + constructor(readonly mapping: TemplateSourceMapping, private file: ParseSourceFile) {} + + toParseSourceSpan(start: number, end: number): ParseSourceSpan { + const startLoc = this.toParseLocation(start); + const endLoc = this.toParseLocation(end); + return new ParseSourceSpan(startLoc, endLoc); + } + + private toParseLocation(position: number) { + const lineStarts = this.acquireLineStarts(); + const {line, character} = getLineAndCharacterFromPosition(lineStarts, position); + return new ParseLocation(this.file, position, line, character); + } + + private acquireLineStarts(): number[] { + if (this.lineStarts === null) { + this.lineStarts = computeLineStartsMap(this.file.content); + } + return this.lineStarts; + } +} + +/** + * Assigns IDs to templates and keeps track of their origins. + * + * Implements `TcbSourceResolver` to resolve the source of a template based on these IDs. + */ +export class TcbSourceManager implements TcbSourceResolver { + private nextTcbId: number = 1; + /** + * This map keeps track of all template sources that have been type-checked by the id that is + * attached to a TCB's function declaration as leading trivia. This enables translation of + * diagnostics produced for TCB code to their source location in the template. + */ + private templateSources = new Map(); + + captureSource(mapping: TemplateSourceMapping, file: ParseSourceFile): string { + const id = `tcb${this.nextTcbId++}`; + this.templateSources.set(id, new TemplateSource(mapping, file)); + return id; + } + + getSourceMapping(id: string): TemplateSourceMapping { + if (!this.templateSources.has(id)) { + throw new Error(`Unexpected unknown TCB ID: ${id}`); + } + return this.templateSources.get(id) !.mapping; + } + + sourceLocationToSpan(location: SourceLocation): ParseSourceSpan|null { + if (!this.templateSources.has(location.id)) { + return null; + } + const templateSource = this.templateSources.get(location.id) !; + return templateSource.toParseSourceSpan(location.start, location.end); + } +} 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 04669baeb7..6e8c71909b 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 @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, PropertyRead, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, ImplicitReceiver, MethodCall, ParseSourceSpan, ParseSpan, PropertyRead, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundText, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; @@ -14,6 +14,7 @@ import {ClassDeclaration} from '../../reflection'; import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api'; import {addParseSpanInfo, addSourceId, toAbsoluteSpan, wrapForDiagnostics} from './diagnostics'; +import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {astToTypescript} from './expression'; import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util'; @@ -33,8 +34,9 @@ import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsC */ export function generateTypeCheckBlock( env: Environment, ref: Reference>, name: ts.Identifier, - meta: TypeCheckBlockMetadata): ts.FunctionDeclaration { - const tcb = new Context(env, meta.boundTarget, meta.pipes); + meta: TypeCheckBlockMetadata, domSchemaChecker: DomSchemaChecker): ts.FunctionDeclaration { + const tcb = + new Context(env, domSchemaChecker, meta.id, meta.boundTarget, meta.pipes, meta.schemas); const scope = Scope.forNodes(tcb, null, tcb.boundTarget.target.template !); const ctxRawType = env.referenceType(ref); if (!ts.isTypeReferenceNode(ctxRawType)) { @@ -299,6 +301,49 @@ class TcbDirectiveOp extends TcbOp { } } +/** + * A `TcbOp` which feeds elements and unclaimed properties to the `DomSchemaChecker`. + * + * The DOM schema is not checked via TCB code generation. Instead, the `DomSchemaChecker` ingests + * elements and property bindings and accumulates synthetic `ts.Diagnostic`s out-of-band. These are + * later merged with the diagnostics generated from the TCB. + * + * For convenience, the TCB iteration of the template is used to drive the `DomSchemaChecker` via + * the `TcbDomSchemaCheckerOp`. + */ +class TcbDomSchemaCheckerOp extends TcbOp { + constructor( + private tcb: Context, private element: TmplAstElement, private checkElement: boolean, + private claimedInputs: Set) { + super(); + } + + execute(): ts.Expression|null { + if (this.checkElement) { + this.tcb.domSchemaChecker.checkElement(this.tcb.id, this.element, this.tcb.schemas); + } + + // TODO(alxhub): this could be more efficient. + for (const binding of this.element.inputs) { + if (binding.type === BindingType.Property && this.claimedInputs.has(binding.name)) { + // Skip this binding as it was claimed by a directive. + continue; + } + + if (binding.type === BindingType.Property) { + if (binding.name !== 'style' && binding.name !== 'class') { + // A direct binding to a property. + const propertyName = ATTR_TO_PROP[binding.name] || binding.name; + this.tcb.domSchemaChecker.checkProperty( + this.tcb.id, this.element, propertyName, binding.sourceSpan, this.tcb.schemas); + } + } + } + return null; + } +} + + /** * Mapping between attributes names that don't correspond to their element property names. * Note: this mapping has to be kept in sync with the equally named mapping in the runtime. @@ -317,6 +362,9 @@ const ATTR_TO_PROP: {[name: string]: string} = { * not attributed to any directive or component, and are instead processed against the HTML element * itself. * + * Currently, only the expressions of these bindings are checked. The targets of the bindings are + * checked against the DOM schema via a `TcbDomSchemaCheckerOp`. + * * Executing this operation returns nothing. */ class TcbUnclaimedInputsOp extends TcbOp { @@ -343,11 +391,11 @@ class TcbUnclaimedInputsOp extends TcbOp { // If checking the type of bindings is disabled, cast the resulting expression to 'any' before // the assignment. - if (!this.tcb.env.config.checkTypeOfBindings) { + if (!this.tcb.env.config.checkTypeOfInputBindings) { expr = tsCastToAny(expr); } - if (binding.type === BindingType.Property) { + if (this.tcb.env.config.checkTypeOfDomBindings && binding.type === BindingType.Property) { if (binding.name !== 'style' && binding.name !== 'class') { // A direct binding to a property. const propertyName = ATTR_TO_PROP[binding.name] || binding.name; @@ -390,8 +438,10 @@ export class Context { private nextId = 1; constructor( - readonly env: Environment, readonly boundTarget: BoundTarget, - private pipes: Map>>) {} + readonly env: Environment, readonly domSchemaChecker: DomSchemaChecker, readonly id: string, + readonly boundTarget: BoundTarget, + private pipes: Map>>, + readonly schemas: SchemaMetadata[]) {} /** * Allocate a new variable name for use within the `Context`. @@ -644,6 +694,8 @@ class Scope { // to add them if needed. if (node instanceof TmplAstElement) { this.opQueue.push(new TcbUnclaimedInputsOp(this.tcb, this, node, claimedInputs)); + this.opQueue.push( + new TcbDomSchemaCheckerOp(this.tcb, node, /* checkElement */ true, claimedInputs)); } return; } @@ -667,6 +719,12 @@ class Scope { } this.opQueue.push(new TcbUnclaimedInputsOp(this.tcb, this, node, claimedInputs)); + // If there are no directives which match this element, then it's a "plain" DOM element (or a + // web component), and should be checked against the DOM schema. If any directives match, + // we must assume that the element could be custom (either a component, or a directive like + // ) and shouldn't validate the element name itself. + const checkElement = directives.length === 0; + this.opQueue.push(new TcbDomSchemaCheckerOp(this.tcb, node, checkElement, claimedInputs)); } } } @@ -722,7 +780,7 @@ function tcbCallTypeCtor( // Construct an array of `ts.PropertyAssignment`s for each input of the directive that has a // matching binding. const members = bindings.map(({field, expression, sourceSpan}) => { - if (!tcb.env.config.checkTypeOfBindings) { + if (!tcb.env.config.checkTypeOfInputBindings) { expression = tsCastToAny(expression); } const assignment = ts.createPropertyAssignment(field, wrapForDiagnostics(expression)); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index 9f1fb7c708..a2ee6c787b 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -13,9 +13,11 @@ import {ClassDeclaration} from '../../reflection'; import {ImportManager} from '../../translator'; import {TypeCheckBlockMetadata, TypeCheckingConfig} from './api'; +import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {generateTypeCheckBlock} from './type_check_block'; + /** * An `Environment` representing the single type-checking file into which most (if not all) Type * Check Blocks (TCBs) will be generated. @@ -35,9 +37,10 @@ export class TypeCheckFile extends Environment { } addTypeCheckBlock( - ref: Reference>, meta: TypeCheckBlockMetadata): void { + ref: Reference>, meta: TypeCheckBlockMetadata, + domSchemaChecker: DomSchemaChecker): void { const fnId = ts.createIdentifier(`_tcb${this.nextTcbId++}`); - const fn = generateTypeCheckBlock(this, ref, fnId, meta); + const fn = generateTypeCheckBlock(this, ref, fnId, meta, domSchemaChecker); this.tcbStatements.push(fn); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 5b133cb3f5..49cf660e73 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -133,7 +133,7 @@ runInEachFileSystem(() => { }); it('interprets interpolation as strings', () => { - const messages = diagnose(`
`, ` + const messages = diagnose(`
`, ` class TestComponent { person: {}; }`); @@ -149,8 +149,8 @@ runInEachFileSystem(() => { }`); expect(messages).toEqual([ - `synthetic.html(1, 6): Property 'srcc' does not exist on type 'HTMLImageElement'. Did you mean 'src'?`, `synthetic.html(1, 29): Property 'heihgt' does not exist on type 'TestComponent'. Did you mean 'height'?`, + `synthetic.html(1, 6): 'srcc' is not a valid property of .`, ]); }); 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 a0876867e2..c0d05b661f 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, R3TargetBinder, SelectorMatcher, parseTemplate} from '@angular/compiler'; +import {CssSelector, ParseSourceFile, ParseSourceSpan, R3TargetBinder, SchemaMetadata, SelectorMatcher, TmplAstElement, parseTemplate} from '@angular/compiler'; import * as ts from 'typescript'; import {AbsoluteFsPath, LogicalFileSystem, absoluteFrom} from '../../file_system'; @@ -17,6 +17,7 @@ import {makeProgram} from '../../testing'; import {getRootDirs} from '../../util/src/typescript'; import {TemplateSourceMapping, TypeCheckBlockMetadata, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../src/api'; import {TypeCheckContext} from '../src/context'; +import {DomSchemaChecker} from '../src/dom'; import {Environment} from '../src/environment'; import {generateTypeCheckBlock} from '../src/type_check_block'; @@ -117,7 +118,10 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = { applyTemplateContextGuards: true, checkQueries: false, checkTemplateBodies: true, - checkTypeOfBindings: true, + checkTypeOfInputBindings: true, + // Feature is still in development. + // TODO(alxhub): enable when DOM checking via lib.dom.d.ts is further along. + checkTypeOfDomBindings: false, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -150,12 +154,13 @@ export function tcb( const binder = new R3TargetBinder(matcher); const boundTarget = binder.bind({template: nodes}); - const meta: TypeCheckBlockMetadata = {boundTarget, pipes, id: 'tcb'}; + const meta: TypeCheckBlockMetadata = {boundTarget, pipes, id: 'tcb', schemas: []}; config = config || { applyTemplateContextGuards: true, checkQueries: false, - checkTypeOfBindings: true, + checkTypeOfInputBindings: true, + checkTypeOfDomBindings: false, checkTypeOfPipes: true, checkTemplateBodies: true, strictSafeNavigationTypes: true, @@ -165,7 +170,8 @@ export function tcb( }; const tcb = generateTypeCheckBlock( - FakeEnvironment.newFake(config), new Reference(clazz), ts.createIdentifier('Test_TCB'), meta); + FakeEnvironment.newFake(config), new Reference(clazz), ts.createIdentifier('Test_TCB'), meta, + new NoopSchemaChecker()); const removeComments = !options.emitSpans; const res = ts.createPrinter({removeComments}).printNode(ts.EmitHint.Unspecified, tcb, sf); @@ -227,7 +233,7 @@ export function typecheck( node: clazz.node.name, }; - ctx.addTemplate(clazz, boundTarget, pipes, sourceMapping, templateFile); + ctx.addTemplate(clazz, boundTarget, pipes, [], sourceMapping, templateFile); return ctx.calculateTemplateDiagnostics(program, host, options).diagnostics; } @@ -309,3 +315,12 @@ class FakeEnvironment /* implements Environment */ { return new FakeEnvironment(config) as Environment; } } + +export class NoopSchemaChecker implements DomSchemaChecker { + get diagnostics(): ReadonlyArray { return []; } + + checkElement(id: string, element: TmplAstElement, schemas: SchemaMetadata[]): void {} + checkProperty( + id: string, element: TmplAstElement, name: string, span: ParseSourceSpan, + schemas: SchemaMetadata[]): void {} +} 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 103081c83b..64dcd2b6cc 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 @@ -7,7 +7,8 @@ */ import {TypeCheckingConfig} from '../src/api'; -import {TestDeclaration, TestDirective, tcb} from './test_utils'; + +import {ALL_ENABLED_CONFIG, TestDeclaration, TestDirective, tcb} from './test_utils'; describe('type check blocks', () => { @@ -34,19 +35,26 @@ describe('type check blocks', () => { expect(tcb(TEMPLATE)).toContain('((ctx).a)[(ctx).b];'); }); - it('should translate unclaimed bindings to their property equivalent', () => { - const TEMPLATE = ``; - expect(tcb(TEMPLATE)).toContain('_t1.htmlFor = ("test");'); - }); - it('should handle empty bindings', () => { - const TEMPLATE = ``; - expect(tcb(TEMPLATE)).toContain('_t1.type = (undefined);'); + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + inputs: {inputA: 'inputA'}, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: (undefined)'); }); it('should handle bindings without value', () => { - const TEMPLATE = ``; - expect(tcb(TEMPLATE)).toContain('_t1.type = (undefined);'); + const TEMPLATE = `
`; + const DIRECTIVES: TestDeclaration[] = [{ + type: 'directive', + name: 'DirA', + selector: '[dir-a]', + inputs: {inputA: 'inputA'}, + }]; + expect(tcb(TEMPLATE, DIRECTIVES)).toContain('inputA: (undefined)'); }); it('should handle implicit vars on ng-template', () => { @@ -142,6 +150,15 @@ describe('type check blocks', () => { expect(block).toContain('((ctx).a as any);'); }); + describe('experimental DOM checking via lib.dom.d.ts', () => { + it('should translate unclaimed bindings to their property equivalent', () => { + const TEMPLATE = ``; + const CONFIG = {...ALL_ENABLED_CONFIG, checkTypeOfDomBindings: true}; + expect(tcb(TEMPLATE, /* declarations */ undefined, CONFIG)) + .toContain('_t1.htmlFor = ("test");'); + }); + }); + describe('template guards', () => { it('should emit invocation guards', () => { const DIRECTIVES: TestDeclaration[] = [{ @@ -189,7 +206,8 @@ describe('type check blocks', () => { applyTemplateContextGuards: true, checkQueries: false, checkTemplateBodies: true, - checkTypeOfBindings: true, + checkTypeOfInputBindings: true, + checkTypeOfDomBindings: false, checkTypeOfPipes: true, strictSafeNavigationTypes: true, }; @@ -203,7 +221,8 @@ describe('type check blocks', () => { expect(block).toContain(GUARD_APPLIED); }); it('should not apply template context guards when disabled', () => { - const DISABLED_CONFIG = {...BASE_CONFIG, applyTemplateContextGuards: false}; + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, applyTemplateContextGuards: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).not.toContain(GUARD_APPLIED); }); @@ -217,7 +236,7 @@ describe('type check blocks', () => { expect(block).toContain('(ctx).a;'); }); it('should not descend into template bodies when disabled', () => { - const DISABLED_CONFIG = {...BASE_CONFIG, checkTemplateBodies: false}; + const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTemplateBodies: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).not.toContain('(ctx).a;'); }); @@ -229,13 +248,14 @@ describe('type check blocks', () => { it('should check types of bindings when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('Dir.ngTypeCtor({ dirInput: ((ctx).a) })'); - expect(block).toContain('.nonDirInput = ((ctx).a);'); + expect(block).toContain('(ctx).a;'); }); it('should not check types of bindings when disabled', () => { - const DISABLED_CONFIG = {...BASE_CONFIG, checkTypeOfBindings: false}; + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfInputBindings: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('Dir.ngTypeCtor({ dirInput: (((ctx).a as any)) })'); - expect(block).toContain('.nonDirInput = (((ctx).a as any));'); + expect(block).toContain('((ctx).a as any);'); }); }); @@ -253,7 +273,7 @@ describe('type check blocks', () => { 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 = {...BASE_CONFIG, checkTypeOfPipes: false}; + 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);'); }); @@ -268,7 +288,8 @@ describe('type check blocks', () => { expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.b : undefined)'); }); it('should use an \'any\' type for safe navigation operations when disabled', () => { - const DISABLED_CONFIG = {...BASE_CONFIG, strictSafeNavigationTypes: false}; + const DISABLED_CONFIG: + TypeCheckingConfig = {...BASE_CONFIG, strictSafeNavigationTypes: false}; const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.method() : null as any)'); expect(block).toContain('(((ctx).a) != null ? ((ctx).a)!.b : null as any)'); diff --git a/packages/compiler-cli/test/ngtsc/fake_core/index.ts b/packages/compiler-cli/test/ngtsc/fake_core/index.ts index 3e5fc4eba6..ddc353bf51 100644 --- a/packages/compiler-cli/test/ngtsc/fake_core/index.ts +++ b/packages/compiler-cli/test/ngtsc/fake_core/index.ts @@ -77,3 +77,6 @@ export enum ChangeDetectionStrategy { OnPush = 0, Default = 1 } + +export const CUSTOM_ELEMENTS_SCHEMA: any = false; +export const NO_ERRORS_SCHEMA: any = false; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 64a9f3f8ba..b19425602c 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -346,6 +346,129 @@ export declare class CommonModule { expect(diags[1].length).toEqual(15); }); + describe('legacy schema checking with the DOM schema', () => { + beforeEach( + () => { env.tsconfig({ivyTemplateTypeCheck: true, fullTemplateTypeCheck: false}); }); + + it('should check for unknown elements', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: 'test', + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`'foo' is not a valid HTML element.`); + }); + + it('should check for unknown properties', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: '
test
', + }) + export class FooCmp {} + @NgModule({ + declarations: [FooCmp], + }) + export class FooModule {} + `); + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(diags[0].messageText).toBe(`'foo' is not a valid property of
.`); + }); + + it('should convert property names when binding special properties', () => { + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + @Component({ + selector: 'blah', + template: '