From ecae75f4771922b8d3732e9df0a30f874aa4e207 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Wed, 13 Jan 2021 14:52:29 -0800 Subject: [PATCH] feat(language-service): Add diagnostics to suggest turning on strict mode (#40423) This PR adds a way for the language server to retrieve compiler options diagnostics via `languageService.getCompilerOptionsDiagnostics()`. This will be used by the language server to show a prompt in the editor if users don't have `strict` or `fullTemplateTypeCheck` turned on. Ref https://github.com/angular/vscode-ng-language-service/issues/1053 PR Close #40423 --- .../public-api/compiler-cli/error_code.d.ts | 3 +- .../src/ngtsc/core/api/src/public_options.ts | 4 +- .../src/ngtsc/core/src/compiler.ts | 2 +- .../src/ngtsc/diagnostics/src/error_code.ts | 10 ++++ packages/language-service/ivy/BUILD.bazel | 1 + .../language-service/ivy/language_service.ts | 32 +++++++++- .../ivy/test/legacy/BUILD.bazel | 2 + .../ivy/test/legacy/language_service_spec.ts | 58 ++++++++++++++----- .../ivy/test/legacy/mock_host.ts | 10 +++- packages/language-service/ivy/ts_plugin.ts | 12 ++++ 10 files changed, 112 insertions(+), 22 deletions(-) diff --git a/goldens/public-api/compiler-cli/error_code.d.ts b/goldens/public-api/compiler-cli/error_code.d.ts index b9ed59e469..3d37af79c6 100644 --- a/goldens/public-api/compiler-cli/error_code.d.ts +++ b/goldens/public-api/compiler-cli/error_code.d.ts @@ -35,5 +35,6 @@ export declare enum ErrorCode { DUPLICATE_VARIABLE_DECLARATION = 8006, INLINE_TCB_REQUIRED = 8900, INLINE_TYPE_CTOR_REQUIRED = 8901, - INJECTABLE_DUPLICATE_PROV = 9001 + INJECTABLE_DUPLICATE_PROV = 9001, + SUGGEST_STRICT_TEMPLATES = 10001 } diff --git a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts index 98b41f8f07..115554b7d9 100644 --- a/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts +++ b/packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts @@ -127,9 +127,9 @@ export interface StrictTemplateOptions { /** * If `true`, implies all template strictness flags below (unless individually disabled). * - * Has no effect unless `fullTemplateTypeCheck` is also enabled. + * This flag is a superset of `fullTemplateTypeCheck`. * - * Defaults to `false`, even if "fullTemplateTypeCheck" is set. + * Defaults to `false`, even if "fullTemplateTypeCheck" is `true`. */ strictTemplates?: boolean; diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 127d7253ad..840d52f500 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -906,7 +906,7 @@ function getR3SymbolsFile(program: ts.Program): ts.SourceFile|null { /** * Since "strictTemplates" is a true superset of type checking capabilities compared to - * "strictTemplateTypeCheck", it is required that the latter is not explicitly disabled if the + * "fullTemplateTypeCheck", it is required that the latter is not explicitly disabled if the * former is enabled. */ function verifyCompatibleTypeCheckOptions(options: NgCompilerOptions): ts.Diagnostic|null { diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index ac7930e3da..d34b791bfd 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -159,6 +159,16 @@ export enum ErrorCode { * An injectable already has a `ɵprov` property. */ INJECTABLE_DUPLICATE_PROV = 9001, + + // 10XXX error codes are reserved for diagnostics with category + // `ts.DiagnosticCategory.Suggestion`. These diagnostics are generated by + // language service. + + /** + * Suggest users to enable `strictTemplates` to make use of full capabilities + * provided by Angular language service. + */ + SUGGEST_STRICT_TEMPLATES = 10001, } /** diff --git a/packages/language-service/ivy/BUILD.bazel b/packages/language-service/ivy/BUILD.bazel index 37b67f6c39..0a7aa1cb78 100644 --- a/packages/language-service/ivy/BUILD.bazel +++ b/packages/language-service/ivy/BUILD.bazel @@ -10,6 +10,7 @@ ts_library( "//packages/compiler-cli", "//packages/compiler-cli/src/ngtsc/core", "//packages/compiler-cli/src/ngtsc/core:api", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/incremental", diff --git a/packages/language-service/ivy/language_service.ts b/packages/language-service/ivy/language_service.ts index 5ff0c40fed..1895ad595d 100644 --- a/packages/language-service/ivy/language_service.ts +++ b/packages/language-service/ivy/language_service.ts @@ -9,6 +9,7 @@ import {AbsoluteSourceSpan, AST, ParseSourceSpan, TmplAstBoundEvent, TmplAstNode} from '@angular/compiler'; import {CompilerOptions, ConfigurationHost, readConfiguration} from '@angular/compiler-cli'; import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; +import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics'; import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system'; import {TypeCheckShimGenerator} from '@angular/compiler-cli/src/ngtsc/typecheck'; import {OptimizeFor, TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; @@ -50,7 +51,8 @@ export class LanguageService { private readonly adapter: LanguageServiceAdapter; private readonly parseConfigHost: LSParseConfigHost; - constructor(project: ts.server.Project, private readonly tsLS: ts.LanguageService) { + constructor( + private readonly project: ts.server.Project, private readonly tsLS: ts.LanguageService) { this.parseConfigHost = new LSParseConfigHost(project.projectService.host); this.options = parseNgCompilerOptions(project, this.parseConfigHost); logCompilerOptions(project, this.options); @@ -268,6 +270,34 @@ export class LanguageService { return result; } + getCompilerOptionsDiagnostics(): ts.Diagnostic[] { + const project = this.project; + if (!(project instanceof ts.server.ConfiguredProject)) { + return []; + } + + const diagnostics: ts.Diagnostic[] = []; + const configSourceFile = ts.readJsonConfigFile( + project.getConfigFilePath(), (path: string) => project.readFile(path)); + + if (!this.options.strictTemplates && !this.options.fullTemplateTypeCheck) { + diagnostics.push({ + messageText: 'Some language features are not available. ' + + 'To access all features, enable `strictTemplates` in `angularCompilerOptions`.', + category: ts.DiagnosticCategory.Suggestion, + code: ngErrorCode(ErrorCode.SUGGEST_STRICT_TEMPLATES), + file: configSourceFile, + start: undefined, + length: undefined, + }); + } + + const compiler = this.compilerFactory.getOrCreate(); + diagnostics.push(...compiler.getOptionDiagnostics()); + + return diagnostics; + } + private watchConfigFile(project: ts.server.Project) { // TODO: Check the case when the project is disposed. An InferredProject // could be disposed when a tsconfig.json is added to the workspace, diff --git a/packages/language-service/ivy/test/legacy/BUILD.bazel b/packages/language-service/ivy/test/legacy/BUILD.bazel index 776992f3f0..28b0ff63ad 100644 --- a/packages/language-service/ivy/test/legacy/BUILD.bazel +++ b/packages/language-service/ivy/test/legacy/BUILD.bazel @@ -6,6 +6,8 @@ ts_library( srcs = glob(["*.ts"]), deps = [ "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core:api", + "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/language-service/ivy", "@npm//typescript", ], diff --git a/packages/language-service/ivy/test/legacy/language_service_spec.ts b/packages/language-service/ivy/test/legacy/language_service_spec.ts index 71ff969447..6874be3bc0 100644 --- a/packages/language-service/ivy/test/legacy/language_service_spec.ts +++ b/packages/language-service/ivy/test/legacy/language_service_spec.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {ErrorCode, ngErrorCode} from '@angular/compiler-cli/src/ngtsc/diagnostics'; import * as ts from 'typescript/lib/tsserverlibrary'; import {LanguageService} from '../../language_service'; @@ -31,15 +32,6 @@ describe('language service adapter', () => { }); describe('parse compiler options', () => { - beforeEach(() => { - // Need to reset project on each test to reinitialize file watchers. - const {project: _project, tsLS, service: _service, configFileFs: _configFileFs} = setup(); - project = _project; - service = _service; - configFileFs = _configFileFs; - ngLS = new LanguageService(project, tsLS); - }); - it('should initialize with angularCompilerOptions from tsconfig.json', () => { expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({ enableIvy: true, // default for ivy is true @@ -55,11 +47,11 @@ describe('language service adapter', () => { strictInjectionParameters: true, })); - configFileFs.overwriteConfigFile(TSCONFIG, `{ - "angularCompilerOptions": { - "strictTemplates": false - } - }`); + configFileFs.overwriteConfigFile(TSCONFIG, { + angularCompilerOptions: { + strictTemplates: false, + } + }); expect(ngLS.getCompilerOptions()).toEqual(jasmine.objectContaining({ strictTemplates: false, @@ -67,6 +59,44 @@ describe('language service adapter', () => { }); }); + describe('compiler options diagnostics', () => { + it('suggests turning on strict flag', () => { + configFileFs.overwriteConfigFile(TSCONFIG, { + angularCompilerOptions: {}, + }); + const diags = ngLS.getCompilerOptionsDiagnostics(); + const diag = diags.find(isSuggestStrictTemplatesDiag); + expect(diag).toBeDefined(); + expect(diag!.category).toBe(ts.DiagnosticCategory.Suggestion); + expect(diag!.file?.getSourceFile().fileName).toBe(TSCONFIG); + }); + + it('does not suggest turning on strict mode is strictTemplates flag is on', () => { + configFileFs.overwriteConfigFile(TSCONFIG, { + angularCompilerOptions: { + strictTemplates: true, + }, + }); + const diags = ngLS.getCompilerOptionsDiagnostics(); + const diag = diags.find(isSuggestStrictTemplatesDiag); + expect(diag).toBeUndefined(); + }); + + it('does not suggest turning on strict mode is fullTemplateTypeCheck flag is on', () => { + configFileFs.overwriteConfigFile(TSCONFIG, { + angularCompilerOptions: { + fullTemplateTypeCheck: true, + }, + }); + const diags = ngLS.getCompilerOptionsDiagnostics(); + const diag = diags.find(isSuggestStrictTemplatesDiag); + expect(diag).toBeUndefined(); + }); + + function isSuggestStrictTemplatesDiag(diag: ts.Diagnostic) { + return diag.code === ngErrorCode(ErrorCode.SUGGEST_STRICT_TEMPLATES); + } + }); describe('last known program', () => { beforeEach(() => { diff --git a/packages/language-service/ivy/test/legacy/mock_host.ts b/packages/language-service/ivy/test/legacy/mock_host.ts index d825907e41..4b97e42b1a 100644 --- a/packages/language-service/ivy/test/legacy/mock_host.ts +++ b/packages/language-service/ivy/test/legacy/mock_host.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {NgCompilerOptions} from '@angular/compiler-cli/src/ngtsc/core/api'; import {join} from 'path'; import * as ts from 'typescript/lib/tsserverlibrary'; @@ -68,11 +69,11 @@ export class MockConfigFileFs implements private configOverwrites = new Map(); private configFileWatchers = new Map(); - overwriteConfigFile(configFile: string, contents: string) { + overwriteConfigFile(configFile: string, contents: {angularCompilerOptions?: NgCompilerOptions}) { if (!configFile.endsWith('.json')) { throw new Error(`${configFile} is not a configuration file.`); } - this.configOverwrites.set(configFile, contents); + this.configOverwrites.set(configFile, JSON.stringify(contents)); this.configFileWatchers.get(configFile)?.changed(); } @@ -98,8 +99,11 @@ export class MockConfigFileFs implements } clear() { + for (const [fileName, watcher] of this.configFileWatchers) { + this.configOverwrites.delete(fileName); + watcher.changed(); + } this.configOverwrites.clear(); - this.configFileWatchers.clear(); } } diff --git a/packages/language-service/ivy/ts_plugin.ts b/packages/language-service/ivy/ts_plugin.ts index 6fae724673..89f4c27a13 100644 --- a/packages/language-service/ivy/ts_plugin.ts +++ b/packages/language-service/ivy/ts_plugin.ts @@ -119,6 +119,17 @@ export function create(info: ts.server.PluginCreateInfo): NgLanguageService { ngLS.getCompletionEntrySymbol(fileName, position, name); } } + /** + * Gets global diagnostics related to the program configuration and compiler options. + */ + function getCompilerOptionsDiagnostics(): ts.Diagnostic[] { + const diagnostics: ts.Diagnostic[] = []; + if (!angularOnly) { + diagnostics.push(...tsLS.getCompilerOptionsDiagnostics()); + } + diagnostics.push(...ngLS.getCompilerOptionsDiagnostics()); + return diagnostics; + } function getTcb(fileName: string, position: number): GetTcbResponse { return ngLS.getTcb(fileName, position); @@ -137,6 +148,7 @@ export function create(info: ts.server.PluginCreateInfo): NgLanguageService { getCompletionEntryDetails, getCompletionEntrySymbol, getTcb, + getCompilerOptionsDiagnostics, }; }