Skip to content

Commit

Permalink
refactor(compiler-cli): support type-checking a single component (ang…
Browse files Browse the repository at this point in the history
…ular#38105)

This commit adds a method `getDiagnosticsForComponent` to the
`TemplateTypeChecker`, which does the minimum amount of work to retrieve
diagnostics for a single component.

With the normal `ReusedProgramStrategy` this offers virtually no improvement
over the standard `getDiagnosticsForFile` operation, but if the
`TypeCheckingProgramStrategy` supports separate shims for each component,
this operation can yield a faster turnaround for components that are
declared in files with many other components.

PR Close angular#38105
  • Loading branch information
alxhub authored and Splaktar committed Aug 8, 2020
1 parent 4096bbf commit 2968435
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 5 deletions.
7 changes: 7 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ export interface TemplateTypeChecker {
*/
getDiagnosticsForFile(sf: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[];

/**
* Get all `ts.Diagnostic`s currently available that pertain to the given component.
*
* This method always runs in `OptimizeFor.SingleFile` mode.
*/
getDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[];

/**
* Retrieve the top-level node representing the TCB for the given component.
*
Expand Down
93 changes: 88 additions & 5 deletions packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {getSourceFileOrNull} from '../../util/src/typescript';
import {OptimizeFor, ProgramTypeCheckAdapter, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';

import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
import {findTypeCheckBlock, shouldReportDiagnostic, TemplateDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
import {TemplateSourceManager} from './source';

/**
Expand Down Expand Up @@ -112,10 +112,44 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
diagnostics.push(...shimRecord.genesisDiagnostics);
}


return diagnostics.filter((diag: ts.Diagnostic|null): diag is ts.Diagnostic => diag !== null);
}

getDiagnosticsForComponent(component: ts.ClassDeclaration): ts.Diagnostic[] {
this.ensureShimForComponent(component);

const sf = component.getSourceFile();
const sfPath = absoluteFromSourceFile(sf);
const shimPath = this.typeCheckingStrategy.shimPathForComponent(component);

const fileRecord = this.getFileData(sfPath);

if (!fileRecord.shimData.has(shimPath)) {
return [];
}

const templateId = fileRecord.sourceManager.getTemplateId(component);
const shimRecord = fileRecord.shimData.get(shimPath)!;

const typeCheckProgram = this.typeCheckingStrategy.getProgram();

const diagnostics: (TemplateDiagnostic|null)[] = [];
if (shimRecord.hasInlines) {
const inlineSf = getSourceFileOrError(typeCheckProgram, sfPath);
diagnostics.push(...typeCheckProgram.getSemanticDiagnostics(inlineSf).map(
diag => convertDiagnostic(diag, fileRecord.sourceManager)));
}

const shimSf = getSourceFileOrError(typeCheckProgram, shimPath);
diagnostics.push(...typeCheckProgram.getSemanticDiagnostics(shimSf).map(
diag => convertDiagnostic(diag, fileRecord.sourceManager)));
diagnostics.push(...shimRecord.genesisDiagnostics);

return diagnostics.filter(
(diag: TemplateDiagnostic|null): diag is TemplateDiagnostic =>
diag !== null && diag.templateId === templateId);
}

getTypeCheckBlock(component: ts.ClassDeclaration): ts.Node|null {
this.ensureAllShimsForOneFile(component.getSourceFile());

Expand Down Expand Up @@ -219,6 +253,28 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
this.updateFromContext(ctx);
}

private ensureShimForComponent(component: ts.ClassDeclaration): void {
const sf = component.getSourceFile();
const sfPath = absoluteFromSourceFile(sf);

this.maybeAdoptPriorResultsForFile(sf);

const fileData = this.getFileData(sfPath);
const shimPath = this.typeCheckingStrategy.shimPathForComponent(component);

if (fileData.shimData.has(shimPath)) {
// All data for this component is available.
return;
}

const host =
new SingleShimTypeCheckingHost(sfPath, fileData, this.typeCheckingStrategy, this, shimPath);
const ctx = this.newContext(host);

this.typeCheckAdapter.typeCheck(sf, ctx);
this.updateFromContext(ctx);
}

private newContext(host: TypeCheckingHost): TypeCheckContextImpl {
const inlining = this.typeCheckingStrategy.supportsInlineOperations ? InliningMode.InlineOps :
InliningMode.Error;
Expand Down Expand Up @@ -272,7 +328,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
}

function convertDiagnostic(
diag: ts.Diagnostic, sourceResolver: TemplateSourceResolver): ts.Diagnostic|null {
diag: ts.Diagnostic, sourceResolver: TemplateSourceResolver): TemplateDiagnostic|null {
if (!shouldReportDiagnostic(diag)) {
return null;
}
Expand Down Expand Up @@ -367,8 +423,8 @@ class SingleFileTypeCheckingHost implements TypeCheckingHost {
private seenInlines = false;

constructor(
private sfPath: AbsoluteFsPath, private fileData: FileTypeCheckingData,
private strategy: TypeCheckingProgramStrategy, private impl: TemplateTypeCheckerImpl) {}
protected sfPath: AbsoluteFsPath, protected fileData: FileTypeCheckingData,
protected strategy: TypeCheckingProgramStrategy, protected impl: TemplateTypeCheckerImpl) {}

private assertPath(sfPath: AbsoluteFsPath): void {
if (this.sfPath !== sfPath) {
Expand Down Expand Up @@ -431,3 +487,30 @@ class SingleFileTypeCheckingHost implements TypeCheckingHost {
this.fileData.isComplete = true;
}
}

/**
* Drives a `TypeCheckContext` to generate type-checking code efficiently for only those components
* which map to a single shim of a single input file.
*/
class SingleShimTypeCheckingHost extends SingleFileTypeCheckingHost {
constructor(
sfPath: AbsoluteFsPath, fileData: FileTypeCheckingData, strategy: TypeCheckingProgramStrategy,
impl: TemplateTypeCheckerImpl, private shimPath: AbsoluteFsPath) {
super(sfPath, fileData, strategy, impl);
}

shouldCheckNode(node: ts.ClassDeclaration): boolean {
if (this.sfPath !== absoluteFromSourceFile(node.getSourceFile())) {
return false;
}

// Only generate a TCB for the component if it maps to the requested shim file.
const shimPath = this.strategy.shimPathForComponent(node);
if (shimPath !== this.shimPath) {
return false;
}

// Only need to generate a TCB for the class if no shim exists for it currently.
return !this.fileData.shimData.has(shimPath);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,5 +318,30 @@ runInEachFileSystem(() => {
expect(currentTcb).toBe(originalTcb);
});
});

it('should allow get diagnostics for a single component', () => {
const fileName = absoluteFrom('/main.ts');

const {program, templateTypeChecker} = setup([{
fileName,
templates: {
'Cmp1': '<invalid-element-a></invalid-element-a>',
'Cmp2': '<invalid-element-b></invalid-element-b>'
},
}]);
const sf = getSourceFileOrError(program, fileName);
const cmp1 = getClass(sf, 'Cmp1');
const cmp2 = getClass(sf, 'Cmp2');

const diags1 = templateTypeChecker.getDiagnosticsForComponent(cmp1);
expect(diags1.length).toBe(1);
expect(diags1[0].messageText).toContain('invalid-element-a');
expect(diags1[0].messageText).not.toContain('invalid-element-b');

const diags2 = templateTypeChecker.getDiagnosticsForComponent(cmp2);
expect(diags2.length).toBe(1);
expect(diags2[0].messageText).toContain('invalid-element-b');
expect(diags2[0].messageText).not.toContain('invalid-element-a');
});
});
});

0 comments on commit 2968435

Please sign in to comment.