honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler-cli): check split two way binding (#42601) 

Check for split two way binding when output is not declared to make error message clearer.

PR Close #42601
This commit is contained in:
Daniel Trevino 2021-06-22 10:48:27 -05:00 committed by Andrew Kushnir
parent 88b15d572f
commit 81dce5c664
7 changed files with 217 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
goldens/public-api/compiler-cli/error_code.md
View File

@ -50,6 +50,7 @@ export enum ErrorCode {
PIPE_MISSING_NAME = 2002,
SCHEMA_INVALID_ATTRIBUTE = 8002,
SCHEMA_INVALID_ELEMENT = 8001,
SPLIT_TWO_WAY_BINDING = 8007,
SUGGEST_STRICT_TEMPLATES = 10001,
SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002,
// (undocumented)

6
packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
View File

@ -166,6 +166,12 @@ export enum ErrorCode {
*/
DUPLICATE_VARIABLE_DECLARATION = 8006,
/**
* A template has a two way binding (two bindings created by a single syntactial element)
* in which the input and output are going to different places.
*/
SPLIT_TWO_WAY_BINDING = 8007,
/**
* The template type-checking engine would need to generate an inline type check block for a
* component, but the current type-checking environment doesn't support it.

49
packages/compiler-cli/src/ngtsc/typecheck/diagnostics/src/diagnostic.ts
View File

@ -33,21 +33,26 @@ export interface TemplateDiagnostic extends ts.Diagnostic {
export function makeTemplateDiagnostic(
templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan,
category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain,
relatedMessage?: {
relatedMessages?: {
text: string,
span: ParseSourceSpan,
}): TemplateDiagnostic {
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): TemplateDiagnostic {
if (mapping.type === 'direct') {
let relatedInformation: ts.DiagnosticRelatedInformation[]|undefined = undefined;
if (relatedMessage !== undefined) {
relatedInformation = [{
category: ts.DiagnosticCategory.Message,
code: 0,
file: mapping.node.getSourceFile(),
start: relatedMessage.span.start.offset,
length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
messageText: relatedMessage.text,
}];
if (relatedMessages !== undefined) {
relatedInformation = [];
for (const relatedMessage of relatedMessages) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile,
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
});
}
}
// 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
@ -82,15 +87,17 @@ export function makeTemplateDiagnostic(
fileName, mapping.template, ts.ScriptTarget.Latest, false, ts.ScriptKind.JSX);
let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
if (relatedMessage !== undefined) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: sf,
start: relatedMessage.span.start.offset,
length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
messageText: relatedMessage.text,
});
if (relatedMessages !== undefined) {
for (const relatedMessage of relatedMessages) {
relatedInformation.push({
category: ts.DiagnosticCategory.Message,
code: 0,
file: relatedMessage.sourceFile,
start: relatedMessage.start,
length: relatedMessage.end - relatedMessage.start,
messageText: relatedMessage.text,
});
}
}
relatedInformation.push({

71
packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts
View File

@ -6,7 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/
import {BindingPipe, PropertyWrite, TmplAstReference, TmplAstVariable} from '@angular/compiler';
import {BindingPipe, PropertyWrite, TmplAstBoundEvent, TmplAstElement, TmplAstReference, TmplAstVariable} from '@angular/compiler';
import {BoundAttribute} from '@angular/compiler/src/render3/r3_ast';
import * as ts from 'typescript';
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
@ -74,6 +75,13 @@ export interface OutOfBandDiagnosticRecorder {
* type-checking configuration prohibits their usage.
*/
suboptimalTypeInference(templateId: TemplateId, variables: TmplAstVariable[]): void;
/**
* Reports a split two way binding error message.
*/
splitTwoWayBinding(
templateId: TemplateId, input: BoundAttribute, output: TmplAstBoundEvent,
inputConsumer: ClassDeclaration, outputConsumer: ClassDeclaration|TmplAstElement): void;
}
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
@ -133,10 +141,12 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
}
this._diagnostics.push(makeTemplateDiagnostic(
templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, {
ngErrorCode(ErrorCode.WRITE_TO_READ_ONLY_VARIABLE), errorMsg, [{
text: `The variable ${assignment.name} is declared here.`,
span: target.valueSpan || target.sourceSpan,
}));
start: target.valueSpan?.start.offset || target.sourceSpan.start.offset,
end: target.valueSpan?.end.offset || target.sourceSpan.end.offset,
sourceFile: mapping.node.getSourceFile(),
}]));
}
duplicateTemplateVar(
@ -152,10 +162,12 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
// TODO(alxhub): allocate to a tighter span once one is available.
this._diagnostics.push(makeTemplateDiagnostic(
templateId, mapping, variable.sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, {
ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, [{
text: `The variable '${firstDecl.name}' was first declared here.`,
span: firstDecl.sourceSpan,
}));
start: firstDecl.sourceSpan.start.offset,
end: firstDecl.sourceSpan.end.offset,
sourceFile: mapping.node.getSourceFile(),
}]));
}
requiresInlineTcb(templateId: TemplateId, node: ClassDeclaration): void {
@ -211,6 +223,51 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
templateId, mapping, diagnosticVar.keySpan, ts.DiagnosticCategory.Suggestion,
ngErrorCode(ErrorCode.SUGGEST_SUBOPTIMAL_TYPE_INFERENCE), message));
}
splitTwoWayBinding(
templateId: TemplateId, input: BoundAttribute, output: TmplAstBoundEvent,
inputConsumer: ClassDeclaration, outputConsumer: ClassDeclaration|TmplAstElement): void {
const mapping = this.resolver.getSourceMapping(templateId);
const errorMsg = `The property and event halves of the two-way binding '${
input.name}' are not bound to the same target.
Find more at https://angular.io/guide/two-way-binding#how-two-way-binding-works`;
const relatedMessages: {text: string; start: number; end: number;
sourceFile: ts.SourceFile;}[] = [];
relatedMessages.push({
text: `The property half of the binding is to the '${inputConsumer.name.text}' component.`,
start: inputConsumer.name.getStart(),
end: inputConsumer.name.getEnd(),
sourceFile: inputConsumer.name.getSourceFile(),
});
if (outputConsumer instanceof TmplAstElement) {
let message = `The event half of the binding is to a native event called '${
input.name}' on the <${outputConsumer.name}> DOM element.`;
if (!mapping.node.getSourceFile().isDeclarationFile) {
message += `\n \n Are you missing an output declaration called '${output.name}'?`;
}
relatedMessages.push({
text: message,
start: outputConsumer.sourceSpan.start.offset + 1,
end: outputConsumer.sourceSpan.start.offset + outputConsumer.name.length + 1,
sourceFile: mapping.node.getSourceFile(),
});
} else {
relatedMessages.push({
text: `The event half of the binding is to the '${outputConsumer.name.text}' component.`,
start: outputConsumer.name.getStart(),
end: outputConsumer.name.getEnd(),
sourceFile: outputConsumer.name.getSourceFile(),
});
}
this._diagnostics.push(makeTemplateDiagnostic(
templateId, mapping, input.keySpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.SPLIT_TWO_WAY_BINDING), errorMsg, relatedMessages));
}
}
function makeInlineDiagnostic(

38
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View File

@ -984,6 +984,11 @@ export class TcbDirectiveOutputsOp extends TcbOp {
if (output.type !== ParsedEventType.Regular || !outputs.hasBindingPropertyName(output.name)) {
continue;
}
if (this.tcb.env.config.checkTypeOfOutputEvents && output.name.endsWith('Change')) {
const inputName = output.name.slice(0, -6);
isSplitTwoWayBinding(inputName, output, this.node.inputs, this.tcb);
}
// TODO(alxhub): consider supporting multiple fields with the same property name for outputs.
const field = outputs.getByBindingPropertyName(output.name)![0].classPropertyName;
@ -1049,6 +1054,14 @@ class TcbUnclaimedOutputsOp extends TcbOp {
continue;
}
if (this.tcb.env.config.checkTypeOfOutputEvents && output.name.endsWith('Change')) {
const inputName = output.name.slice(0, -6);
if (isSplitTwoWayBinding(inputName, output, this.element.inputs, this.tcb)) {
// Skip this event handler as the error was already handled.
continue;
}
}
if (output.type === ParsedEventType.Animation) {
// Animation output bindings always have an `$event` parameter of type `AnimationEvent`.
const eventType = this.tcb.env.config.checkTypeOfAnimationEvents ?
@ -1979,6 +1992,31 @@ function tcbEventHandlerExpression(ast: AST, tcb: Context, scope: Scope): ts.Exp
return translator.translate(ast);
}
function isSplitTwoWayBinding(
inputName: string, output: TmplAstBoundEvent, inputs: TmplAstBoundAttribute[], tcb: Context) {
const input = inputs.find(input => input.name === inputName);
if (input === undefined || input.sourceSpan !== output.sourceSpan) {
return false;
}
// Input consumer should be a directive because it's claimed
const inputConsumer = tcb.boundTarget.getConsumerOfBinding(input) as TypeCheckableDirectiveMeta;
const outputConsumer = tcb.boundTarget.getConsumerOfBinding(output);
if (outputConsumer === null || inputConsumer.ref === undefined ||
outputConsumer instanceof TmplAstTemplate) {
return false;
}
if (outputConsumer instanceof TmplAstElement) {
tcb.oobRecorder.splitTwoWayBinding(
tcb.id, input, output, inputConsumer.ref.node, outputConsumer);
return true;
} else if (outputConsumer.ref !== inputConsumer.ref) {
tcb.oobRecorder.splitTwoWayBinding(
tcb.id, input, output, inputConsumer.ref.node, outputConsumer.ref.node);
return true;
}
return false;
}
class TcbEventHandlerTranslator extends TcbExpressionTranslator {
protected override resolve(ast: AST): ts.Expression|null {
// Recognize a property read on the implicit receiver corresponding with the event parameter

1
packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
View File

@ -694,4 +694,5 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
requiresInlineTcb(): void {}
requiresInlineTypeConstructors(): void {}
suboptimalTypeInference(): void {}
splitTwoWayBinding(): void {}
}

79
packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View File

@ -338,6 +338,85 @@ export declare class AnimationEvent {
expect(diags.length).toBe(0);
});
it('should check split two way binding', () => {
env.tsconfig({strictTemplates: true});
env.write('test.ts', `
import {Component, Input, NgModule} from '@angular/core';
@Component({
selector: 'test',
template: '<child-cmp [(value)]="counterValue"></child-cmp>',
})
export class TestCmp {
counterValue = 0;
}
@Component({
selector: 'child-cmp',
template: '',
})
export class ChildCmp {
@Input() value = 0;
}
@NgModule({
declarations: [TestCmp, ChildCmp],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SPLIT_TWO_WAY_BINDING));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('value');
expect(diags[0].relatedInformation!.length).toBe(2);
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toBe('ChildCmp');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![1])).toBe('child-cmp');
});
it('when input and output go to different directives', () => {
env.tsconfig({strictTemplates: true});
env.write('test.ts', `
import {Component, Input, NgModule, Output, Directive} from '@angular/core';
@Component({
selector: 'test',
template: '<child-cmp [(value)]="counterValue"></child-cmp>',
})
export class TestCmp {
counterValue = 0;
}
@Directive({
selector: 'child-cmp'
})
export class ChildCmpDir {
@Output() valueChange: any;
}
@Component({
selector: 'child-cmp',
template: '',
})
export class ChildCmp {
@Input() value = 0;
}
@NgModule({
declarations: [TestCmp, ChildCmp, ChildCmpDir],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.SPLIT_TWO_WAY_BINDING));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('value');
expect(diags[0].relatedInformation!.length).toBe(2);
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![0])).toBe('ChildCmp');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation![1])).toBe('ChildCmpDir');
});
describe('strictInputTypes', () => {
beforeEach(() => {
env.write('test.ts', `