Skip to content

Commit be9a737

Browse files
mprobstjasonaden
authored andcommitted
fix(compiler-cli): merge @fileoverview comments. (angular#20870)
Previously, this code would unconditionally add a @fileoverview comment to generated files, and only if the contained any code at all. However often existing fileoverview comments should be copied from the file the generated file was originally based off of. This allows users to e.g. include Closure Compiler directives in their original `component.ts` file, which will then automaticallly also apply to code generated from it. This special cases `@license` comments, as Closure disregards directives in comments containing `@license`. PR Close angular#20870
1 parent 1f46949 commit be9a737

File tree

5 files changed

+157
-27
lines changed

5 files changed

+157
-27
lines changed

packages/compiler-cli/src/transformers/node_emitter.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,7 @@ export class TypeScriptNodeEmitter {
2626
...stmts.map(stmt => stmt.visitStatement(converter, null)).filter(stmt => stmt != null));
2727
const preambleStmts: ts.Statement[] = [];
2828
if (preamble) {
29-
if (preamble.startsWith('/*') && preamble.endsWith('*/')) {
30-
preamble = preamble.substr(2, preamble.length - 4);
31-
}
32-
const commentStmt = ts.createNotEmittedStatement(sourceFile);
33-
ts.setSyntheticLeadingComments(
34-
commentStmt,
35-
[{kind: ts.SyntaxKind.MultiLineCommentTrivia, text: preamble, pos: -1, end: -1}]);
36-
ts.setEmitFlags(commentStmt, ts.EmitFlags.CustomPrologue);
29+
const commentStmt = this.createCommentStatement(sourceFile, preamble);
3730
preambleStmts.push(commentStmt);
3831
}
3932
const sourceStatments =
@@ -42,6 +35,19 @@ export class TypeScriptNodeEmitter {
4235
const newSourceFile = ts.updateSourceFileNode(sourceFile, sourceStatments);
4336
return [newSourceFile, converter.getNodeMap()];
4437
}
38+
39+
/** Creates a not emitted statement containing the given comment. */
40+
createCommentStatement(sourceFile: ts.SourceFile, comment: string): ts.Statement {
41+
if (comment.startsWith('/*') && comment.endsWith('*/')) {
42+
comment = comment.substr(2, comment.length - 4);
43+
}
44+
const commentStmt = ts.createNotEmittedStatement(sourceFile);
45+
ts.setSyntheticLeadingComments(
46+
commentStmt,
47+
[{kind: ts.SyntaxKind.MultiLineCommentTrivia, text: comment, pos: -1, end: -1}]);
48+
ts.setEmitFlags(commentStmt, ts.EmitFlags.CustomPrologue);
49+
return commentStmt;
50+
}
4551
}
4652

4753
// A recorded node is a subtype of the node that is marked as being recoreded. This is used

packages/compiler-cli/src/transformers/node_emitter_transform.ts

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,68 @@ import * as ts from 'typescript';
1212
import {TypeScriptNodeEmitter} from './node_emitter';
1313
import {GENERATED_FILES} from './util';
1414

15-
const PREAMBLE = `/**
16-
* @fileoverview This file was generated by the Angular template compiler.
17-
* Do not edit.
18-
* @suppress {suspiciousCode,uselessCode,missingProperties,missingOverride,checkTypes}
19-
* tslint:disable
20-
*/`;
15+
function getPreamble(original: string) {
16+
return `/**
17+
* @fileoverview This file was generated by the Angular template compiler. Do not edit.
18+
* ${original}
19+
* @suppress {suspiciousCode,uselessCode,missingProperties,missingOverride,checkTypes}
20+
* tslint:disable
21+
*/`;
22+
}
2123

22-
export function getAngularEmitterTransformFactory(generatedFiles: Map<string, GeneratedFile>): () =>
24+
/**
25+
* Returns a transformer that does two things for generated files (ngfactory etc):
26+
* - adds a fileoverview JSDoc comment containing Closure Compiler specific "suppress"ions in JSDoc.
27+
* The new comment will contain any fileoverview comment text from the original source file this
28+
* file was generated from.
29+
* - updates generated files that are not in the given map of generatedFiles to have an empty
30+
* list of statements as their body.
31+
*/
32+
export function getAngularEmitterTransformFactory(
33+
generatedFiles: Map<string, GeneratedFile>, program: ts.Program): () =>
2334
(sourceFile: ts.SourceFile) => ts.SourceFile {
2435
return function() {
2536
const emitter = new TypeScriptNodeEmitter();
2637
return function(sourceFile: ts.SourceFile): ts.SourceFile {
2738
const g = generatedFiles.get(sourceFile.fileName);
39+
const orig = g && program.getSourceFile(g.srcFileUrl);
40+
let originalComment = '';
41+
if (orig) originalComment = getFileoverviewComment(orig);
42+
const preamble = getPreamble(originalComment);
2843
if (g && g.stmts) {
29-
const [newSourceFile] = emitter.updateSourceFile(sourceFile, g.stmts, PREAMBLE);
44+
const orig = program.getSourceFile(g.srcFileUrl);
45+
let originalComment = '';
46+
if (orig) originalComment = getFileoverviewComment(orig);
47+
const [newSourceFile] = emitter.updateSourceFile(sourceFile, g.stmts, preamble);
3048
return newSourceFile;
3149
} else if (GENERATED_FILES.test(sourceFile.fileName)) {
32-
return ts.updateSourceFileNode(sourceFile, []);
50+
// The file should be empty, but emitter.updateSourceFile would still add imports
51+
// and various minutiae.
52+
// Clear out the source file entirely, only including the preamble comment, so that
53+
// ngc produces an empty .js file.
54+
return ts.updateSourceFileNode(
55+
sourceFile, [emitter.createCommentStatement(sourceFile, preamble)]);
3356
}
3457
return sourceFile;
3558
};
3659
};
3760
}
61+
62+
/**
63+
* Parses and returns the comment text (without start and end markers) of a \@fileoverview comment
64+
* in the given source file. Returns the empty string if no such comment can be found.
65+
*/
66+
function getFileoverviewComment(sourceFile: ts.SourceFile): string {
67+
const trivia = sourceFile.getFullText().substring(0, sourceFile.getStart());
68+
const leadingComments = ts.getLeadingCommentRanges(trivia, 0);
69+
if (!leadingComments || leadingComments.length === 0) return '';
70+
const comment = leadingComments[0];
71+
if (comment.kind !== ts.SyntaxKind.MultiLineCommentTrivia) return '';
72+
// Only comments separated with a \n\n from the file contents are considered file-level comments
73+
// in TypeScript.
74+
if (sourceFile.getFullText().substring(comment.end, comment.end + 2) !== '\n\n') return '';
75+
const commentText = sourceFile.getFullText().substring(comment.pos, comment.end);
76+
// Closure Compiler ignores @suppress and similar if the comment contains @license.
77+
if (commentText.indexOf('@license') !== -1) return '';
78+
return commentText.replace(/^\/\*\*/, '').replace(/ ?\*\/$/, '');
79+
}

packages/compiler-cli/src/transformers/program.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ class AngularCompilerProgram implements Program {
399399
if (!this.options.disableExpressionLowering) {
400400
beforeTs.push(getExpressionLoweringTransformFactory(this.metadataCache, this.tsProgram));
401401
}
402-
beforeTs.push(getAngularEmitterTransformFactory(genFiles));
402+
beforeTs.push(getAngularEmitterTransformFactory(genFiles, this.getTsProgram()));
403403
if (customTransformers && customTransformers.beforeTs) {
404404
beforeTs.push(...customTransformers.beforeTs);
405405
}
@@ -628,15 +628,12 @@ class AngularCompilerProgram implements Program {
628628
// Filter out generated files for which we didn't generate code.
629629
// This can happen as the stub caclulation is not completely exact.
630630
// Note: sourceFile refers to the .ngfactory.ts / .ngsummary.ts file
631+
// node_emitter_transform already set the file contents to be empty,
632+
// so this code only needs to skip the file if !allowEmptyCodegenFiles.
631633
const isGenerated = GENERATED_FILES.test(outFileName);
632-
if (isGenerated) {
633-
if (!genFile || !genFile.stmts || genFile.stmts.length === 0) {
634-
if (this.options.allowEmptyCodegenFiles) {
635-
outData = '';
636-
} else {
637-
return;
638-
}
639-
}
634+
if (isGenerated && !this.options.allowEmptyCodegenFiles &&
635+
(!genFile || !genFile.stmts || genFile.stmts.length === 0)) {
636+
return;
640637
}
641638
if (baseFile) {
642639
sourceFiles = sourceFiles ? [...sourceFiles, baseFile] : [baseFile];

packages/compiler-cli/test/ngc_spec.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,90 @@ describe('ngc transformer command-line', () => {
249249
.toBe(true);
250250
});
251251

252+
describe('comments', () => {
253+
function compileAndRead(contents: string) {
254+
writeConfig(`{
255+
"extends": "./tsconfig-base.json",
256+
"files": ["mymodule.ts"],
257+
"angularCompilerOptions": {"allowEmptyCodegenFiles": true}
258+
}`);
259+
write('mymodule.ts', contents);
260+
261+
const exitCode = main(['-p', basePath], errorSpy);
262+
expect(exitCode).toEqual(0);
263+
264+
const modPath = path.resolve(outDir, 'mymodule.ngfactory.js');
265+
expect(fs.existsSync(modPath)).toBe(true);
266+
return fs.readFileSync(modPath, {encoding: 'UTF-8'});
267+
}
268+
269+
it('should be added', () => {
270+
const contents = compileAndRead(`
271+
import {CommonModule} from '@angular/common';
272+
import {NgModule} from '@angular/core';
273+
274+
@NgModule({
275+
imports: [CommonModule]
276+
})
277+
export class MyModule {}
278+
`);
279+
expect(contents).toContain('@fileoverview');
280+
expect(contents).toContain('generated by the Angular template compiler');
281+
expect(contents).toContain('@suppress {suspiciousCode');
282+
});
283+
284+
it('should be merged with existing fileoverview comments', () => {
285+
const contents = compileAndRead(`/** Hello world. */
286+
287+
import {CommonModule} from '@angular/common';
288+
import {NgModule} from '@angular/core';
289+
290+
@NgModule({
291+
imports: [CommonModule]
292+
})
293+
export class MyModule {}
294+
`);
295+
expect(contents).toContain('Hello world.');
296+
});
297+
298+
it('should only pick file comments', () => {
299+
const contents = compileAndRead(`
300+
/** Comment on class. */
301+
class MyClass {
302+
303+
}
304+
`);
305+
expect(contents).toContain('@fileoverview');
306+
expect(contents).not.toContain('Comment on class.');
307+
});
308+
309+
it('should not be merged with @license comments', () => {
310+
const contents = compileAndRead(`/** @license Some license. */
311+
312+
import {CommonModule} from '@angular/common';
313+
import {NgModule} from '@angular/core';
314+
315+
@NgModule({
316+
imports: [CommonModule]
317+
})
318+
export class MyModule {}
319+
`);
320+
expect(contents).toContain('@fileoverview');
321+
expect(contents).not.toContain('@license');
322+
});
323+
324+
it('should be included in empty files', () => {
325+
const contents = compileAndRead(`/** My comment. */
326+
327+
import {Inject, Injectable, Optional} from '@angular/core';
328+
329+
@Injectable()
330+
export class NotAnAngularComponent {}
331+
`);
332+
expect(contents).toContain('My comment');
333+
});
334+
});
335+
252336
it('should compile with an explicit tsconfig reference', () => {
253337
writeConfig(`{
254338
"extends": "./tsconfig-base.json",

packages/compiler-cli/test/transformers/program_spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,8 @@ describe('ng program', () => {
437437
sf => sf.fileName === path.join(testSupport.basePath, checks.originalFileName)))
438438
.toBe(true);
439439
if (checks.shouldBeEmpty) {
440-
expect(writeData !.data).toBe('');
440+
// The file should only contain comments (the preamble comment added by ngc).
441+
expect(writeData !.data).toMatch(/^(\s*\/\*([^*]|\*[^/])*\*\/\s*)?$/);
441442
} else {
442443
expect(writeData !.data).not.toBe('');
443444
}

0 commit comments

Comments
 (0)