Skip to content

Commit

Permalink
fix(ivy): restore @fileoverview annotations for Closure (#28723)
Browse files Browse the repository at this point in the history
Prior to this change, the @fileoverview annotations added by users in source files or by tsickle during compilation might have change a location due to the fact that Ngtsc may prepend extra imports or constants. As a result, the output file is considered invalid by Closure (misplaced @fileoverview annotation). In order to resolve the problem we relocate @fileoverview annotation if we detect that its host node shifted.

PR Close #28723
  • Loading branch information
AndrewKushnir authored and IgorMinar committed Feb 21, 2019
1 parent 58436fd commit be121bb
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 9 deletions.
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/ngtsc/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,9 @@ export class NgtscProgram implements api.Program {
};

const customTransforms = opts && opts.customTransformers;
const beforeTransforms =
[ivyTransformFactory(compilation, this.reflector, this.importRewriter, this.isCore)];
const beforeTransforms = [ivyTransformFactory(
compilation, this.reflector, this.importRewriter, this.isCore,
this.closureCompilerEnabled)];
const afterDeclarationsTransforms = [declarationTransformFactory(compilation)];

if (this.factoryToSourceInfo !== null) {
Expand Down
67 changes: 62 additions & 5 deletions packages/compiler-cli/src/ngtsc/transform/src/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,24 @@ import {addImports} from './utils';

const NO_DECORATORS = new Set<ts.Decorator>();

const CLOSURE_FILE_OVERVIEW_REGEXP = /\s+@fileoverview\s+/i;

/**
* Metadata to support @fileoverview blocks (Closure annotations) extracting/restoring.
*/
interface FileOverviewMeta {
comments: ts.SynthesizedComment[];
host: ts.Statement;
trailing: boolean;
}

export function ivyTransformFactory(
compilation: IvyCompilation, reflector: ReflectionHost, importRewriter: ImportRewriter,
isCore: boolean): ts.TransformerFactory<ts.SourceFile> {
isCore: boolean, isClosureCompilerEnabled: boolean): ts.TransformerFactory<ts.SourceFile> {
return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
return (file: ts.SourceFile): ts.SourceFile => {
return transformIvySourceFile(compilation, context, reflector, importRewriter, isCore, file);
return transformIvySourceFile(
compilation, context, reflector, importRewriter, file, isCore, isClosureCompilerEnabled);
};
};
}
Expand Down Expand Up @@ -189,20 +201,65 @@ class IvyVisitor extends Visitor {
*/
function transformIvySourceFile(
compilation: IvyCompilation, context: ts.TransformationContext, reflector: ReflectionHost,
importRewriter: ImportRewriter, isCore: boolean, file: ts.SourceFile): ts.SourceFile {
importRewriter: ImportRewriter, file: ts.SourceFile, isCore: boolean,
isClosureCompilerEnabled: boolean): ts.SourceFile {
const constantPool = new ConstantPool();
const importManager = new ImportManager(importRewriter);

// Recursively scan through the AST and perform any updates requested by the IvyCompilation.
const visitor = new IvyVisitor(compilation, reflector, importManager, isCore, constantPool);
const sf = visit(file, visitor, context);
let sf = visit(file, visitor, context);

// Generate the constant statements first, as they may involve adding additional imports
// to the ImportManager.
const constants = constantPool.statements.map(stmt => translateStatement(stmt, importManager));

// Preserve @fileoverview comments required by Closure, since the location might change as a
// result of adding extra imports and constant pool statements.
const fileOverviewMeta = isClosureCompilerEnabled ? getFileOverviewComment(sf.statements) : null;

// Add new imports for this file.
return addImports(importManager, sf, constants);
sf = addImports(importManager, sf, constants);

if (fileOverviewMeta !== null) {
setFileOverviewComment(sf, fileOverviewMeta);
}

return sf;
}

function getFileOverviewComment(statements: ts.NodeArray<ts.Statement>): FileOverviewMeta|null {
if (statements.length > 0) {
const host = statements[0];
let trailing = false;
let comments = ts.getSyntheticLeadingComments(host);
// If @fileoverview tag is not found in source file, tsickle produces fake node with trailing
// comment and inject it at the very beginning of the generated file. So we need to check for
// leading as well as trailing comments.
if (!comments || comments.length === 0) {
trailing = true;
comments = ts.getSyntheticTrailingComments(host);
}
if (comments && comments.length > 0 && CLOSURE_FILE_OVERVIEW_REGEXP.test(comments[0].text)) {
return {comments, host, trailing};
}
}
return null;
}

function setFileOverviewComment(sf: ts.SourceFile, fileoverview: FileOverviewMeta): void {
const {comments, host, trailing} = fileoverview;
// If host statement is no longer the first one, it means that extra statements were added at the
// very beginning, so we need to relocate @fileoverview comment and cleanup the original statement
// that hosted it.
if (sf.statements.length > 0 && host !== sf.statements[0]) {
if (trailing) {
ts.setSyntheticTrailingComments(host, undefined);
} else {
ts.setSyntheticLeadingComments(host, undefined);
}
ts.setSyntheticLeadingComments(sf.statements[0], comments);
}
}

function maybeFilterDecorator(
Expand Down
8 changes: 6 additions & 2 deletions packages/compiler-cli/src/ngtsc/transform/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ export function addImports(
const body = sf.statements.filter(stmt => !isImportStatement(stmt));
// Prepend imports if needed.
if (addedImports.length > 0) {
sf.statements =
ts.createNodeArray([...existingImports, ...addedImports, ...extraStatements, ...body]);
// If we prepend imports, we also prepend NotEmittedStatement to use it as an anchor
// for @fileoverview Closure annotation. If there is no @fileoverview annotations, this
// statement would be a noop.
const fileoverviewAnchorStmt = ts.createNotEmittedStatement(sf);
sf.statements = ts.createNodeArray(
[fileoverviewAnchorStmt, ...existingImports, ...addedImports, ...extraStatements, ...body]);
}

return sf;
Expand Down
103 changes: 103 additions & 0 deletions packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2179,6 +2179,109 @@ describe('ngtsc behavioral tests', () => {
expect(afterCount).toBe(1);
});

describe('@fileoverview Closure annotations', () => {
it('should be produced if not present in source file', () => {
env.tsconfig({
'annotateForClosureCompiler': true,
});
env.write(`test.ts`, `
import {Component} from '@angular/core';
@Component({
template: '<div class="test"></div>',
})
export class SomeComp {}
`);

env.driveMain();
const jsContents = env.getContents('test.js');
const fileoverview = `
/**
* @fileoverview added by tsickle
* @suppress {checkTypes,extraRequire,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
*/
`;
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});

it('should be produced for empty source files', () => {
env.tsconfig({
'annotateForClosureCompiler': true,
});
env.write(`test.ts`, ``);

env.driveMain();
const jsContents = env.getContents('test.js');
const fileoverview = `
/**
* @fileoverview added by tsickle
* @suppress {checkTypes,extraRequire,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
*/
`;
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});

it('should always be at the very beginning of a script (if placed above imports)', () => {
env.tsconfig({
'annotateForClosureCompiler': true,
});
env.write(`test.ts`, `
/**
* @fileoverview Some Comp overview
* @modName {some_comp}
*/
import {Component} from '@angular/core';
@Component({
template: '<div class="test"></div>',
})
export class SomeComp {}
`);

env.driveMain();
const jsContents = env.getContents('test.js');
const fileoverview = `
/**
*
* @fileoverview Some Comp overview
* @modName {some_comp}
*
* @suppress {checkTypes,extraRequire,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
*/
`;
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});

it('should always be at the very beginning of a script (if placed above non-imports)', () => {
env.tsconfig({
'annotateForClosureCompiler': true,
});
env.write(`test.ts`, `
/**
* @fileoverview Some Comp overview
* @modName {some_comp}
*/
const testConst = 'testConstValue';
const testFn = function() { return true; }
`);

env.driveMain();
const jsContents = env.getContents('test.js');
const fileoverview = `
/**
*
* @fileoverview Some Comp overview
* @modName {some_comp}
*
* @suppress {checkTypes,extraRequire,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
*/
`;
expect(trim(jsContents).startsWith(trim(fileoverview))).toBeTruthy();
});
});

describe('sanitization', () => {
it('should generate sanitizers for unsafe attributes in hostBindings fn in Directives', () => {
env.tsconfig();
Expand Down

0 comments on commit be121bb

Please sign in to comment.