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

fix(language-service): warn, not error, on missing context members (#35036)

Browse Source 
The language service reports an error when a directive's template
context is missing a member that is being used in a template (e.g. if
`$implicit` is being used with a template context typed as `any`).
While this diagnostic message is valuable, typing template contexts
loosely as `any` or `object` is very widespread in community packages,
and often still compiles correctly, so reporting the diagnostic as an
error may be misleading to users.

This commit changes the diagnostic to be a warning, and adds additional
information about how the user can eliminate the warning entirely -- by
refining the template context type.

Closes https://github.com/angular/vscode-ng-language-service/issues/572

PR Close #35036
This commit is contained in:
Ayaz Hafiz 2020-01-29 10:17:36 -08:00 committed by Andrew Kushnir
parent b64ead5cb8
commit b608fa55f0
2 changed files with 17 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
packages/language-service/src/expression_diagnostics.ts
View File

@ -292,16 +292,12 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
if (directive && ast.value) { if (directive && ast.value) {
const context = this.info.query.getTemplateContext(directive.type.reference) !; const context = this.info.query.getTemplateContext(directive.type.reference) !;
if (context && !context.has(ast.value)) { if (context && !context.has(ast.value)) {
if (ast.value === '$implicit') { const missingMember =
this.reportError( ast.value === '$implicit' ? 'an implicit value' : `a member called '${ast.value}'`;
`The template context of '${directive.type.reference.name}' does not define an implicit value.\n` + this.reportDiagnostic(
`If the context type is a base type, consider refining it to a more specific type.`, `The template context of '${directive.type.reference.name}' does not define ${missingMember}.\n` +
spanOf(ast.sourceSpan)); `If the context type is a base type or 'any', consider refining it to a more specific type.`,
} else { spanOf(ast.sourceSpan), ts.DiagnosticCategory.Suggestion);
this.reportError(
`The template context of '${directive.type.reference.name}' does not define a member called '${ast.value}'`,
spanOf(ast.sourceSpan));
}
} }
} }
} }
@ -353,11 +349,9 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
private pop() { this.path.pop(); } private pop() { this.path.pop(); }
private reportError(message: string, span: Span|undefined) { private reportDiagnostic(
if (span) { message: string, span: Span, kind: ts.DiagnosticCategory = ts.DiagnosticCategory.Error) {
this.diagnostics.push( this.diagnostics.push({span: offsetSpan(span, this.info.offset), kind, message});
{span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Error, message});
}
} }
} }

12
packages/language-service/test/diagnostics_spec.ts
View File

@ -257,11 +257,12 @@ describe('diagnostics', () => {
`<button type="button" ~{start-emb}*counter="let hero of heroes"~{end-emb}></button>`); `<button type="button" ~{start-emb}*counter="let hero of heroes"~{end-emb}></button>`);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
const {messageText, start, length} = diags[0]; const {messageText, start, length, category} = diags[0];
expect(category).toBe(ts.DiagnosticCategory.Suggestion);
expect(messageText) expect(messageText)
.toBe( .toBe(
`The template context of 'CounterDirective' does not define an implicit value.\n` + `The template context of 'CounterDirective' does not define an implicit value.\n` +
`If the context type is a base type, consider refining it to a more specific type.`, ); `If the context type is a base type or 'any', consider refining it to a more specific type.`, );
const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb'); const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
expect(start).toBe(span.start); expect(start).toBe(span.start);
@ -274,9 +275,12 @@ describe('diagnostics', () => {
`<div ~{start-emb}*ngFor="let hero of heroes; let e = even_1"~{end-emb}></div>`); `<div ~{start-emb}*ngFor="let hero of heroes; let e = even_1"~{end-emb}></div>`);
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE); const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
expect(diags.length).toBe(1); expect(diags.length).toBe(1);
const {messageText, start, length} = diags[0]; const {messageText, start, length, category} = diags[0];
expect(category).toBe(ts.DiagnosticCategory.Suggestion);
expect(messageText) expect(messageText)
.toBe(`The template context of 'NgForOf' does not define a member called 'even_1'`); .toBe(
`The template context of 'NgForOf' does not define a member called 'even_1'.\n` +
`If the context type is a base type or 'any', consider refining it to a more specific type.`);
const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb'); const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
expect(start).toBe(span.start); expect(start).toBe(span.start);