Skip to content

Commit

Permalink
feat(language-service): specific suggestions for template context diags
Browse files Browse the repository at this point in the history
This commit elaborates diagnostics produced for invalid template
contexts by including the name of the embedded template type using the
template context, and in the common case that the implicity property is
being referenced (e.g. in a `for .. of ..` expression), suggesting to
refine the type of the context. This suggestion is provided because
users will sometimes use a base class as the type of the context in the
embedded view, and a more specific context later on (e.g. in an
`ngOnChanges` method).

Closes angular/vscode-ng-language-service#251
  • Loading branch information
ayazhafiz committed Jan 12, 2020
1 parent 6d534f1 commit 9e7d3b8
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 12 deletions.
6 changes: 4 additions & 2 deletions packages/language-service/src/expression_diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,12 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
if (context && !context.has(ast.value)) {
if (ast.value === '$implicit') {
this.reportError(
'The template context does not have an implicit value', spanOf(ast.sourceSpan));
`The template context of '${directive.type.reference.name}' does not define an implicit value.\n` +
`If the context type is a base type, consider refining it to a more specific type.`,
spanOf(ast.sourceSpan));
} else {
this.reportError(
`The template context does not define a member called '${ast.value}'`,
`The template context of '${directive.type.reference.name}' does not define a member called '${ast.value}'`,
spanOf(ast.sourceSpan));
}
}
Expand Down
89 changes: 79 additions & 10 deletions packages/language-service/test/diagnostics_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {MockTypescriptHost} from './test_utils';

const EXPRESSION_CASES = '/app/expression-cases.ts';
const NG_FOR_CASES = '/app/ng-for-cases.ts';
const NG_IF_CASES = '/app/ng-if-cases.ts';
const TEST_TEMPLATE = '/app/test.ng';
const APP_COMPONENT = '/app/app.component.ts';

Expand Down Expand Up @@ -242,11 +241,6 @@ describe('diagnostics', () => {
`and element references do not contain such a member`);
});

it('should report an unknown context reference', () => {
const diags = ngLS.getDiagnostics(NG_FOR_CASES).map(d => d.messageText);
expect(diags).toContain(`The template context does not define a member called 'even_1'`);
});

it('should report an unknown value in a key expression', () => {
const diags = ngLS.getDiagnostics(NG_FOR_CASES).map(d => d.messageText);
expect(diags).toContain(
Expand All @@ -256,10 +250,85 @@ describe('diagnostics', () => {
});
});

describe('in ng-if-cases.ts', () => {
it('should report an implicit context reference', () => {
const diags = ngLS.getDiagnostics(NG_IF_CASES).map(d => d.messageText);
expect(diags).toContain(`The template context does not define a member called 'unknown'`);
describe('embedded templates', () => {
it('should suggest refining a template context missing a property', () => {
mockHost.override(APP_COMPONENT, `
import {Directive, Component, Input, NgModule, OnChanges, ViewContainerRef, TemplateRef, SimpleChanges} from '@angular/core';
@Directive({
selector: '[counterOf]'
})
export class CounterDirective implements OnChanges {
// Object does not have an "$implicit" property.
constructor(private container: ViewContainerRef, private template: TemplateRef<Object>) {}
@Input('counterOf') counter: number = 0;
ngOnChanges(changes: SimpleChanges) {}
}
@Component({
template: '<button type="button" ~{start-emb}*counter="let page of pageCount"~{end-emb}></button>',
})
export class TestComponent {
pageCount = [1, 2, 3, 4];
}
@NgModule({
declarations: [
TestComponent,
CounterDirective,
]
})
export class TestModule {}
`);
const tsDiags = tsLS.getSemanticDiagnostics(APP_COMPONENT);
expect(tsDiags).toEqual([]);

const diags = ngLS.getDiagnostics(APP_COMPONENT);
expect(diags.length).toBe(1);
const {file, messageText, start, length} = diags[0];
expect(file !.fileName).toBe(APP_COMPONENT);
expect(messageText)
.toBe(
`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.`, );

const span = mockHost.getLocationMarkerFor(APP_COMPONENT, 'emb');
expect(start).toBe(span.start);
expect(length).toBe(span.length);
});

it('should report an unknown context reference', () => {
mockHost.override(APP_COMPONENT, `
import {CommonModule} from '@angular/common';
import {Component, NgModule} from '@angular/core';
@Component({
template: '<div ~{start-emb}*ngFor="let person of people; let e = even_1"~{end-emb}></div>',
})
export class TestComponent {
people: string[] = [];
}
@NgModule({
imports: [CommonModule],
declarations: [TestComponent]
})
export class TestModule {}
`);
const tsDiags = tsLS.getSemanticDiagnostics(APP_COMPONENT);
expect(tsDiags).toEqual([]);

const diags = ngLS.getDiagnostics(APP_COMPONENT);
expect(diags.length).toBe(1);
const {file, messageText, start, length} = diags[0];
expect(file !.fileName).toBe(APP_COMPONENT);
expect(messageText)
.toBe(`The template context of 'NgForOf' does not define a member called 'even_1'`);

const span = mockHost.getLocationMarkerFor(APP_COMPONENT, 'emb');
expect(start).toBe(span.start);
expect(length).toBe(span.length);
});
});

Expand Down

0 comments on commit 9e7d3b8

Please sign in to comment.