From fd810206250903d9af1339f951af3ac63ce7bf96 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Mon, 12 Oct 2015 07:57:39 -0700 Subject: [PATCH 1/3] feat: generate stub declarations for fields. Closure Compiler interprets accesses in the constructor as field declarations. When the code has no side effects, in particular no assignment, this is called a stub declaration. This change prints such a stub declaration, including a type annotations, for each field of each class. --- src/sickle.ts | 109 +++++++++++++++++++++++++++++------ test/sickle_test.ts | 8 +++ test/test_support.ts | 2 +- test_files/fields.js | 18 ++++++ test_files/fields.ts | 13 +++++ test_files/fields_no_ctor.js | 8 +++ test_files/fields_no_ctor.ts | 3 + 7 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 test_files/fields.js create mode 100644 test_files/fields.ts create mode 100644 test_files/fields_no_ctor.js create mode 100644 test_files/fields_no_ctor.ts diff --git a/src/sickle.ts b/src/sickle.ts index 5731b3391..5daee1b64 100644 --- a/src/sickle.ts +++ b/src/sickle.ts @@ -20,6 +20,8 @@ export type StringMap = { export type AnnotatedProgram = StringMap; +const VISIBILITY_FLAGS = ts.NodeFlags.Private | ts.NodeFlags.Protected | ts.NodeFlags.Public; + /** * A source processor that takes TypeScript code and annotates the output with Closure-style JSDoc * comments. @@ -51,6 +53,8 @@ class Annotator { return res; } + private emit(str: string) { this.output.push(str); } + private visit(node: ts.Node) { // console.log('node:', (ts).SyntaxKind[node.kind]); switch (node.kind) { @@ -58,15 +62,57 @@ class Annotator { this.maybeVisitType((node).type); this.writeNode(node); break; + case ts.SyntaxKind.ClassDeclaration: { + let classNode = node; + let hasCtor = classNode.members.some((e) => e.kind === ts.SyntaxKind.Constructor); + if (hasCtor) { + this.writeNode(classNode); + break; + } + // Emit a synthetic ctor. + // TODO(martinprobst): Handle inherited parent ctors. + this.writeTextBetween(classNode, classNode.getLastToken()); + this.emit('constructor() {\n'); + this.emitStubDeclarations(classNode, []); + this.emit('}\n'); + this.writeNode(classNode.getLastToken()); + break; + } + case ts.SyntaxKind.Constructor: { + let ctor = node; + this.writeTextBetween(ctor, ctor.body); + if (ctor.body.statements.length) { + // Insert before the first code in the ctor. + this.writeTextBetween(ctor.body, ctor.body.statements[0]); + } else { + // Empty ctor - just insert before the end of it. + this.writeTextBetween(ctor.body, ctor.body.getLastToken()); + } + + let paramProps = ctor.parameters.filter((p) => !!(p.flags & VISIBILITY_FLAGS)); + this.emitStubDeclarations(ctor.parent, paramProps); + + if (ctor.body.statements.length) { + let firstStmt = ctor.body.statements[0]; + this.emit(ctor.body.getSourceFile().getText().substring(firstStmt.getFullStart(), + ctor.body.getEnd())); + } else { + let remaining = ctor.getSourceFile().getText().substring( + ctor.body.getLastToken().getFullStart(), ctor.body.getEnd()); + this.emit(remaining); + } + + break; + } case ts.SyntaxKind.FunctionDeclaration: case ts.SyntaxKind.ArrowFunction: let fnDecl = node; - this.maybeVisitType(fnDecl.type, true); + this.maybeVisitType(fnDecl.type, '@return'); let writeOffset = fnDecl.getFullStart(); // Parameters. if (fnDecl.parameters.length) { for (let param of fnDecl.parameters) { - this.writeTextBetween(fnDecl, writeOffset, param); + this.writeTextFromOffset(writeOffset, param); writeOffset = param.getEnd(); this.maybeVisitType(param.type); this.visit(param); @@ -74,12 +120,12 @@ class Annotator { } // Return type. if (fnDecl.type) { - this.writeTextBetween(fnDecl, writeOffset, fnDecl.type); + this.writeTextFromOffset(writeOffset, fnDecl.type); this.visit(fnDecl.type); writeOffset = fnDecl.type.getEnd(); } // Body. - this.writeTextBetween(fnDecl, writeOffset, fnDecl.body); + this.writeTextFromOffset(writeOffset, fnDecl.body); this.visit(fnDecl.body); break; default: @@ -88,23 +134,44 @@ class Annotator { } } - private maybeVisitType(type: ts.TypeNode, isReturn?: boolean) { + private emitStubDeclarations( + classDecl: ts.ClassLikeDeclaration, paramProps: ts.ParameterDeclaration[]) { + this.emit('\n\n// Sickle: begin stub declarations.\n'); + this.emit('\n'); + let props = ( + classDecl.members.filter((e) => e.kind === ts.SyntaxKind.PropertyDeclaration)); + props.forEach((p) => this.visitProperty(p)); + paramProps.forEach((p) => this.visitProperty(p)); + this.emit('// Sickle: end stub declarations.\n'); + } + + private visitProperty(p: ts.PropertyDeclaration | ts.ParameterDeclaration) { + this.maybeVisitType(p.type, '@type'); + this.emit('\nthis.'); + this.emit(p.name.getText()); + this.emit(';'); + this.emit('\n'); + } + + private maybeVisitType(type: ts.TypeNode, jsDocTag?: string) { if (!type) return; - this.output.push(' /**'); - if (isReturn) { - this.output.push(' @return {'); + this.emit(' /**'); + if (jsDocTag) { + this.emit(' '); + this.emit(jsDocTag); + this.emit(' {'); } this.visit(type); - if (isReturn) { - this.output.push('}'); + if (jsDocTag) { + this.emit('}'); } - this.output.push(' */'); + this.emit(' */'); } - private writeNode(node: ts.Node, upTo?: number) { + private writeNode(node: ts.Node) { if (node.getChildCount() == 0) { // Directly write complete tokens. - this.output.push(node.getFullText()); + this.emit(node.getFullText()); return; } let lastEnd = node.getFullStart(); @@ -114,17 +181,25 @@ class Annotator { this.visit(child); lastEnd = child.getEnd(); } + // Write any trailing text. + let text = node.getSourceFile().getText().slice(lastEnd, node.getEnd()); + if (text) this.emit(text); + } + + private writeTextBetween(node: ts.Node, to: ts.Node) { + let text = node.getSourceFile().getText().slice(node.getFullStart(), to.getFullStart()); + if (text) this.emit(text); } - private writeTextBetween(node: ts.Node, from: number, to: ts.Node) { - let text = node.getSourceFile().getText().slice(from, to.getFullStart()); - this.output.push(text); + private writeTextFromOffset(from: number, to: ts.Node) { + let text = to.getSourceFile().getText().slice(from, to.getFullStart()); + if (text) this.emit(text); } private writeSourceBefore(offset: number, node: ts.Node) { if (node.getFullStart() == offset) return; assert(node.getFullStart() > offset, 'Offset must not be smaller'); - this.output.push(node.getSourceFile().getText().slice(offset, node.getFullStart())); + this.emit(node.getSourceFile().getText().slice(offset, node.getFullStart())); } private fail(msg: string) { throw new Error(msg); } diff --git a/test/sickle_test.ts b/test/sickle_test.ts index 87d50d758..d41287e3e 100644 --- a/test/sickle_test.ts +++ b/test/sickle_test.ts @@ -6,8 +6,16 @@ import {expect} from 'chai'; import {annotateProgram, formatDiagnostics} from '../src/sickle'; import {expectSource, goldenTests} from './test_support'; +let RUN_TESTS_MATCHING: RegExp = null; +// RUN_TESTS_MATCHING = /fields/; + describe('golden tests', () => { + goldenTests().forEach((test) => { + if (RUN_TESTS_MATCHING && !RUN_TESTS_MATCHING.exec(test.name)) { + it.skip(test.name); + return; + } var tsSource = fs.readFileSync(test.tsPath, 'utf-8'); var jsSource = fs.readFileSync(test.jsPath, 'utf-8'); it(test.name, () => { expectSource(tsSource).to.equal(jsSource); }); diff --git a/test/test_support.ts b/test/test_support.ts index c15ae98ca..41c54027b 100644 --- a/test/test_support.ts +++ b/test/test_support.ts @@ -73,7 +73,7 @@ function transformSource(src: string): string { export function expectSource(src: string) { var annotated = annotateSource(src); - // console.log('Annotated', annotated); + // console.log('Annotated:\n', annotated); var transformed = transformSource(annotated); return expect(transformed); } diff --git a/test_files/fields.js b/test_files/fields.js new file mode 100644 index 000000000..b7f033080 --- /dev/null +++ b/test_files/fields.js @@ -0,0 +1,18 @@ +class Klass { + constructor(field3) { + // Sickle: begin stub declarations. + this.field3 = field3; + /** @type { string} */ + this.field1; + /** @type { number} */ + this.field2; + /** @type { number} */ + this.field3; + // Sickle: end stub declarations. + this.field3 = 2 + 1; + } + getF1() { + // This access print a warning without a generated field stub declaration. + return this.field1; + } +} diff --git a/test_files/fields.ts b/test_files/fields.ts new file mode 100644 index 000000000..6082e14e3 --- /dev/null +++ b/test_files/fields.ts @@ -0,0 +1,13 @@ +class Klass { + field1: string; + field2: number; + + constructor(private field3: number) { + this.field3 = 2 + 1; + } + + getF1() { + // This access print a warning without a generated field stub declaration. + return this.field1; + } +} diff --git a/test_files/fields_no_ctor.js b/test_files/fields_no_ctor.js new file mode 100644 index 000000000..e45a15eef --- /dev/null +++ b/test_files/fields_no_ctor.js @@ -0,0 +1,8 @@ +class NoCtor { + constructor() { + // Sickle: begin stub declarations. + /** @type { number} */ + this.field1; + // Sickle: end stub declarations. + } +} diff --git a/test_files/fields_no_ctor.ts b/test_files/fields_no_ctor.ts new file mode 100644 index 000000000..7e837435f --- /dev/null +++ b/test_files/fields_no_ctor.ts @@ -0,0 +1,3 @@ +class NoCtor { + field1: number; +} From 64688fac0564545568e9c4e8fed73aa5c8ed7a9d Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Sun, 1 Nov 2015 17:21:25 +0100 Subject: [PATCH 2/3] feat: test improvements - Properly print file names - Print the source that failed to parse - Check generated sources in Closure Compiler strict mode. This makes sure we don't forget to generate some field or variable. - start `gulp watch`ing without testing first. This allows to start up gulp watch with failing tests. --- gulpfile.js | 7 +++---- src/sickle.ts | 4 ++-- test/e2e_test.ts | 2 +- test/test_support.ts | 3 ++- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index cb349583a..2a19ce5fc 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -86,11 +86,10 @@ gulp.task('test.e2e', ['test.compile'], function(done) { gulp.task('test', ['test.unit', 'test.e2e', 'test.check-format']); -gulp.task('watch', ['test.unit', 'test.check-format'], function() { +gulp.task('watch', function() { failOnError = false; - // Avoid watching generated .d.ts in the build (aka output) directory. - return gulp.watch( - ['src/**/*.ts', 'test/**/*.ts', 'test_files/**'], {ignoreInitial: true}, ['test.unit']); + gulp.start(['test.unit']); // Trigger initial build. + return gulp.watch(['src/**/*.ts', 'test/**/*.ts', 'test_files/**'], ['test.unit']); }); gulp.task('default', ['compile']); diff --git a/src/sickle.ts b/src/sickle.ts index 5daee1b64..a4cb9942e 100644 --- a/src/sickle.ts +++ b/src/sickle.ts @@ -3,12 +3,12 @@ import * as ts from 'typescript'; export function formatDiagnostics(diags: ts.Diagnostic[]): string { return diags.map((d) => { let res = ts.DiagnosticCategory[d.category]; - if (d.file) res += d.file + ':'; + if (d.file) res += ' at ' + d.file.fileName + ':'; if (d.start) { let {line, character} = d.file.getLineAndCharacterOfPosition(d.start); res += line + ':' + character + ':'; } - res += d.messageText; + res += ' ' + d.messageText; return res; }) .join('\n'); diff --git a/test/e2e_test.ts b/test/e2e_test.ts index 547198ccc..8b812de50 100644 --- a/test/e2e_test.ts +++ b/test/e2e_test.ts @@ -16,7 +16,7 @@ export function checkClosureCompile(jsFiles: string[], done: (err: Error) => voi 'checks-only': true, 'jscomp_error': 'checkTypes', 'js': jsFiles, - 'language_in': 'ECMASCRIPT6' + 'language_in': 'ECMASCRIPT6_STRICT' }; compile(null, CLOSURE_COMPILER_OPTS, (err, stdout, stderr) => { diff --git a/test/test_support.ts b/test/test_support.ts index 41c54027b..6fcf42f08 100644 --- a/test/test_support.ts +++ b/test/test_support.ts @@ -58,7 +58,8 @@ function transformSource(src: string): string { var program = ts.createProgram(['main.ts'], OPTIONS, host); if (program.getSyntacticDiagnostics().length) { - throw new Error(formatDiagnostics(ts.getPreEmitDiagnostics(program))); + throw new Error('Failed to parse ' + src + '\n' + + formatDiagnostics(ts.getPreEmitDiagnostics(program))); } var transformed: StringMap = {}; From ce40815eff0ec3c7f7672ee99597f722f8801de8 Mon Sep 17 00:00:00 2001 From: Martin Probst Date: Wed, 4 Nov 2015 13:07:47 +0100 Subject: [PATCH 3/3] chore: fix indentation for clang-format 1.0.33. --- package.json | 2 +- src/sickle.ts | 5 +++-- test/test_support.ts | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 2e168c460..64c2353ba 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ }, "devDependencies": { "chai": "^2.1.1", - "clang-format": "^1.0.32", + "clang-format": "1.0.33", "closure-compiler": "^0.2.12", "gulp": "^3.8.11", "gulp-clang-format": "^1.0.22", diff --git a/src/sickle.ts b/src/sickle.ts index a4cb9942e..18b463f0b 100644 --- a/src/sickle.ts +++ b/src/sickle.ts @@ -94,8 +94,9 @@ class Annotator { if (ctor.body.statements.length) { let firstStmt = ctor.body.statements[0]; - this.emit(ctor.body.getSourceFile().getText().substring(firstStmt.getFullStart(), - ctor.body.getEnd())); + this.emit( + ctor.body.getSourceFile().getText().substring( + firstStmt.getFullStart(), ctor.body.getEnd())); } else { let remaining = ctor.getSourceFile().getText().substring( ctor.body.getLastToken().getFullStart(), ctor.body.getEnd()); diff --git a/test/test_support.ts b/test/test_support.ts index 6fcf42f08..fd54012fe 100644 --- a/test/test_support.ts +++ b/test/test_support.ts @@ -58,8 +58,8 @@ function transformSource(src: string): string { var program = ts.createProgram(['main.ts'], OPTIONS, host); if (program.getSyntacticDiagnostics().length) { - throw new Error('Failed to parse ' + src + '\n' + - formatDiagnostics(ts.getPreEmitDiagnostics(program))); + throw new Error( + 'Failed to parse ' + src + '\n' + formatDiagnostics(ts.getPreEmitDiagnostics(program))); } var transformed: StringMap = {};