refactor(compiler): produce more dense generated code (#16666)

This changes the formatting to be less verbose but still
tries to be readable.
This commit is contained in:
Tobias Bosch 2017-05-11 10:28:48 -07:00 committed by Jason Aden
parent ce1d7c4a6e
commit 5afaa39e68
7 changed files with 108 additions and 72 deletions

View File

@ -24,6 +24,7 @@ export abstract class OutputEmitter {
}
class _EmittedLine {
partsLength = 0;
parts: string[] = [];
srcSpans: (ParseSourceSpan|null)[] = [];
constructor(public indent: number) {}
@ -51,9 +52,14 @@ export class EmitterVisitorContext {
lineIsEmpty(): boolean { return this._currentLine.parts.length === 0; }
lineLength(): number {
return this._currentLine.indent * _INDENT_WITH.length + this._currentLine.partsLength;
}
print(from: {sourceSpan: ParseSourceSpan | null}|null, part: string, newLine: boolean = false) {
if (part.length > 0) {
this._currentLine.parts.push(part);
this._currentLine.partsLength += part.length;
this._currentLine.srcSpans.push(from && from.sourceSpan || null);
}
if (newLine) {
@ -69,12 +75,16 @@ export class EmitterVisitorContext {
incIndent() {
this._indent++;
this._currentLine.indent = this._indent;
if (this.lineIsEmpty()) {
this._currentLine.indent = this._indent;
}
}
decIndent() {
this._indent--;
this._currentLine.indent = this._indent;
if (this.lineIsEmpty()) {
this._currentLine.indent = this._indent;
}
}
pushClass(clazz: o.ClassStmt) { this._classes.push(clazz); }
@ -424,24 +434,18 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex
return null;
}
visitLiteralArrayExpr(ast: o.LiteralArrayExpr, ctx: EmitterVisitorContext): any {
const useNewLine = ast.entries.length > 1;
ctx.print(ast, `[`, useNewLine);
ctx.incIndent();
this.visitAllExpressions(ast.entries, ctx, ',', useNewLine);
ctx.decIndent();
ctx.print(ast, `]`, useNewLine);
ctx.print(ast, `[`);
this.visitAllExpressions(ast.entries, ctx, ',');
ctx.print(ast, `]`);
return null;
}
visitLiteralMapExpr(ast: o.LiteralMapExpr, ctx: EmitterVisitorContext): any {
const useNewLine = ast.entries.length > 1;
ctx.print(ast, `{`, useNewLine);
ctx.incIndent();
ctx.print(ast, `{`);
this.visitAllObjects(entry => {
ctx.print(ast, `${escapeIdentifier(entry.key, this._escapeDollarInStrings, entry.quoted)}: `);
ctx.print(ast, `${escapeIdentifier(entry.key, this._escapeDollarInStrings, entry.quoted)}:`);
entry.value.visitExpression(this, ctx);
}, ast.entries, ctx, ',', useNewLine);
ctx.decIndent();
ctx.print(ast, `}`, useNewLine);
}, ast.entries, ctx, ',');
ctx.print(ast, `}`);
return null;
}
visitCommaExpr(ast: o.CommaExpr, ctx: EmitterVisitorContext): any {
@ -450,24 +454,35 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex
ctx.print(ast, ')');
return null;
}
visitAllExpressions(
expressions: o.Expression[], ctx: EmitterVisitorContext, separator: string,
newLine: boolean = false): void {
this.visitAllObjects(
expr => expr.visitExpression(this, ctx), expressions, ctx, separator, newLine);
visitAllExpressions(expressions: o.Expression[], ctx: EmitterVisitorContext, separator: string):
void {
this.visitAllObjects(expr => expr.visitExpression(this, ctx), expressions, ctx, separator);
}
visitAllObjects<T>(
handler: (t: T) => void, expressions: T[], ctx: EmitterVisitorContext, separator: string,
newLine: boolean = false): void {
handler: (t: T) => void, expressions: T[], ctx: EmitterVisitorContext,
separator: string): void {
let incrementedIndent = false;
for (let i = 0; i < expressions.length; i++) {
if (i > 0) {
ctx.print(null, separator, newLine);
if (ctx.lineLength() > 80) {
ctx.print(null, separator, true);
if (!incrementedIndent) {
// continuation are marked with double indent.
ctx.incIndent();
ctx.incIndent();
incrementedIndent = true;
}
} else {
ctx.print(null, separator, false);
}
}
handler(expressions[i]);
}
if (newLine) {
ctx.println();
if (incrementedIndent) {
// continuation are marked with double indent.
ctx.decIndent();
ctx.decIndent();
}
}

View File

@ -62,7 +62,7 @@ class JsEmitterVisitor extends AbstractJsEmitterVisitor {
if (filePath != this._genFilePath) {
let prefix = this.importsWithPrefixes.get(filePath);
if (prefix == null) {
prefix = `import${this.importsWithPrefixes.size}`;
prefix = `i${this.importsWithPrefixes.size}`;
this.importsWithPrefixes.set(filePath, prefix);
}
ctx.print(ast, `${prefix}.`);

View File

@ -398,7 +398,7 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
if (filePath != this._genFilePath) {
let prefix = this.importsWithPrefixes.get(filePath);
if (prefix == null) {
prefix = `import${this.importsWithPrefixes.size}`;
prefix = `i${this.importsWithPrefixes.size}`;
this.importsWithPrefixes.set(filePath, prefix);
}
ctx.print(null, `${prefix}.`);

View File

@ -412,7 +412,7 @@ describe('compiler (unbundled Angular)', () => {
const mainNgFactory = genFiles.find(gf => gf.srcFileUrl === '/app/main.ts');
const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy;
expect(mainNgFactory.source)
.toContain(`${flags},(null as any),0,import1.Extends,[import2.AParam]`);
.toContain(`${flags},(null as any),0,i1.Extends,[i2.AParam]`);
});
}));
@ -464,7 +464,7 @@ describe('compiler (unbundled Angular)', () => {
const mainNgFactory = genFiles.find(gf => gf.srcFileUrl === '/app/main.ts');
const flags = NodeFlags.TypeDirective | NodeFlags.Component | NodeFlags.OnDestroy;
expect(mainNgFactory.source)
.toContain(`${flags},(null as any),0,import1.Extends,[import2.AParam_2]`);
.toContain(`${flags},(null as any),0,i1.Extends,[i2.AParam_2]`);
});
}));

View File

@ -44,11 +44,11 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toContain(`import * as import0 from '/app/app.module'`);
expect(genFile.source).toContain(`import * as i0 from '/app/app.module'`);
expect(genFile.source).toContain('export function MyServiceNgSummary()');
// Note: CompileSummaryKind.Injectable = 3
expect(genFile.source).toMatch(/summaryKind: 3,\s*type: \{\s*reference: import0.MyService/);
expect(genFile.source).toContain('token: {identifier: {reference: import0.Dep}}');
expect(genFile.source).toMatch(/summaryKind:3,\s*type:\{\s*reference:i0.MyService/);
expect(genFile.source).toContain('token:{identifier:{reference:i0.Dep}}');
}));
it('should create @Pipe summaries', fakeAsync(() => {
@ -72,11 +72,11 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toContain(`import * as import0 from '/app/app.module'`);
expect(genFile.source).toContain(`import * as i0 from '/app/app.module'`);
expect(genFile.source).toContain('export function MyPipeNgSummary()');
// Note: CompileSummaryKind.Pipe = 1
expect(genFile.source).toMatch(/summaryKind: 0,\s*type: \{\s*reference: import0.MyPipe/);
expect(genFile.source).toContain('token: {identifier: {reference: import0.Dep}}');
expect(genFile.source).toMatch(/summaryKind:0,\s*type:\{\s*reference:i0.MyPipe/);
expect(genFile.source).toContain('token:{identifier:{reference:i0.Dep}}');
}));
it('should create @Directive summaries', fakeAsync(() => {
@ -100,12 +100,11 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toContain(`import * as import0 from '/app/app.module'`);
expect(genFile.source).toContain(`import * as i0 from '/app/app.module'`);
expect(genFile.source).toContain('export function MyDirectiveNgSummary()');
// Note: CompileSummaryKind.Directive = 1
expect(genFile.source)
.toMatch(/summaryKind: 1,\s*type: \{\s*reference: import0.MyDirective/);
expect(genFile.source).toContain('token: {identifier: {reference: import0.Dep}}');
expect(genFile.source).toMatch(/summaryKind:1,\s*type:\{\s*reference:i0.MyDirective/);
expect(genFile.source).toContain('token:{identifier:{reference:i0.Dep}}');
}));
it('should create @NgModule summaries', fakeAsync(() => {
@ -126,11 +125,11 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toContain(`import * as import0 from '/app/app.module'`);
expect(genFile.source).toContain(`import * as i0 from '/app/app.module'`);
expect(genFile.source).toContain('export function MyModuleNgSummary()');
// Note: CompileSummaryKind.NgModule = 2
expect(genFile.source).toMatch(/summaryKind: 2,\s*type: \{\s*reference: import0.MyModule/);
expect(genFile.source).toContain('token: {identifier: {reference: import0.Dep}}');
expect(genFile.source).toMatch(/summaryKind:2,\s*type:\{\s*reference:i0.MyModule/);
expect(genFile.source).toContain('token:{identifier:{reference:i0.Dep}}');
}));
it('should embed useClass provider summaries in @Directive summaries', fakeAsync(() => {
@ -164,10 +163,10 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toMatch(/useClass: \{\s*reference: import1.MyService/);
expect(genFile.source).toMatch(/useClass:\{\s*reference:i1.MyService/);
// Note: CompileSummaryKind.Injectable = 3
expect(genFile.source).toMatch(/summaryKind: 3,\s*type: \{\s*reference: import1.MyService/);
expect(genFile.source).toContain('token: {identifier: {reference: import1.Dep}}');
expect(genFile.source).toMatch(/summaryKind:3,\s*type:\{\s*reference:i1.MyService/);
expect(genFile.source).toContain('token:{identifier:{reference:i1.Dep}}');
}));
it('should embed useClass provider summaries into @NgModule summaries', fakeAsync(() => {
@ -197,10 +196,10 @@ describe('aot summaries for jit', () => {
const genFile =
compileApp(rootDir).genFiles.find(f => f.genFileUrl === '/app/app.module.ngsummary.ts');
expect(genFile.source).toMatch(/useClass: \{\s*reference: import1.MyService/);
expect(genFile.source).toMatch(/useClass:\{\s*reference:i1.MyService/);
// Note: CompileSummaryKind.Injectable = 3
expect(genFile.source).toMatch(/summaryKind: 3,\s*type: \{\s*reference: import1.MyService/);
expect(genFile.source).toContain('token: {identifier: {reference: import1.Dep}}');
expect(genFile.source).toMatch(/summaryKind:3,\s*type:\{\s*reference:i1.MyService/);
expect(genFile.source).toContain('token:{identifier:{reference:i1.Dep}}');
}));
it('should reference declared @Directive and @Pipe summaries in @NgModule summaries',
@ -327,13 +326,12 @@ describe('aot summaries for jit', () => {
lib3Gen.find(f => f.genFileUrl === '/lib3/reexport.ngsummary.ts');
// ngsummary.ts files should use the reexported values from direct and deep deps
expect(lib3ModuleNgSummary.source).toContain(`import * as i4 from '/lib2/module.ngsummary'`);
expect(lib3ModuleNgSummary.source)
.toContain(`import * as import4 from '/lib2/module.ngsummary'`);
expect(lib3ModuleNgSummary.source)
.toContain(`import * as import5 from '/lib2/reexport.ngsummary'`);
.toContain(`import * as i5 from '/lib2/reexport.ngsummary'`);
expect(lib3ModuleNgSummary.source)
.toMatch(
/export function Lib3ModuleNgSummary()[^;]*,\s*import4.Lib1Module_1NgSummary,\s*import4.Lib2ModuleNgSummary,\s*import5.ReexportModule_2NgSummary\s*\]\s*;/);
/export function Lib3ModuleNgSummary()[^;]*,\s*i4.Lib1Module_1NgSummary,\s*i4.Lib2ModuleNgSummary,\s*i5.ReexportModule_2NgSummary\s*\]\s*;/);
// ngsummaries should add reexports for imported NgModules from a deep dependency
expect(lib3ModuleNgSummary.source)

View File

@ -115,7 +115,19 @@ export function main() {
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey: 1};`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`);
});
it('should break expressions into multiple lines if they are too long', () => {
const values: o.Expression[] = new Array(100);
values.fill(o.literal(1));
values.splice(50, 0, o.fn([], [new o.ReturnStatement(o.literal(1))]));
expect(emitStmt(o.variable('fn').callFn(values).toStmt())).toEqual([
'fn(1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,',
' 1,1,1,1,1,1,1,1,1,1,function() {', ' return 1;',
' },1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,',
' 1,1,1,1,1,1,1,1,1,1,1,1);'
].join('\n'));
});
it('should support blank literals', () => {
@ -126,9 +138,9 @@ export function main() {
it('should support external identifiers', () => {
expect(emitStmt(o.importExpr(sameModuleIdentifier).toStmt())).toEqual('someLocalId;');
expect(emitStmt(o.importExpr(externalModuleIdentifier).toStmt())).toEqual([
`var import0 = re` +
`var i0 = re` +
`quire('somePackage/someOtherPath');`,
`import0.someExternalId;`
`i0.someExternalId;`
].join('\n'));
});
@ -136,9 +148,9 @@ export function main() {
spyOn(importResolver, 'getImportAs')
.and.returnValue(new StaticSymbol('somePackage/importAsModule', 'importAsName', []));
expect(emitStmt(o.importExpr(externalModuleIdentifier).toStmt())).toEqual([
`var import0 = re` +
`var i0 = re` +
`quire('somePackage/importAsModule');`,
`import0.importAsName;`
`i0.importAsName;`
].join('\n'));
});

View File

@ -80,8 +80,7 @@ export function main() {
it('should create no reexport if the variable is not exported', () => {
expect(emitStmt(someVar.set(o.importExpr(externalModuleIdentifier)).toDeclStmt())).toEqual([
`import * as import0 from 'somePackage/someOtherPath';`,
`var someVar:any = import0.someExternalId;`
`import * as i0 from 'somePackage/someOtherPath';`, `var someVar:any = i0.someExternalId;`
].join('\n'));
});
@ -90,8 +89,8 @@ export function main() {
someVar.set(o.importExpr(externalModuleIdentifier)).toDeclStmt(o.DYNAMIC_TYPE),
['someVar']))
.toEqual([
`import * as import0 from 'somePackage/someOtherPath';`,
`export var someVar:any = import0.someExternalId;`
`import * as i0 from 'somePackage/someOtherPath';`,
`export var someVar:any = i0.someExternalId;`
].join('\n'));
});
@ -103,8 +102,8 @@ export function main() {
someVar.set(o.importExpr(externalModuleIdentifierWithMembers)).toDeclStmt(),
['someVar']))
.toEqual([
`import * as import0 from 'somePackage/someOtherPath';`,
`export var someVar:any = import0.someExternalId.a;`
`import * as i0 from 'somePackage/someOtherPath';`,
`export var someVar:any = i0.someExternalId.a;`
].join('\n'));
});
@ -195,7 +194,19 @@ export function main() {
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey: 1};`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`);
});
it('should break expressions into multiple lines if they are too long', () => {
const values: o.Expression[] = new Array(100);
values.fill(o.literal(1));
values.splice(50, 0, o.fn([], [new o.ReturnStatement(o.literal(1))]));
expect(emitStmt(o.variable('fn').callFn(values).toStmt())).toEqual([
'fn(1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,',
' 1,1,1,1,1,1,1,1,1,1,():void => {', ' return 1;',
' },1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,',
' 1,1,1,1,1,1,1,1,1,1,1,1);'
].join('\n'));
});
it('should apply quotes to each entry within a map produced with literalMap when true', () => {
@ -215,7 +226,7 @@ export function main() {
it('should support external identifiers', () => {
expect(emitStmt(o.importExpr(sameModuleIdentifier).toStmt())).toEqual('someLocalId;');
expect(emitStmt(o.importExpr(externalModuleIdentifier).toStmt())).toEqual([
`import * as import0 from 'somePackage/someOtherPath';`, `import0.someExternalId;`
`import * as i0 from 'somePackage/someOtherPath';`, `i0.someExternalId;`
].join('\n'));
});
@ -223,7 +234,7 @@ export function main() {
spyOn(importResolver, 'getImportAs')
.and.returnValue(new StaticSymbol('somePackage/importAsModule', 'importAsName', []));
expect(emitStmt(o.importExpr(externalModuleIdentifier).toStmt())).toEqual([
`import * as import0 from 'somePackage/importAsModule';`, `import0.importAsName;`
`import * as i0 from 'somePackage/importAsModule';`, `i0.importAsName;`
].join('\n'));
});
@ -423,8 +434,8 @@ export function main() {
expect(emitStmt(writeVarExpr.toDeclStmt(o.importType(sameModuleIdentifier))))
.toEqual('var a:someLocalId = (null as any);');
expect(emitStmt(writeVarExpr.toDeclStmt(o.importType(externalModuleIdentifier)))).toEqual([
`import * as import0 from 'somePackage/someOtherPath';`,
`var a:import0.someExternalId = (null as any);`
`import * as i0 from 'somePackage/someOtherPath';`,
`var a:i0.someExternalId = (null as any);`
].join('\n'));
});
@ -433,8 +444,8 @@ export function main() {
.and.returnValue(new StaticSymbol('somePackage/importAsModule', 'importAsName', []));
const writeVarExpr = o.variable('a').set(o.NULL_EXPR);
expect(emitStmt(writeVarExpr.toDeclStmt(o.importType(externalModuleIdentifier)))).toEqual([
`import * as import0 from 'somePackage/importAsModule';`,
`var a:import0.importAsName = (null as any);`
`import * as i0 from 'somePackage/importAsModule';`,
`var a:i0.importAsName = (null as any);`
].join('\n'));
});
@ -449,8 +460,8 @@ export function main() {
.set(o.NULL_EXPR)
.toDeclStmt(o.importType(externalModuleIdentifier, [o.STRING_TYPE]))))
.toEqual([
`import * as import0 from 'somePackage/someOtherPath';`,
`var a:import0.someExternalId<string> = (null as any);`
`import * as i0 from 'somePackage/someOtherPath';`,
`var a:i0.someExternalId<string> = (null as any);`
].join('\n'));
});