refactor(compiler-cli): Add additional key/value spans to TCB (#39665)

In order to more accurately map from a node in the TCB to a template position,
we need to provide more span information in the TCB. These changes are necessary
for the Language Service to map from a TCB node back to a specific
locations in the template for actions like "find references" and
"refactor/rename". After the TS "find references" returns results,
including those in the TCB, we need to map specifically to the matching
key/value spans in the template rather than the entire source span.

This also has the benefit of producing diagnostics which align more
closely with what TypeScript produces.
The following example shows TS code and the diagnostic produced by an invalid assignment to a property:

```
let a: {age: number} = {} as any;
a.age = 'laksjdf';
^^^^^ <-- Type 'string' is not assignable to type 'number'.
```
A corollary to this in a template file would be [age]="'someString'". The diagnostic we currently produce for this is:

```
Type 'number' is not assignable to type 'string'.

1 <app-hello [greeting]="1"></app-hello>
             ~~~~~~~~~~~~~~
```
Notice that the underlined text includes the entire span.
If we included the keySpan for the assignment to the property,
this diagnostic underline would be more similar to the one produced by TypeScript;
that is, it would only underline “greeting”.

[design/discussion doc]
(https://docs.google.com/document/d/1FtaHdVL805wKe4E6FxVTnVHl38lICoHIjS2nThtRJ6I/edit?usp=sharing)

PR Close #39665
This commit is contained in:
Andrew Scott 2020-11-12 15:25:02 -08:00 committed by atscott
parent 7bcfc0db09
commit 7e724add7e
4 changed files with 24 additions and 11 deletions

View File

@ -15,7 +15,7 @@ import {ClassDeclaration} from '../../reflection';
import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata} from '../api';
import {addExpressionIdentifier, ExpressionIdentifier, markIgnoreDiagnostics} from './comments';
import {addParseSpanInfo, addTemplateId, wrapForDiagnostics} from './diagnostics';
import {addParseSpanInfo, addTemplateId, wrapForDiagnostics, wrapForTypeChecker} from './diagnostics';
import {DomSchemaChecker} from './dom';
import {Environment} from './environment';
import {astToTypescript, NULL_AS_ANY} from './expression';
@ -176,10 +176,18 @@ class TcbVariableOp extends TcbOp {
const initializer = ts.createPropertyAccess(
/* expression */ ctx,
/* name */ this.variable.value || '$implicit');
addParseSpanInfo(initializer, this.variable.sourceSpan);
addParseSpanInfo(id, this.variable.keySpan);
// Declare the variable, and return its identifier.
this.scope.addStatement(tsCreateVariable(id, initializer));
let variable: ts.VariableStatement;
if (this.variable.valueSpan !== undefined) {
addParseSpanInfo(initializer, this.variable.valueSpan);
variable = tsCreateVariable(id, wrapForTypeChecker(initializer));
} else {
variable = tsCreateVariable(id, initializer);
}
addParseSpanInfo(variable.declarationList.declarations[0], this.variable.sourceSpan);
this.scope.addStatement(variable);
return id;
}
}
@ -443,6 +451,7 @@ class TcbReferenceOp extends TcbOp {
initializer = ts.createParen(initializer);
}
addParseSpanInfo(initializer, this.node.sourceSpan);
addParseSpanInfo(id, this.node.keySpan);
this.scope.addStatement(tsCreateVariable(id, initializer));
return id;
@ -617,6 +626,9 @@ class TcbDirectiveInputsOp extends TcbOp {
ts.createPropertyAccess(dirId, ts.createIdentifier(fieldName));
}
if (input.attribute.keySpan !== undefined) {
addParseSpanInfo(target, input.attribute.keySpan);
}
// Finally the assignment is extended by assigning it into the target expression.
assignment = ts.createBinary(target, ts.SyntaxKind.EqualsToken, assignment);
}
@ -839,6 +851,7 @@ export class TcbDirectiveOutputsOp extends TcbOp {
dirId = this.scope.resolve(this.node, this.dir);
}
const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
addParseSpanInfo(outputField, output.keySpan);
const outputHelper =
ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]);
const subscribeFn = ts.createPropertyAccess(outputHelper, 'subscribe');

View File

@ -36,7 +36,7 @@ runInEachFileSystem(() => {
}]);
expect(messages).toEqual(
[`TestComponent.html(1, 10): Type 'string' is not assignable to type 'number'.`]);
[`TestComponent.html(1, 11): Type 'string' is not assignable to type 'number'.`]);
});
it('infers type of template variables', () => {

View File

@ -184,7 +184,7 @@ describe('type check blocks diagnostics', () => {
}];
const TEMPLATE = `<my-cmp [inputA]="''"></my-cmp>`;
expect(tcbWithSpans(TEMPLATE, DIRECTIVES))
.toContain('_t1.inputA = ("" /*18,20*/) /*8,21*/;');
.toContain('_t1.inputA /*9,15*/ = ("" /*18,20*/) /*8,21*/;');
});
});
});

View File

@ -1267,11 +1267,11 @@ export declare class AnimationEvent {
const diags = env.driveDiagnostics();
expect(diags.length).toBe(3);
expect(diags[0].messageText).toBe(`Type 'boolean' is not assignable to type 'number'.`);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"');
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('fromAbstract');
expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"');
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('fromBase');
expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"');
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('fromChild');
});
it('should properly type-check inherited directives from external libraries', () => {
@ -1324,11 +1324,11 @@ export declare class AnimationEvent {
const diags = env.driveDiagnostics();
expect(diags.length).toBe(3);
expect(diags[0].messageText).toBe(`Type 'boolean' is not assignable to type 'number'.`);
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('[fromAbstract]="true"');
expect(getSourceCodeForDiagnostic(diags[0])).toEqual('fromAbstract');
expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('[fromBase]="3"');
expect(getSourceCodeForDiagnostic(diags[1])).toEqual('fromBase');
expect(diags[2].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('[fromChild]="4"');
expect(getSourceCodeForDiagnostic(diags[2])).toEqual('fromChild');
});
it('should detect an illegal write to a template variable', () => {