diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 3ed958274b..4e8b6f344c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -46,6 +46,16 @@ 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. + */ +export function ignoreDiagnostics(node: ts.Node): void { + ts.addSyntheticTrailingComment( + node, ts.SyntaxKind.MultiLineCommentTrivia, IGNORE_MARKER, /* hasTrailingNewLine */ false); +} + /** * Adds a synthetic comment to the expression that represents the parse span of the provided node. * This comment can later be retrieved as trivia of a node to recover original source locations. @@ -214,17 +224,20 @@ interface SourceLocation { span: AbsoluteSourceSpan; } +/** + * Traverses up the AST starting from the given node to extract the source location from comments + * that have been emitted into the TCB. If the node does not exist within a TCB, or if an ignore + * marker comment is found up the tree, this function returns null. + */ function findSourceLocation(node: ts.Node, sourceFile: ts.SourceFile): SourceLocation|null { // Search for comments until the TCB's function declaration is encountered. while (node !== undefined && !ts.isFunctionDeclaration(node)) { - const span = - ts.forEachTrailingCommentRange(sourceFile.text, node.getEnd(), (pos, end, kind) => { - if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { - return null; - } - const commentText = sourceFile.text.substring(pos, end); - return parseSourceSpanComment(commentText); - }) || null; + if (hasIgnoreMarker(node, sourceFile)) { + // There's an ignore marker on this node, so the diagnostic should not be reported. + return null; + } + + const span = readSpanComment(sourceFile, node); if (span !== null) { // Once the positional information has been extracted, search further up the TCB to extract // the unique id that is attached with the TCB's function declaration. @@ -243,17 +256,21 @@ function findSourceLocation(node: ts.Node, sourceFile: ts.SourceFile): SourceLoc function getTemplateId(node: ts.Node, sourceFile: ts.SourceFile): TemplateId|null { // Walk up to the function declaration of the TCB, the file information is attached there. - let tcb = node; - while (!ts.isFunctionDeclaration(tcb)) { - tcb = tcb.parent; + while (!ts.isFunctionDeclaration(node)) { + if (hasIgnoreMarker(node, sourceFile)) { + // There's an ignore marker on this node, so the diagnostic should not be reported. + return null; + } + node = node.parent; // Bail once we have reached the root. - if (tcb === undefined) { + if (node === undefined) { return null; } } - return ts.forEachLeadingCommentRange(sourceFile.text, tcb.getFullStart(), (pos, end, kind) => { + const start = node.getFullStart(); + return ts.forEachLeadingCommentRange(sourceFile.text, start, (pos, end, kind) => { if (kind !== ts.SyntaxKind.MultiLineCommentTrivia) { return null; } @@ -262,13 +279,29 @@ function getTemplateId(node: ts.Node, sourceFile: ts.SourceFile): TemplateId|nul }) as TemplateId || null; } -const parseSpanComment = /^\/\*(\d+),(\d+)\*\/$/; +const parseSpanComment = /^(\d+),(\d+)$/; -function parseSourceSpanComment(commentText: string): AbsoluteSourceSpan|null { - const match = commentText.match(parseSpanComment); - if (match === null) { - return null; - } +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]); + 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 99a6365175..2bb19db850 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, ASTWithSource, AstVisitor, Binary, BindingPipe, Chain, Conditional, import * as ts from 'typescript'; import {TypeCheckingConfig} from './api'; -import {addParseSpanInfo, wrapForDiagnostics} from './diagnostics'; +import {addParseSpanInfo, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics'; export const NULL_AS_ANY = ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); @@ -222,11 +222,13 @@ class AstTranslator implements AstVisitor { // See the comment in SafePropertyRead above for an explanation of the need for the non-null // assertion here. const receiver = wrapForDiagnostics(this.translate(ast.receiver)); + const guard = ts.getMutableClone(receiver); + ignoreDiagnostics(guard); const method = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const args = ast.args.map(expr => this.translate(expr)); const expr = ts.createCall(method, undefined, args); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; - const node = safeTernary(receiver, expr, whenNull); + const node = safeTernary(guard, expr, whenNull); addParseSpanInfo(node, ast.sourceSpan); return node; } @@ -237,9 +239,11 @@ class AstTranslator implements AstVisitor { // assertion is necessary because in practice 'a' may be a method call expression, which won't // have a narrowed type when repeated in the ternary true branch. const receiver = wrapForDiagnostics(this.translate(ast.receiver)); + const guard = ts.getMutableClone(receiver); + ignoreDiagnostics(guard); const expr = ts.createPropertyAccess(ts.createNonNullExpression(receiver), ast.name); const whenNull = this.config.strictSafeNavigationTypes ? UNDEFINED : NULL_AS_ANY; - const node = safeTernary(receiver, expr, whenNull); + const node = safeTernary(guard, expr, whenNull); addParseSpanInfo(node, ast.sourceSpan); return node; } 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 b358ef3300..a71944c421 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 @@ -13,7 +13,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; import {TemplateId, TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api'; -import {addParseSpanInfo, addTemplateId, wrapForDiagnostics} from './diagnostics'; +import {addParseSpanInfo, addTemplateId, ignoreDiagnostics, wrapForDiagnostics} from './diagnostics'; import {DomSchemaChecker} from './dom'; import {Environment} from './environment'; import {NULL_AS_ANY, astToTypescript} from './expression'; @@ -217,6 +217,10 @@ class TcbTemplateBodyOp extends TcbOp { // If there is such a binding, generate an expression for it. const expr = tcbExpression(boundInput.value, this.tcb, this.scope); + // The expression has already been checked in the type constructor invocation, so + // it should be ignored when used within a template guard. + ignoreDiagnostics(expr); + if (guard.type === 'binding') { // Use the binding expression itself as guard. directiveGuards.push(expr); 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 b00c2efd65..1c281971de 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -43,7 +43,7 @@ runInEachFileSystem(() => { `
{{ render(idx) }}
`, ` class TestComponent { persons: {}[]; - + render(input: string): string { return input; } }`, [ngForDeclaration()], [ngForDts()]); @@ -58,7 +58,7 @@ runInEachFileSystem(() => { `
{{ render(person.namme) }}
`, ` class TestComponent { persons: any; - + render(input: string): string { return input; } }`, [ngForDeclaration()], [ngForDts()]); @@ -194,6 +194,46 @@ runInEachFileSystem(() => { ]); }); + it('does not repeat diagnostics for errors within LHS of safe-navigation operator', () => { + const messages = diagnose(`{{ personn?.name }} {{ personn?.getName() }}`, ` + class TestComponent { + person: { + name: string; + getName: () => string; + } | null; + }`); + + expect(messages).toEqual([ + `synthetic.html(1, 4): Property 'personn' does not exist on type 'TestComponent'. Did you mean 'person'?`, + `synthetic.html(1, 24): Property 'personn' does not exist on type 'TestComponent'. Did you mean 'person'?`, + ]); + }); + + it('does not repeat diagnostics for errors used in template guard expressions', () => { + const messages = diagnose( + `
`, ` + class GuardDir { + static ngTemplateGuard_guard: 'binding'; + } + + class TestComponent { + person: { + name: string; + }; + }`, + [{ + type: 'directive', + name: 'GuardDir', + selector: '[guard]', + inputs: {'guard': 'guard'}, + ngTemplateGuards: [{inputName: 'guard', type: 'binding'}] + }]); + + expect(messages).toEqual([ + `synthetic.html(1, 14): Property 'personn' does not exist on type 'TestComponent'. Did you mean 'person'?`, + ]); + }); + it('does not produce diagnostics for user code', () => { const messages = diagnose(`{{ person.name }}`, ` class TestComponent { @@ -313,7 +353,7 @@ runInEachFileSystem(() => {
-
+ `, ` class TestComponent { 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 81627c40ce..35c3de00a6 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 @@ -97,14 +97,15 @@ describe('type check blocks diagnostics', () => { it('should annotate safe property access', () => { const TEMPLATE = `{{ a?.b }}`; expect(tcbWithSpans(TEMPLATE)) - .toContain('(((ctx).a /*3,4*/) != null ? ((ctx).a /*3,4*/)!.b : undefined) /*3,8*/;'); + .toContain( + '(((ctx).a /*3,4*/) /*ignore*/ != null ? ((ctx).a /*3,4*/)!.b : undefined) /*3,8*/;'); }); it('should annotate safe method calls', () => { const TEMPLATE = `{{ a?.method(b) }}`; expect(tcbWithSpans(TEMPLATE)) .toContain( - '(((ctx).a /*3,4*/) != null ? ((ctx).a /*3,4*/)!.method((ctx).b /*13,14*/) : undefined) /*3,16*/;'); + '(((ctx).a /*3,4*/) /*ignore*/ != null ? ((ctx).a /*3,4*/)!.method((ctx).b /*13,14*/) : undefined) /*3,16*/;'); }); it('should annotate $any casts', () => {