fix(language-service): infer context type of structural directives (#35537) (#35561)

PR Close #35561
This commit is contained in:
Pusztai Tibor 2020-02-19 20:38:21 +01:00 committed by Miško Hevery
parent bd6a39c364
commit 54fd33fb80
4 changed files with 98 additions and 41 deletions

View File

@ -91,7 +91,10 @@ function getVarDeclarations(
continue; continue;
} }
for (const variable of current.variables) { for (const variable of current.variables) {
let symbol = info.members.get(variable.value) || info.query.getBuiltinType(BuiltinType.Any); let symbol = info.members.get(variable.value);
if (!symbol) {
symbol = getVariableTypeFromDirectiveContext(variable.value, info.query, current);
}
const kind = info.query.getTypeKind(symbol); const kind = info.query.getTypeKind(symbol);
if (kind === BuiltinType.Any || kind === BuiltinType.Unbound) { if (kind === BuiltinType.Any || kind === BuiltinType.Unbound) {
// For special cases such as ngFor and ngIf, the any type is not very useful. // For special cases such as ngFor and ngIf, the any type is not very useful.
@ -115,22 +118,24 @@ function getVarDeclarations(
} }
/** /**
* Gets the type of an ngFor exported value, as enumerated in * Resolve the type for the variable in `templateElement` by finding the structural
* https://angular.io/api/common/NgForOfContext * directive which has the context member. Returns any when not found.
* @param value exported value name * @param value variable value name
* @param query type symbol query * @param query type symbol query
* @param templateElement
*/ */
function getNgForExportedValueType(value: string, query: SymbolQuery): Symbol|undefined { function getVariableTypeFromDirectiveContext(
switch (value) { value: string, query: SymbolQuery, templateElement: EmbeddedTemplateAst): Symbol {
case 'index': for (const {directive} of templateElement.directives) {
case 'count': const context = query.getTemplateContext(directive.type.reference);
return query.getBuiltinType(BuiltinType.Number); if (context) {
case 'first': const member = context.get(value);
case 'last': if (member && member.type) {
case 'even': return member.type;
case 'odd':
return query.getBuiltinType(BuiltinType.Boolean);
} }
}
}
return query.getBuiltinType(BuiltinType.Any);
} }
/** /**
@ -145,6 +150,7 @@ function getNgForExportedValueType(value: string, query: SymbolQuery): Symbol|un
function refinedVariableType( function refinedVariableType(
value: string, mergedTable: SymbolTable, query: SymbolQuery, value: string, mergedTable: SymbolTable, query: SymbolQuery,
templateElement: EmbeddedTemplateAst): Symbol { templateElement: EmbeddedTemplateAst): Symbol {
if (value === '$implicit') {
// Special case the ngFor directive // Special case the ngFor directive
const ngForDirective = templateElement.directives.find(d => { const ngForDirective = templateElement.directives.find(d => {
const name = identifierName(d.directive.type); const name = identifierName(d.directive.type);
@ -153,22 +159,20 @@ function refinedVariableType(
if (ngForDirective) { if (ngForDirective) {
const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf'); const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf');
if (ngForOfBinding) { if (ngForOfBinding) {
// Check if the variable value is a type exported by the ngFor statement. // Check if there is a known type for the ngFor binding.
let result = getNgForExportedValueType(value, query);
// Otherwise, check if there is a known type for the ngFor binding.
const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value); const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value);
if (!result && bindingType) { if (bindingType) {
result = query.getElementType(bindingType); const result = query.getElementType(bindingType);
}
if (result) { if (result) {
return result; return result;
} }
} }
} }
}
}
// Special case the ngIf directive ( *ngIf="data$ | async as variable" ) // Special case the ngIf directive ( *ngIf="data$ | async as variable" )
if (value === 'ngIf') {
const ngIfDirective = const ngIfDirective =
templateElement.directives.find(d => identifierName(d.directive.type) === 'NgIf'); templateElement.directives.find(d => identifierName(d.directive.type) === 'NgIf');
if (ngIfDirective) { if (ngIfDirective) {
@ -180,6 +184,7 @@ function refinedVariableType(
} }
} }
} }
}
// We can't do better, return any // We can't do better, return any
return query.getBuiltinType(BuiltinType.Any); return query.getBuiltinType(BuiltinType.Any);

View File

@ -286,6 +286,42 @@ describe('diagnostics', () => {
expect(start).toBe(span.start); expect(start).toBe(span.start);
expect(length).toBe(span.length); expect(length).toBe(span.length);
}); });
it('report an unknown field in $implicit context', () => {
mockHost.override(TEST_TEMPLATE, `
<div *withContext="let myVar">
{{ ~{start-emb}myVar.missingField ~{end-emb}}}
</div>
`);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
const {messageText, start, length, category} = diags[0];
expect(category).toBe(ts.DiagnosticCategory.Error);
expect(messageText)
.toBe(
`Identifier 'missingField' is not defined. '{ implicitPerson: Person; }' does not contain such a member`, );
const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
expect(start).toBe(span.start);
expect(length).toBe(span.length);
});
it('report an unknown field in non implicit context', () => {
mockHost.override(TEST_TEMPLATE, `
<div *withContext="let myVar = nonImplicitPerson">
{{ ~{start-emb}myVar.missingField ~{end-emb}}}
</div>
`);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1);
const {messageText, start, length, category} = diags[0];
expect(category).toBe(ts.DiagnosticCategory.Error);
expect(messageText)
.toBe(
`Identifier 'missingField' is not defined. 'Person' does not contain such a member`, );
const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
expect(start).toBe(span.start);
expect(length).toBe(span.length);
});
}); });
// #17611 // #17611

View File

@ -43,6 +43,7 @@ import * as ParsingCases from './parsing-cases';
ParsingCases.StringModel, ParsingCases.StringModel,
ParsingCases.TemplateReference, ParsingCases.TemplateReference,
ParsingCases.TestComponent, ParsingCases.TestComponent,
ParsingCases.WithContextDirective,
] ]
}) })
export class AppModule { export class AppModule {

View File

@ -127,6 +127,21 @@ export class CounterDirective implements OnChanges {
} }
} }
interface WithContextDirectiveContext {
$implicit: {implicitPerson: Person;};
nonImplicitPerson: Person;
}
@Directive({selector: '[withContext]'})
export class WithContextDirective {
constructor(_template: TemplateRef<WithContextDirectiveContext>) {}
static ngTemplateContextGuard(dir: WithContextDirective, ctx: unknown):
ctx is WithContextDirectiveContext {
return true;
}
}
/** /**
* This Component provides the `test-comp` selector. * This Component provides the `test-comp` selector.
*/ */