Skip to content

Commit 63624a2

Browse files
kyliauatscott
authored andcommittedOct 8, 2020
feat(language-service): [Ivy] getSemanticDiagnostics for external templates (#39065)
This PR enables `getSemanticDiagnostics()` to be called on external templates. Several changes are needed to land this feature: 1. The adapter needs to implement two additional methods: a. `readResource()` Load the template from snapshot instead of reading from disk b. `getModifiedResourceFiles()` Inform the compiler that external templates have changed so that the loader could invalidate its internal cache. 2. Create `ScriptInfo` for external templates in MockHost. Prior to this, MockHost only track changes in TypeScript files. Now it needs to create `ScriptInfo` for external templates as well. For (1), in order to make sure we don't reload the template if it hasn't changed, we need to keep track of its version. Since the complexity has increased, the adapter is refactored into its own class. PR Close #39065
1 parent 4604fe9 commit 63624a2

File tree

5 files changed

+241
-46
lines changed

5 files changed

+241
-46
lines changed
 

‎packages/language-service/ivy/language_service.ts

+31-42
Original file line numberDiff line numberDiff line change
@@ -8,60 +8,74 @@
88

99
import {CompilerOptions, createNgCompilerOptions} from '@angular/compiler-cli';
1010
import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core';
11-
import {NgCompilerAdapter} from '@angular/compiler-cli/src/ngtsc/core/api';
12-
import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
11+
import {absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
1312
import {PatchedProgramIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental';
14-
import {isShim} from '@angular/compiler-cli/src/ngtsc/shims';
1513
import {TypeCheckShimGenerator} from '@angular/compiler-cli/src/ngtsc/typecheck';
1614
import {OptimizeFor, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
1715
import * as ts from 'typescript/lib/tsserverlibrary';
1816

1917
import {DefinitionBuilder} from './definitions';
18+
import {isExternalTemplate, isTypeScriptFile, LanguageServiceAdapter} from './language_service_adapter';
2019
import {QuickInfoBuilder} from './quick_info';
2120

2221
export class LanguageService {
2322
private options: CompilerOptions;
2423
private lastKnownProgram: ts.Program|null = null;
2524
private readonly strategy: TypeCheckingProgramStrategy;
26-
private readonly adapter: NgCompilerAdapter;
25+
private readonly adapter: LanguageServiceAdapter;
2726

2827
constructor(project: ts.server.Project, private readonly tsLS: ts.LanguageService) {
2928
this.options = parseNgCompilerOptions(project);
3029
this.strategy = createTypeCheckingProgramStrategy(project);
31-
this.adapter = createNgCompilerAdapter(project);
30+
this.adapter = new LanguageServiceAdapter(project);
3231
this.watchConfigFile(project);
3332
}
3433

3534
getSemanticDiagnostics(fileName: string): ts.Diagnostic[] {
3635
const program = this.strategy.getProgram();
37-
const compiler = this.createCompiler(program);
38-
if (fileName.endsWith('.ts')) {
36+
const compiler = this.createCompiler(program, fileName);
37+
const ttc = compiler.getTemplateTypeChecker();
38+
const diagnostics: ts.Diagnostic[] = [];
39+
if (isTypeScriptFile(fileName)) {
3940
const sourceFile = program.getSourceFile(fileName);
40-
if (!sourceFile) {
41-
return [];
41+
if (sourceFile) {
42+
diagnostics.push(...ttc.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile));
43+
}
44+
} else {
45+
const components = compiler.getComponentsWithTemplateFile(fileName);
46+
for (const component of components) {
47+
if (ts.isClassDeclaration(component)) {
48+
diagnostics.push(...ttc.getDiagnosticsForComponent(component));
49+
}
4250
}
43-
const ttc = compiler.getTemplateTypeChecker();
44-
const diagnostics = ttc.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile);
45-
this.lastKnownProgram = compiler.getNextProgram();
46-
return diagnostics;
4751
}
48-
throw new Error('Ivy LS currently does not support external template');
52+
this.lastKnownProgram = compiler.getNextProgram();
53+
return diagnostics;
4954
}
5055

5156
getDefinitionAndBoundSpan(fileName: string, position: number): ts.DefinitionInfoAndBoundSpan
5257
|undefined {
5358
const program = this.strategy.getProgram();
54-
const compiler = this.createCompiler(program);
59+
const compiler = this.createCompiler(program, fileName);
5560
return new DefinitionBuilder(this.tsLS, compiler).getDefinitionAndBoundSpan(fileName, position);
5661
}
5762

5863
getQuickInfoAtPosition(fileName: string, position: number): ts.QuickInfo|undefined {
5964
const program = this.strategy.getProgram();
60-
const compiler = this.createCompiler(program);
65+
const compiler = this.createCompiler(program, fileName);
6166
return new QuickInfoBuilder(this.tsLS, compiler).get(fileName, position);
6267
}
6368

64-
private createCompiler(program: ts.Program): NgCompiler {
69+
/**
70+
* Create a new instance of Ivy compiler.
71+
* If the specified `fileName` refers to an external template, check if it has
72+
* changed since the last time it was read. If it has changed, signal the
73+
* compiler to reload the file via the adapter.
74+
*/
75+
private createCompiler(program: ts.Program, fileName: string): NgCompiler {
76+
if (isExternalTemplate(fileName)) {
77+
this.adapter.registerTemplateUpdate(fileName);
78+
}
6579
return new NgCompiler(
6680
this.adapter,
6781
this.options,
@@ -107,31 +121,6 @@ export function parseNgCompilerOptions(project: ts.server.Project): CompilerOpti
107121
return createNgCompilerOptions(basePath, config, project.getCompilationSettings());
108122
}
109123

110-
function createNgCompilerAdapter(project: ts.server.Project): NgCompilerAdapter {
111-
return {
112-
entryPoint: null, // entry point is only needed if code is emitted
113-
constructionDiagnostics: [],
114-
ignoreForEmit: new Set(),
115-
factoryTracker: null, // no .ngfactory shims
116-
unifiedModulesHost: null, // only used in Bazel
117-
rootDirs: project.getCompilationSettings().rootDirs?.map(absoluteFrom) || [],
118-
isShim,
119-
fileExists(fileName: string): boolean {
120-
return project.fileExists(fileName);
121-
},
122-
readFile(fileName: string): string |
123-
undefined {
124-
return project.readFile(fileName);
125-
},
126-
getCurrentDirectory(): string {
127-
return project.getCurrentDirectory();
128-
},
129-
getCanonicalFileName(fileName: string): string {
130-
return project.projectService.toCanonicalFileName(fileName);
131-
},
132-
};
133-
}
134-
135124
function createTypeCheckingProgramStrategy(project: ts.server.Project):
136125
TypeCheckingProgramStrategy {
137126
return {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {NgCompilerAdapter} from '@angular/compiler-cli/src/ngtsc/core/api';
10+
import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
11+
import {isShim} from '@angular/compiler-cli/src/ngtsc/shims';
12+
import * as ts from 'typescript/lib/tsserverlibrary';
13+
14+
export class LanguageServiceAdapter implements NgCompilerAdapter {
15+
readonly entryPoint = null;
16+
readonly constructionDiagnostics: ts.Diagnostic[] = [];
17+
readonly ignoreForEmit: Set<ts.SourceFile> = new Set();
18+
readonly factoryTracker = null; // no .ngfactory shims
19+
readonly unifiedModulesHost = null; // only used in Bazel
20+
readonly rootDirs: AbsoluteFsPath[];
21+
private readonly templateVersion = new Map<string, string>();
22+
private readonly modifiedTemplates = new Set<string>();
23+
24+
constructor(private readonly project: ts.server.Project) {
25+
this.rootDirs = project.getCompilationSettings().rootDirs?.map(absoluteFrom) || [];
26+
}
27+
28+
isShim(sf: ts.SourceFile): boolean {
29+
return isShim(sf);
30+
}
31+
32+
fileExists(fileName: string): boolean {
33+
return this.project.fileExists(fileName);
34+
}
35+
36+
readFile(fileName: string): string|undefined {
37+
return this.project.readFile(fileName);
38+
}
39+
40+
getCurrentDirectory(): string {
41+
return this.project.getCurrentDirectory();
42+
}
43+
44+
getCanonicalFileName(fileName: string): string {
45+
return this.project.projectService.toCanonicalFileName(fileName);
46+
}
47+
48+
/**
49+
* readResource() is an Angular-specific method for reading files that are not
50+
* managed by the TS compiler host, namely templates and stylesheets.
51+
* It is a method on ExtendedTsCompilerHost, see
52+
* packages/compiler-cli/src/ngtsc/core/api/src/interfaces.ts
53+
*/
54+
readResource(fileName: string): string {
55+
if (isTypeScriptFile(fileName)) {
56+
throw new Error(`readResource() should not be called on TS file: ${fileName}`);
57+
}
58+
// Calling getScriptSnapshot() will actually create a ScriptInfo if it does
59+
// not exist! The same applies for getScriptVersion().
60+
// getScriptInfo() will not create one if it does not exist.
61+
// In this case, we *want* a script info to be created so that we could
62+
// keep track of its version.
63+
const snapshot = this.project.getScriptSnapshot(fileName);
64+
if (!snapshot) {
65+
// This would fail if the file does not exist, or readFile() fails for
66+
// whatever reasons.
67+
throw new Error(`Failed to get script snapshot while trying to read ${fileName}`);
68+
}
69+
const version = this.project.getScriptVersion(fileName);
70+
this.templateVersion.set(fileName, version);
71+
this.modifiedTemplates.delete(fileName);
72+
return snapshot.getText(0, snapshot.getLength());
73+
}
74+
75+
/**
76+
* getModifiedResourceFiles() is an Angular-specific method for notifying
77+
* the Angular compiler templates that have changed since it last read them.
78+
* It is a method on ExtendedTsCompilerHost, see
79+
* packages/compiler-cli/src/ngtsc/core/api/src/interfaces.ts
80+
*/
81+
getModifiedResourceFiles(): Set<string> {
82+
return this.modifiedTemplates;
83+
}
84+
85+
/**
86+
* Check whether the specified `fileName` is newer than the last time it was
87+
* read. If it is newer, register it and return true, otherwise do nothing and
88+
* return false.
89+
* @param fileName path to external template
90+
*/
91+
registerTemplateUpdate(fileName: string): boolean {
92+
if (!isExternalTemplate(fileName)) {
93+
return false;
94+
}
95+
const lastVersion = this.templateVersion.get(fileName);
96+
const latestVersion = this.project.getScriptVersion(fileName);
97+
if (lastVersion !== latestVersion) {
98+
this.modifiedTemplates.add(fileName);
99+
return true;
100+
}
101+
return false;
102+
}
103+
}
104+
105+
export function isTypeScriptFile(fileName: string): boolean {
106+
return fileName.endsWith('.ts');
107+
}
108+
109+
export function isExternalTemplate(fileName: string): boolean {
110+
return !isTypeScriptFile(fileName);
111+
}

‎packages/language-service/ivy/test/diagnostic_spec.ts

+33-2
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import * as ts from 'typescript/lib/tsserverlibrary';
1010

1111
import {LanguageService} from '../language_service';
1212

13-
import {APP_COMPONENT, setup} from './mock_host';
13+
import {APP_COMPONENT, setup, TEST_TEMPLATE} from './mock_host';
1414

15-
describe('diagnostic', () => {
15+
describe('getSemanticDiagnostics', () => {
1616
const {project, service, tsLS} = setup();
1717
const ngLS = new LanguageService(project, tsLS);
1818

@@ -35,4 +35,35 @@ describe('diagnostic', () => {
3535
expect(text.substring(start!, start! + length!)).toBe('nope');
3636
expect(messageText).toBe(`Property 'nope' does not exist on type 'AppComponent'.`);
3737
});
38+
39+
it('should process external template', () => {
40+
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
41+
expect(diags).toEqual([]);
42+
});
43+
44+
it('should report member does not exist in external template', () => {
45+
const {text} = service.overwrite(TEST_TEMPLATE, `{{ nope }}`);
46+
const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
47+
expect(diags.length).toBe(1);
48+
const {category, file, start, length, messageText} = diags[0];
49+
expect(category).toBe(ts.DiagnosticCategory.Error);
50+
expect(file?.fileName).toBe(TEST_TEMPLATE);
51+
expect(text.substring(start!, start! + length!)).toBe('nope');
52+
expect(messageText).toBe(`Property 'nope' does not exist on type 'TemplateReference'.`);
53+
});
54+
55+
it('should retrieve external template from latest snapshot', () => {
56+
// This test is to make sure we are reading from snapshot instead of disk
57+
// if content from snapshot is newer. It also makes sure the internal cache
58+
// of the resource loader is invalidated on content change.
59+
service.overwrite(TEST_TEMPLATE, `{{ foo }}`);
60+
const d1 = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
61+
expect(d1.length).toBe(1);
62+
expect(d1[0].messageText).toBe(`Property 'foo' does not exist on type 'TemplateReference'.`);
63+
64+
service.overwrite(TEST_TEMPLATE, `{{ bar }}`);
65+
const d2 = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
66+
expect(d2.length).toBe(1);
67+
expect(d2[0].messageText).toBe(`Property 'bar' does not exist on type 'TemplateReference'.`);
68+
});
3869
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {LanguageServiceAdapter} from '../language_service_adapter';
10+
import {setup, TEST_TEMPLATE} from './mock_host';
11+
12+
const {project, service} = setup();
13+
14+
describe('Language service adapter', () => {
15+
it('should register update if it has not seen the template before', () => {
16+
const adapter = new LanguageServiceAdapter(project);
17+
// Note that readResource() has never been called, so the adapter has no
18+
// knowledge of the template at all.
19+
const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
20+
expect(isRegistered).toBeTrue();
21+
expect(adapter.getModifiedResourceFiles().size).toBe(1);
22+
});
23+
24+
it('should not register update if template has not changed', () => {
25+
const adapter = new LanguageServiceAdapter(project);
26+
adapter.readResource(TEST_TEMPLATE);
27+
const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
28+
expect(isRegistered).toBeFalse();
29+
expect(adapter.getModifiedResourceFiles().size).toBe(0);
30+
});
31+
32+
it('should register update if template has changed', () => {
33+
const adapter = new LanguageServiceAdapter(project);
34+
adapter.readResource(TEST_TEMPLATE);
35+
service.overwrite(TEST_TEMPLATE, '<p>Hello World</p>');
36+
const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
37+
expect(isRegistered).toBe(true);
38+
expect(adapter.getModifiedResourceFiles().size).toBe(1);
39+
});
40+
41+
it('should clear template updates on read', () => {
42+
const adapter = new LanguageServiceAdapter(project);
43+
const isRegistered = adapter.registerTemplateUpdate(TEST_TEMPLATE);
44+
expect(isRegistered).toBeTrue();
45+
expect(adapter.getModifiedResourceFiles().size).toBe(1);
46+
adapter.readResource(TEST_TEMPLATE);
47+
expect(adapter.getModifiedResourceFiles().size).toBe(0);
48+
});
49+
});

‎packages/language-service/ivy/test/mock_host.ts

+17-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {join} from 'path';
1010
import * as ts from 'typescript/lib/tsserverlibrary';
11+
import {isTypeScriptFile} from '../language_service_adapter';
1112

1213
const logger: ts.server.Logger = {
1314
close(): void{},
@@ -161,10 +162,24 @@ class MockService {
161162

162163
getScriptInfo(fileName: string): ts.server.ScriptInfo {
163164
const scriptInfo = this.ps.getScriptInfo(fileName);
164-
if (!scriptInfo) {
165+
if (scriptInfo) {
166+
return scriptInfo;
167+
}
168+
// There is no script info for external template, so create one.
169+
// But we need to make sure it's not a TS file.
170+
if (isTypeScriptFile(fileName)) {
165171
throw new Error(`No existing script info for ${fileName}`);
166172
}
167-
return scriptInfo;
173+
const newScriptInfo = this.ps.getOrCreateScriptInfoForNormalizedPath(
174+
ts.server.toNormalizedPath(fileName),
175+
true, // openedByClient
176+
'', // fileContent
177+
ts.ScriptKind.External, // scriptKind
178+
);
179+
if (!newScriptInfo) {
180+
throw new Error(`Failed to create new script info for ${fileName}`);
181+
}
182+
return newScriptInfo;
168183
}
169184

170185
/**

0 commit comments

Comments
 (0)
Please sign in to comment.