diff --git a/goldens/public-api/compiler-cli/error_code.d.ts b/goldens/public-api/compiler-cli/error_code.d.ts index ccfbd47e71..9116012fe4 100644 --- a/goldens/public-api/compiler-cli/error_code.d.ts +++ b/goldens/public-api/compiler-cli/error_code.d.ts @@ -38,5 +38,6 @@ export declare enum ErrorCode { INLINE_TCB_REQUIRED = 8900, INLINE_TYPE_CTOR_REQUIRED = 8901, INJECTABLE_DUPLICATE_PROV = 9001, - SUGGEST_STRICT_TEMPLATES = 10001 + SUGGEST_STRICT_TEMPLATES = 10001, + SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002 } diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index d483243aad..32cedf2641 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -692,6 +692,10 @@ export class NgCompiler { strictLiteralTypes: true, enableTemplateTypeChecker: this.enableTemplateTypeChecker, useInlineTypeConstructors, + // Warnings for suboptimal type inference are only enabled if in Language Service mode + // (providing the full TemplateTypeChecker API) and if strict mode is not enabled. In strict + // mode, the user is in full control of type inference. + suggestionsForSuboptimalTypeInference: this.enableTemplateTypeChecker && !strictTemplates, }; } else { typeCheckingConfig = { @@ -717,6 +721,9 @@ export class NgCompiler { strictLiteralTypes: false, enableTemplateTypeChecker: this.enableTemplateTypeChecker, useInlineTypeConstructors, + // In "basic" template type-checking mode, no warnings are produced since most things are + // not checked anyways. + suggestionsForSuboptimalTypeInference: false, }; } 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 89292b56fa..910abcbde6 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -171,15 +171,22 @@ export enum ErrorCode { */ INJECTABLE_DUPLICATE_PROV = 9001, - // 10XXX error codes are reserved for diagnostics with category - // `ts.DiagnosticCategory.Suggestion`. These diagnostics are generated by - // language service. + // 10XXX error codes are reserved for diagnostics with categories other than + // `ts.DiagnosticCategory.Error`. These diagnostics are generated by the compiler when configured + // to do so by a tool such as the Language Service, or by the Language Service itself. /** * Suggest users to enable `strictTemplates` to make use of full capabilities * provided by Angular language service. */ SUGGEST_STRICT_TEMPLATES = 10001, + + /** + * Indicates that a particular structural directive provides advanced type narrowing + * functionality, but the current template type-checking configuration does not allow its usage in + * type inference. + */ + SUGGEST_SUBOPTIMAL_TYPE_INFERENCE = 10002, } /** diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts index 81b8c998e9..8f3c5a7d07 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/api.ts @@ -275,6 +275,20 @@ export interface TypeCheckingConfig { * inlining, this must be set to `false`. */ useInlineTypeConstructors: boolean; + + /** + * Whether or not to produce diagnostic suggestions in cases where the compiler could have + * inferred a better type for a construct, but was prevented from doing so by the current type + * checking configuration. + * + * For example, if the compiler could have used a template context guard to infer a better type + * for a structural directive's context and `let-` variables, but the user is in + * `fullTemplateTypeCheck` mode and such guards are therefore disabled. + * + * This mode is useful for clients like the Language Service which want to inform users of + * opportunities to improve their own developer experience. + */ + suggestionsForSuboptimalTypeInference: boolean; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts index aaec8d7927..446debb4ee 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts @@ -68,6 +68,12 @@ export interface OutOfBandDiagnosticRecorder { requiresInlineTypeConstructors( templateId: TemplateId, node: ClassDeclaration, directives: ClassDeclaration[]): void; + + /** + * Report a warning when structural directives support context guards, but the current + * type-checking configuration prohibits their usage. + */ + suboptimalTypeInference(templateId: TemplateId, variables: TmplAstVariable[]): void; } export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder { @@ -174,6 +180,37 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor directives.map( dir => makeRelatedInformation(dir.name, `Requires an inline type constructor.`)))); } + + suboptimalTypeInference(templateId: TemplateId, variables: TmplAstVariable[]): void { + const mapping = this.resolver.getSourceMapping(templateId); + + // Select one of the template variables that's most suitable for reporting the diagnostic. Any + // variable will do, but prefer one bound to the context's $implicit if present. + let diagnosticVar: TmplAstVariable|null = null; + for (const variable of variables) { + if (diagnosticVar === null || (variable.value === '' || variable.value === '$implicit')) { + diagnosticVar = variable; + } + } + if (diagnosticVar === null) { + // There is no variable on which to report the diagnostic. + return; + } + + let varIdentification = `'${diagnosticVar.name}'`; + if (variables.length === 2) { + varIdentification += ` (and 1 other)`; + } else if (variables.length > 2) { + varIdentification += ` (and ${variables.length - 1} others)`; + } + const message = + `This structural directive supports advanced type inference, but the current compiler configuration prevents its usage. The variable ${ + varIdentification} will have type 'any' as a result.\n\nConsider enabling the 'strictTemplates' option in your tsconfig.json for better type inference within this template.`; + + this._diagnostics.push(makeTemplateDiagnostic( + templateId, mapping, diagnosticVar.keySpan, ts.DiagnosticCategory.Suggestion, + ngErrorCode(ErrorCode.SUGGEST_SUBOPTIMAL_TYPE_INFERENCE), message)); + } } function makeInlineDiagnostic( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 6a6a6abdad..bf384afc45 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -286,11 +286,20 @@ class TcbTemplateBodyOp extends TcbOp { // The second kind of guard is a template context guard. This guard narrows the template // rendering context variable `ctx`. - if (dir.hasNgTemplateContextGuard && this.tcb.env.config.applyTemplateContextGuards) { - const ctx = this.scope.resolve(this.template); - const guardInvoke = tsCallMethod(dirId, 'ngTemplateContextGuard', [dirInstId, ctx]); - addParseSpanInfo(guardInvoke, this.template.sourceSpan); - directiveGuards.push(guardInvoke); + if (dir.hasNgTemplateContextGuard) { + if (this.tcb.env.config.applyTemplateContextGuards) { + const ctx = this.scope.resolve(this.template); + const guardInvoke = tsCallMethod(dirId, 'ngTemplateContextGuard', [dirInstId, ctx]); + addParseSpanInfo(guardInvoke, this.template.sourceSpan); + directiveGuards.push(guardInvoke); + } else if ( + this.template.variables.length > 0 && + this.tcb.env.config.suggestionsForSuboptimalTypeInference) { + // The compiler could have inferred a better type for the variables in this template, + // but was prevented from doing so by the type-checking configuration. Issue a warning + // diagnostic. + this.tcb.oobRecorder.suboptimalTypeInference(this.tcb.id, this.template.variables); + } } } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index e3935db015..0c8083b357 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -188,7 +188,8 @@ export const ALL_ENABLED_CONFIG: Readonly = { useContextGenericType: true, strictLiteralTypes: true, enableTemplateTypeChecker: false, - useInlineTypeConstructors: true + useInlineTypeConstructors: true, + suggestionsForSuboptimalTypeInference: false, }; // Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead. @@ -272,6 +273,7 @@ export function tcb( strictLiteralTypes: true, enableTemplateTypeChecker: false, useInlineTypeConstructors: true, + suggestionsForSuboptimalTypeInference: false, ...config }; options = options || { @@ -690,4 +692,5 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { duplicateTemplateVar(): void {} requiresInlineTcb(): void {} requiresInlineTypeConstructors(): void {} + suboptimalTypeInference(): void {} } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 5f06d3a358..9ccb53db84 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -745,7 +745,8 @@ describe('type check blocks', () => { useContextGenericType: true, strictLiteralTypes: true, enableTemplateTypeChecker: false, - useInlineTypeConstructors: true + useInlineTypeConstructors: true, + suggestionsForSuboptimalTypeInference: false, }; describe('config.applyTemplateContextGuards', () => { diff --git a/packages/language-service/ivy/test/BUILD.bazel b/packages/language-service/ivy/test/BUILD.bazel index 2ffce0d65a..2fc4b2a7f5 100644 --- a/packages/language-service/ivy/test/BUILD.bazel +++ b/packages/language-service/ivy/test/BUILD.bazel @@ -7,6 +7,7 @@ ts_library( deps = [ "//packages/compiler", "//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/file_system/testing", "//packages/compiler-cli/src/ngtsc/testing", diff --git a/packages/language-service/ivy/test/diagnostic_spec.ts b/packages/language-service/ivy/test/diagnostic_spec.ts index 97f7239d33..d1264db207 100644 --- a/packages/language-service/ivy/test/diagnostic_spec.ts +++ b/packages/language-service/ivy/test/diagnostic_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 {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import * as ts from 'typescript'; @@ -245,4 +246,40 @@ describe('getSemanticDiagnostics', () => { 'component is missing a template', ]); }); + + it('reports a warning when the project configuration prevents good type inference', () => { + const files = { + 'app.ts': ` + import {Component, NgModule} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({ + template: '
{{user}}
', + }) + export class MyComponent { + users = ['Alpha', 'Beta']; + } + ` + }; + + const project = createModuleAndProjectWithDeclarations(env, 'test', files, { + // Disable `strictTemplates`. + strictTemplates: false, + // Use `fullTemplateTypeCheck` mode instead. + fullTemplateTypeCheck: true, + }); + const diags = project.getDiagnosticsForFile('app.ts'); + expect(diags.length).toBe(1); + const diag = diags[0]; + expect(diag.code).toBe(ngErrorCode(ErrorCode.SUGGEST_SUBOPTIMAL_TYPE_INFERENCE)); + expect(diag.category).toBe(ts.DiagnosticCategory.Suggestion); + expect(getTextOfDiagnostic(diag)).toBe('user'); + }); }); + +function getTextOfDiagnostic(diag: ts.Diagnostic): string { + expect(diag.file).not.toBeUndefined(); + expect(diag.start).not.toBeUndefined(); + expect(diag.length).not.toBeUndefined(); + return diag.file!.text.substring(diag.start!, diag.start! + diag.length!); +} diff --git a/packages/language-service/ivy/testing/src/project.ts b/packages/language-service/ivy/testing/src/project.ts index 5f2ca94203..5577f7c705 100644 --- a/packages/language-service/ivy/testing/src/project.ts +++ b/packages/language-service/ivy/testing/src/project.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {StrictTemplateOptions} from '@angular/compiler-cli/src/ngtsc/core/api'; +import {LegacyNgcOptions, StrictTemplateOptions} from '@angular/compiler-cli/src/ngtsc/core/api'; import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system'; import {OptimizeFor, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import * as ts from 'typescript/lib/tsserverlibrary'; @@ -19,7 +19,7 @@ export type ProjectFiles = { function writeTsconfig( fs: FileSystem, tsConfigPath: AbsoluteFsPath, entryFiles: AbsoluteFsPath[], - options: StrictTemplateOptions): void { + options: TestableOptions): void { fs.writeFile( tsConfigPath, JSON.stringify( @@ -44,7 +44,7 @@ function writeTsconfig( null, 2)); } -export type TestableOptions = StrictTemplateOptions; +export type TestableOptions = StrictTemplateOptions&Pick; export class Project { private tsProject: ts.server.Project;