Skip to content

Commit

Permalink
feat(compiler-cli): produce template diagnostics error messages (#17125)
Browse files Browse the repository at this point in the history
Refactoring the compiler to use transformers moves the code generation
after type-checking which suppresses the errors TypeScript would
generate in the user code.

`TypeChecker` currently produces the same factory code that was
generated prior the switch to transfomers, getting back the same
diagnostics as before. The refactoring will allow the code to
diverge from the factory code and allow better diagnostic error
messages than was previously possible by type-checking the factories.
  • Loading branch information
chuckjaz authored and vicb committed Jun 1, 2017
1 parent 1338995 commit 230255f
Show file tree
Hide file tree
Showing 6 changed files with 398 additions and 7 deletions.
225 changes: 225 additions & 0 deletions packages/compiler-cli/src/diagnostics/check_types.ts
@@ -0,0 +1,225 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {AotCompiler, AotCompilerHost, AotCompilerOptions, EmitterVisitorContext, GeneratedFile, NgAnalyzedModules, ParseSourceSpan, Statement, StaticReflector, TypeScriptEmitter, createAotCompiler} from '@angular/compiler';
import * as ts from 'typescript';

interface FactoryInfo {
source: ts.SourceFile;
context: EmitterVisitorContext;
}

type FactoryInfoMap = Map<string, FactoryInfo>;

export enum DiagnosticKind {
Message,
Warning,
Error,
}

export interface Diagnostic {
message: string;
kind: DiagnosticKind;
span: ParseSourceSpan;
}

export class TypeChecker {
private _aotCompiler: AotCompiler|undefined;
private _reflector: StaticReflector|undefined;
private _factories: Map<string, FactoryInfo>|undefined;
private _factoryNames: string[]|undefined;
private _diagnosticProgram: ts.Program|undefined;
private _diagnosticsByFile: Map<string, Diagnostic[]>|undefined;

constructor(
private program: ts.Program, private tsOptions: ts.CompilerOptions,
private compilerHost: ts.CompilerHost, private aotCompilerHost: AotCompilerHost,
private aotOptions: AotCompilerOptions, private _analyzedModules?: NgAnalyzedModules,
private _generatedFiles?: GeneratedFile[]) {}

getDiagnostics(fileName?: string): Diagnostic[] {
return fileName ?
this.diagnosticsByFileName.get(fileName) || [] :
([] as Diagnostic[]).concat(...Array.from(this.diagnosticsByFileName.values()));
}

private get analyzedModules(): NgAnalyzedModules {
return this._analyzedModules || (this._analyzedModules = this.aotCompiler.analyzeModulesSync(
this.program.getSourceFiles().map(sf => sf.fileName)));
}

private get diagnosticsByFileName(): Map<string, Diagnostic[]> {
return this._diagnosticsByFile || this.createDiagnosticsByFile();
}

private get diagnosticProgram(): ts.Program {
return this._diagnosticProgram || this.createDiagnosticProgram();
}

private get generatedFiles(): GeneratedFile[] {
let result = this._generatedFiles;
if (!result) {
this._generatedFiles = result = this.aotCompiler.emitAllImpls(this.analyzedModules);
}
return result;
}

private get aotCompiler(): AotCompiler {
return this._aotCompiler || this.createCompilerAndReflector();
}

private get reflector(): StaticReflector {
let result = this._reflector;
if (!result) {
this.createCompilerAndReflector();
result = this._reflector !;
}
return result;
}

private get factories(): Map<string, FactoryInfo> {
return this._factories || this.createFactories();
}

private get factoryNames(): string[] {
return this._factoryNames || (this.createFactories() && this._factoryNames !);
}

private createCompilerAndReflector() {
const {compiler, reflector} = createAotCompiler(this.aotCompilerHost, this.aotOptions);
this._reflector = reflector;
return this._aotCompiler = compiler;
}

private createDiagnosticProgram() {
// Create a program that is all the files from the original program plus the factories.
const existingFiles = this.program.getSourceFiles().map(source => source.fileName);
const host = new TypeCheckingHost(this.compilerHost, this.program, this.factories);
return this._diagnosticProgram =
ts.createProgram([...existingFiles, ...this.factoryNames], this.tsOptions, host);
}

private createFactories() {
// Create all the factory files with enough information to map the diagnostics reported for the
// created file back to the original source.
const emitter = new TypeScriptEmitter();
const factorySources =
this.generatedFiles.filter(file => file.stmts != null && file.stmts.length)
.map<[string, FactoryInfo]>(
file => [file.genFileUrl, createFactoryInfo(emitter, file)]);
this._factories = new Map(factorySources);
this._factoryNames = Array.from(this._factories.keys());
return this._factories;
}

private createDiagnosticsByFile() {
// Collect all the diagnostics binned by original source file name.
const result = new Map<string, Diagnostic[]>();
const diagnosticsFor = (fileName: string) => {
let r = result.get(fileName);
if (!r) {
r = [];
result.set(fileName, r);
}
return r;
};
const program = this.diagnosticProgram;
for (const factoryName of this.factoryNames) {
const sourceFile = program.getSourceFile(factoryName);
for (const diagnostic of this.diagnosticProgram.getSemanticDiagnostics(sourceFile)) {
const span = this.sourceSpanOf(diagnostic.file, diagnostic.start, diagnostic.length);
if (span) {
const fileName = span.start.file.url;
const diagnosticsList = diagnosticsFor(fileName);
diagnosticsList.push({
message: diagnosticMessageToString(diagnostic.messageText),
kind: diagnosticKindConverter(diagnostic.category), span
});
}
}
}
return result;
}

private sourceSpanOf(source: ts.SourceFile, start: number, length: number): ParseSourceSpan|null {
// Find the corresponding TypeScript node
const info = this.factories.get(source.fileName);
if (info) {
const {line, character} = ts.getLineAndCharacterOfPosition(source, start);
return info.context.spanOf(line, character);
}
return null;
}
}

function diagnosticMessageToString(message: ts.DiagnosticMessageChain | string): string {
return typeof message === 'string' ? message : diagnosticMessageToString(message.messageText);
}

function diagnosticKindConverter(kind: ts.DiagnosticCategory) {
// The diagnostics kind matches ts.DiagnosticCategory. Review this code if this changes.
return kind as any as DiagnosticKind;
}

function createFactoryInfo(emitter: TypeScriptEmitter, file: GeneratedFile): FactoryInfo {
const {sourceText, context} =
emitter.emitStatementsAndContext(file.srcFileUrl, file.genFileUrl, file.stmts !);
const source = ts.createSourceFile(
file.genFileUrl, sourceText, ts.ScriptTarget.Latest, /* setParentNodes */ true);
return {source, context};
}

class TypeCheckingHost implements ts.CompilerHost {
constructor(
private host: ts.CompilerHost, private originalProgram: ts.Program,
private factories: Map<string, FactoryInfo>) {}

getSourceFile(
fileName: string, languageVersion: ts.ScriptTarget,
onError?: ((message: string) => void)): ts.SourceFile {
const originalSource = this.originalProgram.getSourceFile(fileName);
if (originalSource) {
return originalSource;
}
const factoryInfo = this.factories.get(fileName);
if (factoryInfo) {
return factoryInfo.source;
}
return this.host.getSourceFile(fileName, languageVersion, onError);
}

getDefaultLibFileName(options: ts.CompilerOptions): string {
return this.host.getDefaultLibFileName(options);
}

writeFile: ts.WriteFileCallback =
() => { throw new Error('Unexpected write in diagnostic program'); }

getCurrentDirectory(): string {
return this.host.getCurrentDirectory();
}

getDirectories(path: string): string[] { return this.host.getDirectories(path); }

getCanonicalFileName(fileName: string): string {
return this.host.getCanonicalFileName(fileName);
}

useCaseSensitiveFileNames(): boolean { return this.host.useCaseSensitiveFileNames(); }

getNewLine(): string { return this.host.getNewLine(); }

fileExists(fileName: string): boolean {
return this.factories.has(fileName) || this.host.fileExists(fileName);
}

readFile(fileName: string): string {
const factoryInfo = this.factories.get(fileName);
return (factoryInfo && factoryInfo.source.text) || this.host.readFile(fileName);
}
}
140 changes: 140 additions & 0 deletions packages/compiler-cli/test/diagnostics/check_types_spec.ts
@@ -0,0 +1,140 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {AotCompilerOptions, createAotCompiler} from '@angular/compiler';
import {EmittingCompilerHost, MockAotCompilerHost, MockCompilerHost, MockData, MockDirectory, MockMetadataBundlerHost, arrayToMockDir, arrayToMockMap, isSource, settings, setup, toMockFileArray} from '@angular/compiler/test/aot/test_util';
import * as ts from 'typescript';

import {Diagnostic, TypeChecker} from '../../src/diagnostics/check_types';

function compile(
rootDirs: MockData, options: AotCompilerOptions = {},
tsOptions: ts.CompilerOptions = {}): Diagnostic[] {
const rootDirArr = toMockFileArray(rootDirs);
const scriptNames = rootDirArr.map(entry => entry.fileName).filter(isSource);
const host = new MockCompilerHost(scriptNames, arrayToMockDir(rootDirArr));
const aotHost = new MockAotCompilerHost(host);
const tsSettings = {...settings, ...tsOptions};
const program = ts.createProgram(host.scriptNames.slice(0), tsSettings, host);
const ngChecker = new TypeChecker(program, tsSettings, host, aotHost, options);
return ngChecker.getDiagnostics();
}

fdescribe('ng type checker', () => {
let angularFiles = setup();

function accept(...files: MockDirectory[]) {
expectNoDiagnostics(compile([angularFiles, QUICKSTART, ...files]));
}

function reject(message: string | RegExp, ...files: MockDirectory[]) {
const diagnostics = compile([angularFiles, QUICKSTART, ...files]);
if (!diagnostics || !diagnostics.length) {
throw new Error('Expected a diagnostic erorr message');
} else {
const matches: (d: Diagnostic) => boolean =
typeof message === 'string' ? d => d.message == message : d => message.test(d.message);
const matchingDiagnostics = diagnostics.filter(matches);
if (!matchingDiagnostics || !matchingDiagnostics.length) {
throw new Error(
`Expected a diagnostics matching ${message}, received\n ${diagnostics.map(d => d.message).join('\n ')}`);
}
}
}

it('should accept unmodified QuickStart', () => { accept(); });

describe('with modified quickstart', () => {
function a(template: string) {
accept({quickstart: {app: {'app.component.ts': appComponentSource(template)}}});
}

function r(template: string, message: string | RegExp) {
reject(message, {quickstart: {app: {'app.component.ts': appComponentSource(template)}}});
}

it('should report an invalid field access',
() => { r('{{fame}}', `Property 'fame' does not exist on type 'AppComponent'.`); });
it('should reject a reference to a field of a nullable',
() => { r('{{maybePerson.name}}', `Object is possibly 'undefined'.`); });
it('should accept a reference to a field of a nullable using using non-null-assert',
() => { a('{{maybePerson!.name}}'); });
it('should accept a safe property access of a nullable person',
() => { a('{{maybePerson?.name}}'); });
it('should accept a function call', () => { a('{{getName()}}'); });
it('should reject an invalid method',
() => { r('{{getFame()}}', `Property 'getFame' does not exist on type 'AppComponent'.`); });
it('should accept a field access of a method result', () => { a('{{getPerson().name}}'); });
it('should reject an invalid field reference of a method result',
() => { r('{{getPerson().fame}}', `Property 'fame' does not exist on type 'Person'.`); });
it('should reject an access to a nullable field of a method result',
() => { r('{{getMaybePerson().name}}', `Object is possibly 'undefined'.`); });
it('should accept a nullable assert of a nullable field refernces of a method result',
() => { a('{{getMaybePerson()!.name}}'); });
it('should accept a safe property access of a nullable field reference of a method result',
() => { a('{{getMaybePerson()?.name}}'); });
});
});

function appComponentSource(template: string): string {
return `
import {Component} from '@angular/core';
export interface Person {
name: string;
address: Address;
}
export interface Address {
street: string;
city: string;
state: string;
zip: string;
}
@Component({
template: '${template}'
})
export class AppComponent {
name = 'Angular';
person: Person;
people: Person[];
maybePerson?: Person;
getName(): string { return this.name; }
getPerson(): Person { return this.person; }
getMaybePerson(): Person | undefined { this.maybePerson; }
}
`;
}

const QUICKSTART: MockDirectory = {
quickstart: {
app: {
'app.component.ts': appComponentSource('<h1>Hello {{name}}</h1>'),
'app.module.ts': `
import { NgModule } from '@angular/core';
import { toString } from './utils';
import { AppComponent } from './app.component';
@NgModule({
declarations: [ AppComponent ],
bootstrap: [ AppComponent ]
})
export class AppModule { }
`
}
}
};

function expectNoDiagnostics(diagnostics: Diagnostic[]) {
if (diagnostics && diagnostics.length) {
throw new Error(diagnostics.map(d => `${d.span}: ${d.message}`).join('\n'));
}
}
1 change: 1 addition & 0 deletions packages/compiler/src/compiler.ts
Expand Up @@ -61,6 +61,7 @@ export * from './ml_parser/interpolation_config';
export * from './ml_parser/tags';
export {NgModuleCompiler} from './ng_module_compiler';
export {AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinMethod, BuiltinVar, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, ExpressionStatement, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, StatementVisitor, ThrowStmt, TryCatchStmt, WriteKeyExpr, WritePropExpr, WriteVarExpr, StmtModifier, Statement} from './output/output_ast';
export {EmitterVisitorContext} from './output/abstract_emitter';
export * from './output/ts_emitter';
export * from './parse_util';
export * from './schema/dom_element_schema_registry';
Expand Down

0 comments on commit 230255f

Please sign in to comment.