Skip to content

Commit

Permalink
Internal fix to summary extraction for toggles
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 623542905
  • Loading branch information
shicks authored and copybara-github committed Apr 10, 2024
1 parent d5c2921 commit 7dd33a0
Show file tree
Hide file tree
Showing 214 changed files with 1,524 additions and 998 deletions.
2 changes: 1 addition & 1 deletion demo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"dependencies": {
"minimist": "^1.2.3",
"tsickle": "file:../",
"typescript": "5.2.2"
"typescript": "5.4.2"
},
"devDependencies": {
"@types/minimist": "1.2.0",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"out/src/*"
],
"peerDependencies": {
"typescript": "~5.1.5"
"typescript": "~5.4.2"
},
"devDependencies": {
"@types/diff-match-patch": "^1.0.32",
Expand All @@ -28,7 +28,7 @@
"source-map-support": "^0.5.19",
"tslib": "^2.2.0",
"tslint": "^6.1.3",
"typescript": "5.2.2"
"typescript": "5.4.2"
},
"scripts": {
"build": "tsc",
Expand Down
59 changes: 48 additions & 11 deletions src/clutz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,19 @@ import * as googmodule from './googmodule';
import * as path from './path';
import {isDeclaredInClutzDts} from './type_translator';

interface ClutzHost {
/** See compiler_host.ts */
rootDirsRelative(fileName: string): string;
}

/**
* Constructs a ts.CustomTransformerFactory that postprocesses the .d.ts
* that are generated by ordinary TypeScript compilations to add some
* Clutz-specific logic. See generateClutzAliases.
*/
export function makeDeclarationTransformerFactory(
typeChecker: ts.TypeChecker,
googmoduleHost: googmodule.GoogModuleProcessorHost):
ts.CustomTransformerFactory {
host: ClutzHost&googmodule.GoogModuleProcessorHost): ts.CustomTransformerFactory {
return (context: ts.TransformationContext): ts.CustomTransformer => {
return {
transformBundle(): ts.Bundle {
Expand All @@ -49,8 +53,7 @@ export function makeDeclarationTransformerFactory(
// import 'path/to/the/js_file';
// so to for that import to resolve, you need to first import the clutz
// d.ts that defines that declared module.
const imports =
gatherNecessaryClutzImports(googmoduleHost, typeChecker, file);
const imports = gatherNecessaryClutzImports(host, typeChecker, file);
let importStmts: ts.Statement[]|undefined;
if (imports.length > 0) {
importStmts = imports.map(fileName => {
Expand All @@ -66,22 +69,56 @@ export function makeDeclarationTransformerFactory(
// Construct `declare global {}` in the Clutz namespace for symbols
// Clutz might use.
const globalBlock = generateClutzAliases(
file, googmoduleHost.pathToModuleName('', file.fileName),
typeChecker, options);
file, host.pathToModuleName('', file.fileName), typeChecker,
options);

// Only need to transform file if we needed one of the above additions.
if (!importStmts && !globalBlock) return file;

return ts.factory.updateSourceFile(file, [
...(importStmts ?? []),
...file.statements,
...(globalBlock ? [globalBlock] : []),
]);
return ts.factory.updateSourceFile(
file,
ts.setTextRange(
ts.factory.createNodeArray([
...(importStmts ?? []),
...file.statements,
...(globalBlock ? [globalBlock] : []),
]),
file.statements),
file.isDeclarationFile,
file.referencedFiles.map(
f => fixRelativeReference(f, file, options, host)),
// /// <reference types="x" /> directives are ignored under bazel.
/*typeReferences=*/[]);
}
};
};
}

/**
* Fixes a relative reference from an output file with respect to multiple
* rootDirs. See https://github.com/Microsoft/TypeScript/issues/8245 for
* details.
*/
function fixRelativeReference(
reference: ts.FileReference, origin: ts.SourceFile,
options: ts.CompilerOptions, host: ClutzHost): ts.FileReference {
if (!options.outDir || !options.rootDir) {
return reference;
}
const originDir = path.dirname(origin.fileName);
// Where TypeScript expects the output to be.
const expectedOutDir =
path.join(options.outDir, path.relative(options.rootDir, originDir));
const referencedFile = path.join(expectedOutDir, reference.fileName);
// Where the output is actually emitted.
const actualOutDir =
path.join(options.outDir, host.rootDirsRelative(originDir));
const fixedReference = path.relative(actualOutDir, referencedFile);

reference.fileName = fixedReference;
return reference;
}

/** Compares two strings and returns a number suitable for use in sort(). */
function stringCompare(a: string, b: string): number {
if (a < b) return -1;
Expand Down
6 changes: 2 additions & 4 deletions src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
return (context: ts.TransformationContext) => {
const result: ts.Transformer<ts.SourceFile> = (sourceFile: ts.SourceFile) => {
let nodeNeedingGoogReflect: undefined|ts.Node = undefined;
const visitor: ts.Visitor = (node) => {
const visitor = (node: ts.Node) => {
const replacementNode = rewriteDecorator(node);
if (replacementNode) {
nodeNeedingGoogReflect = node;
Expand All @@ -107,9 +107,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
return ts.visitEachChild(node, visitor, context);
};
let updatedSourceFile =
// TODO: go/ts50upgrade - Remove after upgrade.
// tslint:disable-next-line:no-unnecessary-type-assertion
ts.visitNode(sourceFile, visitor, ts.isSourceFile)!;
ts.visitNode(sourceFile, visitor, ts.isSourceFile);
if (nodeNeedingGoogReflect !== undefined) {
const statements = [...updatedSourceFile.statements];
const googModuleIndex = statements.findIndex(isGoogModuleStatement);
Expand Down
9 changes: 7 additions & 2 deletions src/enum_transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* type resolve ("@type {Foo}").
*/

import {TsickleHost} from 'tsickle';
import * as ts from 'typescript';

import * as jsdoc from './jsdoc';
Expand Down Expand Up @@ -95,7 +96,7 @@ export function getEnumType(typeChecker: ts.TypeChecker, enumDecl: ts.EnumDeclar
/**
* Transformer factory for the enum transformer. See fileoverview for details.
*/
export function enumTransformer(typeChecker: ts.TypeChecker):
export function enumTransformer(host: TsickleHost, typeChecker: ts.TypeChecker):
(context: ts.TransformationContext) => ts.Transformer<ts.SourceFile> {
return (context: ts.TransformationContext) => {
function visitor<T extends ts.Node>(node: T): T|ts.Node[] {
Expand Down Expand Up @@ -180,7 +181,11 @@ export function enumTransformer(typeChecker: ts.TypeChecker):
/* modifiers */ undefined,
ts.factory.createVariableDeclarationList(
[varDecl],
/* create a const var */ ts.NodeFlags.Const)),
/* When using unoptimized namespaces, create a var
declaration, otherwise create a const var. See b/157460535 */
host.useDeclarationMergingTransformation ?
ts.NodeFlags.Const :
undefined)),
node),
node);

Expand Down
13 changes: 1 addition & 12 deletions src/externs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,24 +295,13 @@ export function generateExterns(
* interface Foo { x: number; }
* interface Foo { y: number; }
* we only want to emit the "\@record" for Foo on the first one.
*
* The exception are variable declarations, which - in externs - do not assign a value:
* /.. \@type {...} ./
* var someVariable;
* /.. \@type {...} ./
* someNamespace.someVariable;
* If a later declaration wants to add additional properties on someVariable, tsickle must still
* emit an assignment into the object, as it's otherwise absent.
*/
function isFirstValueDeclaration(decl: ts.DeclarationStatement): boolean {
if (!decl.name) return true;
const sym = typeChecker.getSymbolAtLocation(decl.name)!;
if (!sym.declarations || sym.declarations.length < 2) return true;
const earlierDecls = sym.declarations.slice(0, sym.declarations.indexOf(decl));
// Either there are no earlier declarations, or all of them are variables (see above). tsickle
// emits a value for all other declaration kinds (function for functions, classes, interfaces,
// {} object for namespaces).
return earlierDecls.length === 0 || earlierDecls.every(ts.isVariableDeclaration);
return earlierDecls.length === 0 || earlierDecls.every(d => ts.isVariableDeclaration(d) && d.getSourceFile() !== decl.getSourceFile());
}

/** Writes the actual variable statement of a Closure variable declaration. */
Expand Down
82 changes: 35 additions & 47 deletions src/fileoverview_comment_transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as ts from 'typescript';

import * as jsdoc from './jsdoc';
import * as path from './path';
import {createNotEmittedStatement, reportDiagnostic, synthesizeCommentRanges, updateSourceFileNode} from './transformer_util';
import {reportDiagnostic, synthesizeCommentRanges, updateSourceFileNode} from './transformer_util';

/**
* A set of JSDoc tags that mark a comment as a fileoverview comment. These are
Expand Down Expand Up @@ -65,6 +65,9 @@ function augmentFileoverviewComments(
// * Suppress uselessCode. We emit an "if (false)" around type
// declarations, which is flagged as unused code unless we suppress it.
'uselessCode',
// suspiciousCode errors flag patterns that are suspicious if human-written
// but not inherently wrong. See also b/323580655.
'suspiciousCode',
// * Suppress some checks for user errors that TS already checks.
'missingReturn',
'unusedPrivateMembers',
Expand Down Expand Up @@ -152,35 +155,34 @@ export function transformFileoverviewCommentFactory(
// they do not get lost later on.
const synthesizedComments =
jsdoc.synthesizeLeadingComments(firstStatement);
const notEmitted = ts.factory.createNotEmittedStatement(sourceFile);
// Modify the comments on the firstStatement in place by removing the
// file-level comments.
fileComments = synthesizedComments.splice(0, i + 1);
// Move the fileComments onto notEmitted.
ts.setSyntheticLeadingComments(notEmitted, fileComments);
sourceFile =
updateSourceFileNode(sourceFile, ts.factory.createNodeArray([
notEmitted, firstStatement, ...sourceFile.statements.slice(1)
]));
break;
}
}


// Now walk every top level statement and escape/drop any @fileoverview
// comments found. Closure ignores all @fileoverview comments but the
// last, so tsickle must make sure not to emit duplicated ones.
for (let i = 0; i < sourceFile.statements.length; i++) {
const stmt = sourceFile.statements[i];
// Accept the NotEmittedStatement inserted above.
if (i === 0 && stmt.kind === ts.SyntaxKind.NotEmittedStatement) {
continue;
}
const comments = jsdoc.synthesizeLeadingComments(stmt);
checkNoFileoverviewComments(
stmt, comments,
`file comments must be at the top of the file, ` +
`separated from the file body by an empty line.`);
// Move the fileComments onto notEmitted.
const notEmitted = ts.factory.createNotEmittedStatement(sourceFile);
ts.setSyntheticLeadingComments(notEmitted, fileComments);
sourceFile = updateSourceFileNode(
sourceFile,
ts.factory.createNodeArray([notEmitted, ...sourceFile.statements]));

// Now walk every top level statement and escape/drop any @fileoverview
// comments found. Closure ignores all @fileoverview comments but the
// last, so tsickle must make sure not to emit duplicated ones.
for (let i = 0; i < sourceFile.statements.length; i++) {
const stmt = sourceFile.statements[i];
// Accept the NotEmittedStatement inserted above.
if (i === 0 && stmt.kind === ts.SyntaxKind.NotEmittedStatement) {
continue;
}
const comments = jsdoc.synthesizeLeadingComments(stmt);
checkNoFileoverviewComments(
stmt, comments,
`file comments must be at the top of the file, ` +
`separated from the file body by an empty line.`);
}

// Closure Compiler considers the *last* comment with @fileoverview (or
Expand All @@ -192,14 +194,17 @@ export function transformFileoverviewCommentFactory(
let fileoverviewIdx = -1;
let tags: jsdoc.Tag[] = [];
for (let i = fileComments.length - 1; i >= 0; i--) {
const parse = jsdoc.parseContents(fileComments[i].text);
if (parse !== null &&
parse.tags.some(t => FILEOVERVIEW_COMMENT_MARKERS.has(t.tagName))) {
const parsed = jsdoc.parse(fileComments[i]);
if (parsed !== null &&
parsed.tags.some(
t => FILEOVERVIEW_COMMENT_MARKERS.has(t.tagName))) {
fileoverviewIdx = i;
tags = parse.tags;
tags = parsed.tags;
break;
}
}
const mutableJsDoc = new jsdoc.MutableJSDoc(
notEmitted, fileComments, fileoverviewIdx, tags);

if (fileoverviewIdx !== -1) {
checkNoFileoverviewComments(
Expand All @@ -208,28 +213,11 @@ export function transformFileoverviewCommentFactory(
`duplicate file level comment`);
}

augmentFileoverviewComments(options, sourceFile, tags, generateExtraSuppressions);
const commentText = jsdoc.toStringWithoutStartEnd(tags);

if (fileoverviewIdx < 0) {
// No existing comment to merge with, just emit a new one.
return addNewFileoverviewComment(sourceFile, commentText);
}
augmentFileoverviewComments(
options, sourceFile, mutableJsDoc.tags, generateExtraSuppressions);
mutableJsDoc.updateComment();

fileComments[fileoverviewIdx].text = commentText;
// sf does not need to be updated, synthesized comments are mutable.
return sourceFile;
};
};
}

function addNewFileoverviewComment(
sf: ts.SourceFile, commentText: string): ts.SourceFile {
let syntheticFirstStatement = createNotEmittedStatement(sf);
syntheticFirstStatement = ts.addSyntheticTrailingComment(
syntheticFirstStatement, ts.SyntaxKind.MultiLineCommentTrivia,
commentText, true);
return updateSourceFileNode(
sf,
ts.factory.createNodeArray([syntheticFirstStatement, ...sf.statements]));
}
Loading

0 comments on commit 7dd33a0

Please sign in to comment.