refactor(compiler-cli): use TemplateDiagnostic in extended package (#43134)

Return `TemplateDiagnostic` instead of `ts.Diagnostic` when getting the
extended template diagnostics. This makes the integration with the
language service easier. This also fixes the error code and now uses the
`ngErrorCode` for extended template diagnostics.

Refs #42966

PR Close #43134
This commit is contained in:
Daniel Trevino
2021-08-11 23:33:58 +00:00
committed by Alex Rickabaugh
parent 27fdd52fb9
commit 2d8fc4a7ce
18 changed files with 73 additions and 67 deletions
@@ -8,6 +8,7 @@
import {AbsoluteSourceSpan, BoundTarget, DirectiveMeta, ParseSourceSpan, SchemaMetadata} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode} from '../../diagnostics';
import {AbsoluteFsPath} from '../../file_system';
import {Reference} from '../../imports';
@@ -28,6 +29,27 @@ export interface TypeCheckableDirectiveMeta extends DirectiveMeta, DirectiveType
export type TemplateId = string&{__brand: 'TemplateId'};
/**
* A `ts.Diagnostic` with additional information about the diagnostic related to template
* type-checking.
*/
export interface TemplateDiagnostic extends ts.Diagnostic {
/**
* The component with the template that resulted in this diagnostic.
*/
componentFile: ts.SourceFile;
/**
* The template id of the component that resulted in this diagnostic.
*/
templateId: TemplateId;
}
/**
* A `TemplateDiagnostic` with a specific error code.
*/
export type NgTemplateDiagnostic<T extends ErrorCode> = TemplateDiagnostic&{__ngCode: T};
/**
* Metadata required in addition to a component class in order to generate a type check block (TCB)
* for that component.
@@ -12,7 +12,7 @@ import {TextAttribute} from '@angular/compiler/src/render3/r3_ast';
import * as ts from 'typescript';
import {ErrorCode} from '../../diagnostics';
import {FullTemplateMapping, TypeCheckableDirectiveMeta} from './api';
import {FullTemplateMapping, NgTemplateDiagnostic, TypeCheckableDirectiveMeta} from './api';
import {GlobalCompletion} from './completion';
import {DirectiveInScope, PipeInScope} from './scope';
import {DirectiveSymbol, ElementSymbol, ShimLocation, Symbol, TemplateSymbol} from './symbols';
@@ -175,7 +175,7 @@ export interface TemplateTypeChecker {
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): ts.Diagnostic;
}[]): NgTemplateDiagnostic<T>;
}
/**
@@ -9,23 +9,7 @@
import {ParseSourceSpan} from '@angular/compiler';
import * as ts from 'typescript';
import {ExternalTemplateSourceMapping, TemplateId, TemplateSourceMapping} from '../../api';
/**
* A `ts.Diagnostic` with additional information about the diagnostic related to template
* type-checking.
*/
export interface TemplateDiagnostic extends ts.Diagnostic {
/**
* The component with the template that resulted in this diagnostic.
*/
componentFile: ts.SourceFile;
/**
* The template id of the component that resulted in this diagnostic.
*/
templateId: TemplateId;
}
import {ExternalTemplateSourceMapping, TemplateDiagnostic, TemplateId, TemplateSourceMapping} from '../../api';
/**
* Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
@@ -10,12 +10,7 @@ import {TmplAstNode} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode} from '../../../diagnostics';
import {TemplateTypeChecker} from '../../api';
/**
* A `ts.Diangostic` with a specific error code.
*/
export type TemplateDiagnostic<T extends ErrorCode> = ts.Diagnostic&{code: T};
import {NgTemplateDiagnostic, TemplateTypeChecker} from '../../api';
/**
* A Template Check receives information about the template it's checking and returns
@@ -26,7 +21,7 @@ export interface TemplateCheck<T extends ErrorCode> {
code: T;
/** Runs check and returns information about the diagnostics to be generated. */
run(ctx: TemplateContext, template: TmplAstNode[]): TemplateDiagnostic<T>[];
run(ctx: TemplateContext, template: TmplAstNode[]): NgTemplateDiagnostic<T>[];
}
/**
@@ -7,6 +7,7 @@
*/
import * as ts from 'typescript';
import {TemplateDiagnostic} from '../../api';
/**
* Interface to generate extended template diangostics from the component tempaltes.
@@ -15,5 +16,5 @@ export interface ExtendedTemplateChecker {
/**
* Run `TemplateCheck`s for a component and return the generated `ts.Diagnostic`s.
*/
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[];
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): TemplateDiagnostic[];
}
@@ -9,7 +9,7 @@
import * as ts from 'typescript';
import {ErrorCode} from '../../../diagnostics';
import {TemplateTypeChecker} from '../../api';
import {TemplateDiagnostic, TemplateTypeChecker} from '../../api';
import {ExtendedTemplateChecker, TemplateCheck, TemplateContext} from '../api';
export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
@@ -18,7 +18,7 @@ export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
private readonly typeChecker: ts.TypeChecker,
private readonly templateChecks: TemplateCheck<ErrorCode>[]) {}
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[] {
getExtendedTemplateDiagnosticsForComponent(component: ts.ClassDeclaration): TemplateDiagnostic[] {
const template = this.templateTypeChecker.getTemplate(component);
// Skip checks if component has no template. This can happen if the user writes a
// `@Component()` but doesn't add the template, could happen in the language service
@@ -26,7 +26,7 @@ export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
if (template === null) {
return [];
}
const diagnostics: ts.Diagnostic[] = [];
const diagnostics: TemplateDiagnostic[] = [];
const ctx = {
templateTypeChecker: this.templateTypeChecker,
@@ -78,8 +78,8 @@ export class ExtendedTemplateCheckerImpl implements ExtendedTemplateChecker {
// TODO(danieltrevino): handle duplicated diagnostics when they are being generated
// to avoid extra work (could be directly in the visitor).
// https://github.com/angular/angular/pull/42984#discussion_r684823926
function deduplicateDiagnostics(diagnostics: ts.Diagnostic[]): ts.Diagnostic[] {
const result: ts.Diagnostic[] = [];
function deduplicateDiagnostics<T extends ts.Diagnostic>(diagnostics: T[]): T[] {
const result: T[] = [];
for (const newDiag of diagnostics) {
const isDuplicateDiag = result.some(existingDiag => areDiagnosticsEqual(newDiag, existingDiag));
if (!isDuplicateDiag) {
@@ -7,6 +7,7 @@ ts_library(
deps = [
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/diagnostics",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
"@npm//typescript",
],
@@ -8,8 +8,10 @@
import {TmplAstBoundEvent, TmplAstNode, TmplAstRecursiveVisitor} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode} from '../../../../../diagnostics';
import {TemplateCheck, TemplateContext, TemplateDiagnostic} from '../../../api';
import {NgTemplateDiagnostic} from '../../../../api';
import {TemplateCheck, TemplateContext} from '../../../api';
/**
* Ensures the two-way binding syntax is correct.
@@ -20,7 +22,7 @@ export class InvalidBananaInBoxCheck implements TemplateCheck<ErrorCode.INVALID_
code: ErrorCode.INVALID_BANANA_IN_BOX = 8101;
run(ctx: TemplateContext,
template: TmplAstNode[]): TemplateDiagnostic<ErrorCode.INVALID_BANANA_IN_BOX>[] {
template: TmplAstNode[]): NgTemplateDiagnostic<ErrorCode.INVALID_BANANA_IN_BOX>[] {
const visitor = new BananaVisitor(ctx);
return visitor.getDiagnostics(template);
@@ -28,7 +30,7 @@ export class InvalidBananaInBoxCheck implements TemplateCheck<ErrorCode.INVALID_
}
class BananaVisitor extends TmplAstRecursiveVisitor {
private diagnostics: ts.Diagnostic[] = [];
private diagnostics: NgTemplateDiagnostic<ErrorCode.INVALID_BANANA_IN_BOX>[] = [];
constructor(public readonly ctx: TemplateContext) {
super();
@@ -45,17 +47,16 @@ class BananaVisitor extends TmplAstRecursiveVisitor {
if (name.startsWith('[') && name.endsWith(']')) {
const boundSyntax = boundEvent.sourceSpan.toString();
const expectedBoundSyntax = boundSyntax.replace(`(${name})`, `[(${name.slice(1, -1)})]`);
this.diagnostics.push(
this.ctx.templateTypeChecker.makeTemplateDiagnostic<ErrorCode.INVALID_BANANA_IN_BOX>(
this.ctx.component, boundEvent.sourceSpan, ts.DiagnosticCategory.Warning,
ErrorCode.INVALID_BANANA_IN_BOX,
`In the two-way binding syntax the parentheses should be inside the brackets, ex. '${
expectedBoundSyntax}'.
this.diagnostics.push(this.ctx.templateTypeChecker.makeTemplateDiagnostic(
this.ctx.component, boundEvent.sourceSpan, ts.DiagnosticCategory.Warning,
ErrorCode.INVALID_BANANA_IN_BOX,
`In the two-way binding syntax the parentheses should be inside the brackets, ex. '${
expectedBoundSyntax}'.
Find more at https://angular.io/guide/two-way-binding`));
}
}
getDiagnostics(template: TmplAstNode[]): ts.Diagnostic[] {
getDiagnostics(template: TmplAstNode[]): NgTemplateDiagnostic<ErrorCode.INVALID_BANANA_IN_BOX>[] {
this.diagnostics = [];
for (const node of template) {
node.visit(this);
@@ -7,7 +7,7 @@
*/
import * as ts from 'typescript';
import {ErrorCode} from '../../../../../diagnostics';
import {ErrorCode, ngErrorCode} from '../../../../../diagnostics';
import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system';
import {runInEachFileSystem} from '../../../../../file_system/testing';
import {getSourceCodeForDiagnostic} from '../../../../../testing';
@@ -33,7 +33,7 @@ runInEachFileSystem(() => {
const diags = extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="var1"');
});
@@ -94,10 +94,10 @@ runInEachFileSystem(() => {
const diags = extendedTemplateChecker.getExtendedTemplateDiagnosticsForComponent(component);
expect(diags.length).toBe(2);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="var1"');
expect(diags[1].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[1].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(diags[1].code).toBe(ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX));
expect(getSourceCodeForDiagnostic(diags[1])).toBe('([notARealThing2])="var1"');
});
});
@@ -9,7 +9,7 @@
import {AST, CssSelector, DomElementSchemaRegistry, LiteralPrimitive, MethodCall, ParseError, ParseSourceSpan, parseTemplate, PropertyRead, SafeMethodCall, SafePropertyRead, TmplAstElement, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstVariable} from '@angular/compiler';
import {TextAttribute} from '@angular/compiler/src/render3/r3_ast';
import * as ts from 'typescript';
import {ErrorCode} from '../../diagnostics';
import {ErrorCode, ngErrorCode} from '../../diagnostics';
import {absoluteFrom, absoluteFromSourceFile, AbsoluteFsPath, getSourceFileOrError} from '../../file_system';
import {Reference, ReferenceEmitter} from '../../imports';
@@ -20,8 +20,8 @@ import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../r
import {ComponentScopeReader, TypeCheckScopeRegistry} from '../../scope';
import {isShim} from '../../shims';
import {getSourceFileOrNull, isSymbolWithValueDeclaration} from '../../util/src/typescript';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, ShimLocation, Symbol, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
import {DirectiveInScope, ElementSymbol, FullTemplateMapping, GlobalCompletion, NgTemplateDiagnostic, OptimizeFor, PipeInScope, ProgramTypeCheckAdapter, ShimLocation, Symbol, TemplateDiagnostic, TemplateId, TemplateSymbol, TemplateTypeChecker, TypeCheckableDirectiveMeta, TypeCheckingConfig} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';
import {CompletionEngine} from './completion';
import {InliningMode, ShimTypeCheckingData, TemplateData, TypeCheckContextImpl, TypeCheckingHost} from './context';
@@ -316,14 +316,18 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
start: number,
end: number,
sourceFile: ts.SourceFile,
}[]): TemplateDiagnostic {
}[]): NgTemplateDiagnostic<T> {
const sfPath = absoluteFromSourceFile(clazz.getSourceFile());
const fileRecord = this.state.get(sfPath)!;
const templateId = fileRecord.sourceManager.getTemplateId(clazz);
const mapping = fileRecord.sourceManager.getSourceMapping(templateId);
return makeTemplateDiagnostic(
templateId, mapping, sourceSpan, category, errorCode, message, relatedInformation);
return {
...makeTemplateDiagnostic(
templateId, mapping, sourceSpan, category, ngErrorCode(errorCode), message,
relatedInformation),
__ngCode: errorCode
};
}
private getOrCreateCompletionEngine(component: ts.ClassDeclaration): CompletionEngine|null {
@@ -16,8 +16,8 @@ import {PerfEvent, PerfRecorder} from '../../perf';
import {FileUpdate} from '../../program_driver';
import {ClassDeclaration, ReflectionHost} from '../../reflection';
import {ImportManager} from '../../translator';
import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
import {TemplateDiagnostic, TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';
import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
import {Environment} from './environment';
@@ -7,8 +7,8 @@
*/
import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler';
import * as ts from 'typescript';
import {TemplateId} from '../api';
import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
import {TemplateDiagnostic, TemplateId} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';
import {getTemplateMapping, TemplateSourceResolver} from './tcb_util';
@@ -10,8 +10,8 @@ import {DomElementSchemaRegistry, ParseSourceSpan, SchemaMetadata, TmplAstElemen
import * as ts from 'typescript';
import {ErrorCode, ngErrorCode} from '../../diagnostics';
import {TemplateId} from '../api';
import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
import {TemplateDiagnostic, TemplateId} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';
import {TemplateSourceResolver} from './tcb_util';
@@ -12,8 +12,8 @@ import * as ts from 'typescript';
import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
import {ClassDeclaration} from '../../reflection';
import {TemplateId} from '../api';
import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
import {TemplateDiagnostic, TemplateId} from '../api';
import {makeTemplateDiagnostic} from '../diagnostics';
import {TemplateSourceResolver} from './tcb_util';
@@ -21,7 +21,6 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/testing",
"//packages/compiler-cli/src/ngtsc/typecheck",
"//packages/compiler-cli/src/ngtsc/typecheck/api",
"//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
"//packages/compiler-cli/src/ngtsc/util",
"@npm//typescript",
],
@@ -20,9 +20,8 @@ import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} fro
import {ComponentScopeReader, LocalModuleScope, ScopeData, TypeCheckScopeRegistry} from '../../scope';
import {makeProgram} from '../../testing';
import {getRootDirs} from '../../util/src/typescript';
import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '../api';
import {ProgramTypeCheckAdapter, TemplateDiagnostic, TemplateTypeChecker, TypeCheckContext} from '../api';
import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig} from '../api/api';
import {TemplateDiagnostic} from '../diagnostics';
import {TemplateTypeCheckerImpl} from '../src/checker';
import {DomSchemaChecker} from '../src/dom';
import {Environment} from '../src/environment';
@@ -7,7 +7,7 @@
*/
import * as ts from 'typescript';
import {ErrorCode} from '../../src/ngtsc/diagnostics';
import {ErrorCode, ngErrorCode} from '../../src/ngtsc/diagnostics';
import {runInEachFileSystem} from '../../src/ngtsc/file_system/testing';
import {getSourceCodeForDiagnostic, loadStandardTestFiles} from '../../src/ngtsc/testing';
@@ -39,7 +39,7 @@ runInEachFileSystem(() => {
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="bar"');
});
@@ -62,7 +62,7 @@ runInEachFileSystem(() => {
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
expect(diags[0].code).toBe(ErrorCode.INVALID_BANANA_IN_BOX);
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INVALID_BANANA_IN_BOX));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('([notARealThing])="bar"');
});