Skip to content

Eliminate (ts as any).SyntaxKind (and similar) in favor of Debug.format functions #49485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 10, 2022
Merged

Eliminate (ts as any).SyntaxKind (and similar) in favor of Debug.format functions #49485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ namespace ts {
case SyntaxKind.Parameter:
// Parameters with names are handled at the top of this function. Parameters
// without names can only come from JSDocFunctionTypes.
Debug.assert(node.parent.kind === SyntaxKind.JSDocFunctionType, "Impossible parameter parent kind", () => `parent is: ${(ts as any).SyntaxKind ? (ts as any).SyntaxKind[node.parent.kind] : node.parent.kind}, expected JSDocFunctionType`);
Debug.assert(node.parent.kind === SyntaxKind.JSDocFunctionType, "Impossible parameter parent kind", () => `parent is: ${Debug.formatSyntaxKind(node.parent.kind)}, expected JSDocFunctionType`);
const functionType = node.parent as JSDocFunctionType;
const index = functionType.parameters.indexOf(node as ParameterDeclaration);
return "arg" + index as __String;
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/transformers/declarations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ namespace ts {
while (length(lateMarkedStatements)) {
const i = lateMarkedStatements!.shift()!;
if (!isLateVisibilityPaintedStatement(i)) {
return Debug.fail(`Late replaced statement was found which is not handled by the declaration transformer!: ${(ts as any).SyntaxKind ? (ts as any).SyntaxKind[(i as any).kind] : (i as any).kind}`);
return Debug.fail(`Late replaced statement was found which is not handled by the declaration transformer!: ${Debug.formatSyntaxKind((i as Node).kind)}`);
}
const priorNeedsDeclare = needsDeclare;
needsDeclare = i.parent && isSourceFile(i.parent) && !(isExternalModule(i.parent) && isBundledEmit);
Expand Down Expand Up @@ -1067,7 +1067,7 @@ namespace ts {
input.isTypeOf
));
}
default: Debug.assertNever(input, `Attempted to process unhandled node kind: ${(ts as any).SyntaxKind[(input as any).kind]}`);
default: Debug.assertNever(input, `Attempted to process unhandled node kind: ${Debug.formatSyntaxKind((input as Node).kind)}`);
}
}

Expand Down Expand Up @@ -1481,7 +1481,7 @@ namespace ts {
}
}
// Anything left unhandled is an error, so this should be unreachable
return Debug.assertNever(input, `Unhandled top-level node in declaration emit: ${(ts as any).SyntaxKind[(input as any).kind]}`);
return Debug.assertNever(input, `Unhandled top-level node in declaration emit: ${Debug.formatSyntaxKind((input as Node).kind)}`);

function cleanup<T extends Node>(node: T | undefined): T | undefined {
if (isEnclosingDeclaration(input)) {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/declarations/diagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ namespace ts {
return getTypeAliasDeclarationVisibilityError;
}
else {
return Debug.assertNever(node, `Attempted to set a declaration diagnostic context for unhandled node kind: ${(ts as any).SyntaxKind[(node as any).kind]}`);
return Debug.assertNever(node, `Attempted to set a declaration diagnostic context for unhandled node kind: ${Debug.formatSyntaxKind((node as Node).kind)}`);
}

function getVariableDeclarationTypeVisibilityDiagnosticMessage(symbolAccessibilityResult: SymbolAccessibilityResult) {
Expand Down Expand Up @@ -386,7 +386,7 @@ namespace ts {
Diagnostics.Parameter_0_of_accessor_has_or_is_using_private_name_1;

default:
return Debug.fail(`Unknown parent for parameter: ${(ts as any).SyntaxKind[node.parent.kind]}`);
return Debug.fail(`Unknown parent for parameter: ${Debug.formatSyntaxKind(node.parent.kind)}`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,9 @@ namespace ts {
JSDocFunctionType,
JSDocVariadicType,
JSDocNamepathType, // https://jsdoc.app/about-namepaths.html
JSDoc,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved this up here because our debug helpers choose the earliest name as the printed name. It was previously defined with the markers, which meant we printed out the deprecated form instead.

/** @deprecated Use SyntaxKind.JSDoc */
JSDocComment,
JSDocComment = JSDoc,
JSDocText,
JSDocTypeLiteral,
JSDocSignature,
Expand Down Expand Up @@ -457,7 +458,6 @@ namespace ts {
LastJSDocTagNode = JSDocPropertyTag,
/* @internal */ FirstContextualKeyword = AbstractKeyword,
/* @internal */ LastContextualKeyword = OfKeyword,
JSDoc = JSDocComment,
}

export type TriviaSyntaxKind =
Expand Down
49 changes: 5 additions & 44 deletions src/harness/harnessUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ namespace Utils {
const child = (node as any)[childName];
if (isNodeOrArray(child)) {
assert.isFalse(childNodesAndArrays.indexOf(child) < 0,
"Missing child when forEach'ing over node: " + (ts as any).SyntaxKind[node.kind] + "-" + childName);
"Missing child when forEach'ing over node: " + ts.Debug.formatSyntaxKind(node.kind) + "-" + childName);
}
}
}
Expand Down Expand Up @@ -169,54 +169,15 @@ namespace Utils {
export function sourceFileToJSON(file: ts.Node): string {
return JSON.stringify(file, (_, v) => isNodeOrArray(v) ? serializeNode(v) : v, " ");

function getKindName(k: number | string): string {
if (ts.isString(k)) {
function getKindName(k: number | string | undefined): string | undefined {
if (k === undefined || ts.isString(k)) {
return k;
}

// For some markers in SyntaxKind, we should print its original syntax name instead of
// the marker name in tests.
if (k === (ts as any).SyntaxKind.FirstJSDocNode ||
k === (ts as any).SyntaxKind.LastJSDocNode ||
k === (ts as any).SyntaxKind.FirstJSDocTagNode ||
k === (ts as any).SyntaxKind.LastJSDocTagNode) {
for (const kindName in (ts as any).SyntaxKind) {
if ((ts as any).SyntaxKind[kindName] === k) {
return kindName;
}
}
}
Comment on lines -179 to -188
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using ts.Debug.formatSyntaxKind is slower because it does a lot more work than these 4 hardcoded things. To regain the perf, I have another PR that will make formatSyntaxKind faster.

I still think it's better to not write the same enum-member-to-string code more than once, but it's annoying that a perf fix is required to do it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#49487 recovers the performance; it's 25% of the test run :(


return (ts as any).SyntaxKind[k];
}

function getFlagName(flags: any, f: number): any {
if (f === 0) {
return 0;
}

let result = "";
ts.forEach(Object.getOwnPropertyNames(flags), (v: any) => {
if (isFinite(v)) {
v = +v;
if (f === +v) {
result = flags[v];
return true;
}
else if ((f & v) > 0) {
if (result.length) {
result += " | ";
}
result += flags[v];
return false;
}
}
});
return result;
return ts.Debug.formatSyntaxKind(k);
}

function getNodeFlagName(f: number) {
return getFlagName((ts as any).NodeFlags, f);
return ts.Debug.formatNodeFlags(f);
}

function serializeNode(n: ts.Node): any {
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.@link tags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
"modifierFlagsCache": 0,
"transformFlags": 0,
"name": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 86,
"end": 97,
"modifierFlagsCache": 0,
Expand Down Expand Up @@ -194,7 +194,7 @@
"modifierFlagsCache": 0,
"transformFlags": 0,
"name": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 163,
"end": 181,
"modifierFlagsCache": 0,
Expand Down Expand Up @@ -273,7 +273,7 @@
"modifierFlagsCache": 0,
"transformFlags": 0,
"name": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 333,
"end": 338,
"modifierFlagsCache": 0,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.Nested @param tags.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
}
},
"name": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 50,
"end": 53,
"modifierFlagsCache": 0,
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/JSDocParsing/TypeExpressions.parsesCorrectly.newType1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"modifierFlagsCache": 0,
"transformFlags": 1,
"typeName": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 14,
"end": 17,
"flags": "JSDoc",
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/JSDocParsing/TypeExpressions.parsesCorrectly.thisType1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"modifierFlagsCache": 0,
"transformFlags": 1,
"typeName": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 15,
"end": 18,
"flags": "JSDoc",
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/JSDocParsing/TypeExpressions.parsesCorrectly.typeReference3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"modifierFlagsCache": 0,
"transformFlags": 1,
"typeName": {
"kind": "FirstNode",
"kind": "QualifiedName",
"pos": 1,
"end": 11,
"flags": "JSDoc",
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ declare namespace ts {
JSDocFunctionType = 317,
JSDocVariadicType = 318,
JSDocNamepathType = 319,
JSDoc = 320,
/** @deprecated Use SyntaxKind.JSDoc */
JSDocComment = 320,
JSDocText = 321,
Expand Down Expand Up @@ -493,7 +494,6 @@ declare namespace ts {
LastJSDocNode = 347,
FirstJSDocTagNode = 327,
LastJSDocTagNode = 347,
JSDoc = 320
}
export type TriviaSyntaxKind = SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia | SyntaxKind.NewLineTrivia | SyntaxKind.WhitespaceTrivia | SyntaxKind.ShebangTrivia | SyntaxKind.ConflictMarkerTrivia;
export type LiteralSyntaxKind = SyntaxKind.NumericLiteral | SyntaxKind.BigIntLiteral | SyntaxKind.StringLiteral | SyntaxKind.JsxText | SyntaxKind.JsxTextAllWhiteSpaces | SyntaxKind.RegularExpressionLiteral | SyntaxKind.NoSubstitutionTemplateLiteral;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ declare namespace ts {
JSDocFunctionType = 317,
JSDocVariadicType = 318,
JSDocNamepathType = 319,
JSDoc = 320,
/** @deprecated Use SyntaxKind.JSDoc */
JSDocComment = 320,
JSDocText = 321,
Expand Down Expand Up @@ -493,7 +494,6 @@ declare namespace ts {
LastJSDocNode = 347,
FirstJSDocTagNode = 327,
LastJSDocTagNode = 347,
JSDoc = 320
}
export type TriviaSyntaxKind = SyntaxKind.SingleLineCommentTrivia | SyntaxKind.MultiLineCommentTrivia | SyntaxKind.NewLineTrivia | SyntaxKind.WhitespaceTrivia | SyntaxKind.ShebangTrivia | SyntaxKind.ConflictMarkerTrivia;
export type LiteralSyntaxKind = SyntaxKind.NumericLiteral | SyntaxKind.BigIntLiteral | SyntaxKind.StringLiteral | SyntaxKind.JsxText | SyntaxKind.JsxTextAllWhiteSpaces | SyntaxKind.RegularExpressionLiteral | SyntaxKind.NoSubstitutionTemplateLiteral;
Expand Down