fix(language-service): Diagnostic span should point to class name (#34932)
Right now, if an Angular diagnostic is generated for a TypeScript node, the span points to the decorator Identifier, i.e. the Identifier node like `@NgModule`, `@Component`, etc. This is weird. It should point to the class name instead. Note, we do not have a more fine-grained breakdown of the span when diagnostics are emitted, this work remains to be done. PR Close #34932
This commit is contained in:

committed by
Andrew Kushnir

parent
37727ce898
commit
c9db7bdb8a
@ -362,9 +362,7 @@ describe('diagnostics', () => {
|
||||
.toBe(
|
||||
`Component 'MyComponent' is not included in a module and will not be available inside a template. Consider adding it to a NgModule declaration.`);
|
||||
const content = mockHost.readFile(fileName) !;
|
||||
const keyword = '@Component';
|
||||
expect(start).toBe(content.lastIndexOf(keyword) + 1); // exclude leading '@'
|
||||
expect(length).toBe(keyword.length - 1); // exclude leading '@'
|
||||
expect(content.substring(start !, start ! + length !)).toBe('MyComponent');
|
||||
});
|
||||
|
||||
|
||||
@ -604,9 +602,7 @@ describe('diagnostics', () => {
|
||||
.toBe(
|
||||
'Invalid providers for "AppComponent in /app/app.component.ts" - only instances of Provider and Type are allowed, got: [?null?]');
|
||||
// TODO: Looks like this is the wrong span. Should point to 'null' instead.
|
||||
const keyword = '@Component';
|
||||
expect(start).toBe(content.lastIndexOf(keyword) + 1); // exclude leading '@'
|
||||
expect(length).toBe(keyword.length - 1); // exclude leading '@
|
||||
expect(content.substring(start !, start ! + length !)).toBe('AppComponent');
|
||||
});
|
||||
|
||||
// Issue #15768
|
||||
@ -775,8 +771,7 @@ describe('diagnostics', () => {
|
||||
const {file, messageText, start, length} = diags[0];
|
||||
expect(file !.fileName).toBe(APP_COMPONENT);
|
||||
expect(messageText).toBe(`Component 'AppComponent' must have a template or templateUrl`);
|
||||
expect(start).toBe(content.indexOf(`@Component`) + 1);
|
||||
expect(length).toBe('Component'.length);
|
||||
expect(content.substring(start !, start ! + length !)).toBe('AppComponent');
|
||||
});
|
||||
|
||||
it('should report diagnostic for both template and templateUrl', () => {
|
||||
@ -795,8 +790,7 @@ describe('diagnostics', () => {
|
||||
expect(file !.fileName).toBe(APP_COMPONENT);
|
||||
expect(messageText)
|
||||
.toBe(`Component 'AppComponent' must not have both template and templateUrl`);
|
||||
expect(start).toBe(content.indexOf(`@Component`) + 1);
|
||||
expect(length).toBe('Component'.length);
|
||||
expect(content.substring(start !, start ! + length !)).toBe('AppComponent');
|
||||
});
|
||||
|
||||
it('should report errors for invalid styleUrls', () => {
|
||||
|
Reference in New Issue
Block a user