Skip to content

Commit 86ba694

Browse files
rbucktonrbuckton
committed
PR Feedback
1 parent 45ebecf commit 86ba694

File tree

11 files changed

+43
-49
lines changed

11 files changed

+43
-49
lines changed

‎src/compiler/binder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ import {
114114
HasLocals,
115115
hasSyntacticModifier,
116116
Identifier,
117+
identifierToKeywordKind,
117118
idText,
118119
IfStatement,
119120
ImportClause,
@@ -283,7 +284,6 @@ import {
283284
SpreadElement,
284285
Statement,
285286
StringLiteral,
286-
stringToToken,
287287
SuperExpression,
288288
SwitchStatement,
289289
Symbol,
@@ -2422,7 +2422,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
24222422
!isIdentifierName(node)) {
24232423

24242424
// strict mode identifiers
2425-
const originalKeywordKind = stringToToken(node.escapedText as string);
2425+
const originalKeywordKind = identifierToKeywordKind(node);
24262426
if (inStrictMode &&
24272427
originalKeywordKind! >= SyntaxKind.FirstFutureReservedWord &&
24282428
originalKeywordKind! <= SyntaxKind.LastFutureReservedWord) {

‎src/compiler/checker.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,7 @@ import {
377377
hasSyntacticModifiers,
378378
HeritageClause,
379379
Identifier,
380+
identifierToKeywordKind,
380381
IdentifierTypePredicate,
381382
idText,
382383
IfStatement,
@@ -927,7 +928,6 @@ import {
927928
StringLiteralLike,
928929
StringLiteralType,
929930
StringMappingType,
930-
stringToToken,
931931
stripQuotes,
932932
StructuredType,
933933
SubstitutionType,
@@ -23213,7 +23213,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
2321323213
case SyntaxKind.Parameter:
2321423214
const param = declaration as ParameterDeclaration;
2321523215
if (isIdentifier(param.name)) {
23216-
const originalKeywordKind = stringToToken(param.name.escapedText as string);
23216+
const originalKeywordKind = identifierToKeywordKind(param.name);
2321723217
if ((isCallSignatureDeclaration(param.parent) || isMethodSignature(param.parent) || isFunctionTypeNode(param.parent)) &&
2321823218
param.parent.parameters.indexOf(param) > -1 &&
2321923219
(resolveName(param, param.name.escapedText, SymbolFlags.Type, undefined, param.name.escapedText, /*isUse*/ true) ||

‎src/compiler/transformers/es5.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
Expression,
66
getOriginalNodeId,
77
Identifier,
8+
identifierToKeywordKind,
89
isIdentifier,
910
isPrivateIdentifier,
1011
isPropertyAccessExpression,
@@ -18,7 +19,6 @@ import {
1819
PropertyAssignment,
1920
setTextRange,
2021
SourceFile,
21-
stringToToken,
2222
SyntaxKind,
2323
TransformationContext,
2424
} from "../_namespaces/ts";
@@ -137,7 +137,7 @@ export function transformES5(context: TransformationContext): (x: SourceFile | B
137137
* @param name An Identifier
138138
*/
139139
function trySubstituteReservedName(name: Identifier) {
140-
const token = stringToToken(name.escapedText as string);
140+
const token = identifierToKeywordKind(name);
141141
if (token !== undefined && token >= SyntaxKind.FirstReservedWord && token <= SyntaxKind.LastReservedWord) {
142142
return setTextRange(factory.createStringLiteralFromNode(name), name);
143143
}

‎src/compiler/types.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,7 @@ export const enum NodeFlags {
834834

835835
// The following flags repurpose other NodeFlags as different meanings for Identifier nodes
836836
/** @internal */ IdentifierHasExtendedUnicodeEscape = ContainsThis, // Indicates whether the identifier contains an extended unicode escape sequence
837-
IdentifierIsInJSDocNamespace = HasAsyncFunctions, // Indicates whether the identifier is part of a JSDoc namespace
837+
/** @internal */ IdentifierIsInJSDocNamespace = HasAsyncFunctions, // Indicates whether the identifier is part of a JSDoc namespace
838838
}
839839

840840
export const enum ModifierFlags {
@@ -1685,10 +1685,6 @@ export interface Identifier extends PrimaryExpression, Declaration, JSDocContain
16851685
* Text of identifier, but if the identifier begins with two underscores, this will begin with three.
16861686
*/
16871687
readonly escapedText: __String;
1688-
/** @deprecated Use `idKeyword(identifier)` instead. */
1689-
readonly originalKeywordKind?: SyntaxKind; // Original syntaxKind which get set so that we can report an error later
1690-
/** @deprecated Use `identifier.flags & NodeFlags.IdentifierIsInJSDocNamespace` instead. */
1691-
readonly isInJSDocNamespace?: boolean; // if the node is a member in a JSDoc namespace.
16921688
}
16931689

16941690
// Transient identifier node (marked by id === -1)

‎src/compiler/utilities.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ import {
203203
HasTypeArguments,
204204
HeritageClause,
205205
Identifier,
206+
identifierToKeywordKind,
206207
IdentifierTypePredicate,
207208
identity,
208209
idText,
@@ -4347,7 +4348,7 @@ export function isStringAKeyword(name: string) {
43474348

43484349
/** @internal */
43494350
export function isIdentifierANonContextualKeyword(node: Identifier): boolean {
4350-
const originalKeywordKind = stringToToken(node.escapedText as string);
4351+
const originalKeywordKind = identifierToKeywordKind(node);
43514352
return !!originalKeywordKind && !isContextualKeyword(originalKeywordKind);
43524353
}
43534354

@@ -7335,20 +7336,6 @@ function Identifier(this: Mutable<Node>, kind: SyntaxKind, pos: number, end: num
73357336
this.emitNode = undefined;
73367337
}
73377338

7338-
Object.defineProperties(Identifier.prototype, {
7339-
originalKeywordKind: {
7340-
get(this: Identifier) {
7341-
return stringToToken(this.escapedText as string);
7342-
}
7343-
},
7344-
isInJSDocNamespace: {
7345-
get(this: Identifier) {
7346-
// NOTE: Returns `true` or `undefined` to match previous possible values.
7347-
return this.flags & NodeFlags.IdentifierIsInJSDocNamespace ? true : undefined;
7348-
}
7349-
}
7350-
});
7351-
73527339
function SourceMapSource(this: SourceMapSource, fileName: string, text: string, skipTrivia?: (pos: number) => number) {
73537340
this.fileName = fileName;
73547341
this.text = text;

‎src/compiler/utilitiesPublic.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ import {
138138
isJSDocTypeAlias,
139139
isJSDocTypeLiteral,
140140
isJSDocTypeTag,
141+
isKeyword,
141142
isModuleBlock,
142143
isNonNullExpression,
143144
isNotEmittedStatement,
@@ -259,6 +260,7 @@ import {
259260
TextChangeRange,
260261
TextRange,
261262
TextSpan,
263+
tryCast,
262264
TypeElement,
263265
TypeNode,
264266
TypeOnlyAliasDeclaration,
@@ -767,8 +769,9 @@ export function idText(identifierOrPrivateName: Identifier | PrivateIdentifier):
767769
* If the text of an Identifier matches a keyword (including contextual and TypeScript-specific keywords), returns the
768770
* SyntaxKind for the matching keyword.
769771
*/
770-
export function idKeyword(node: Identifier) {
771-
return stringToToken(node.escapedText as string);
772+
export function identifierToKeywordKind(node: Identifier) {
773+
const token = stringToToken(node.escapedText as string);
774+
return token ? tryCast(token, isKeyword) : undefined;
772775
}
773776

774777
export function symbolName(symbol: Symbol): string {

‎src/compiler/visitorPublic.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
FunctionBody,
88
getEmitFlags,
99
getEmitScriptTarget,
10+
HasChildren,
1011
Identifier,
1112
isArray,
1213
isArrayBindingElement,
@@ -98,7 +99,6 @@ import {
9899
Statement,
99100
SyntaxKind,
100101
TransformationContext,
101-
VisitEachChildNodes,
102102
Visitor,
103103
} from "./_namespaces/ts";
104104

@@ -518,7 +518,7 @@ type VisitEachChildFunction<T extends Node> = (node: T, visitor: Visitor, contex
518518
// }
519519
//
520520
// This is then used as the expected type for `visitEachChildTable`.
521-
type VisitEachChildTable = { [TNode in VisitEachChildNodes as TNode["kind"]]: VisitEachChildFunction<TNode> };
521+
type VisitEachChildTable = { [TNode in HasChildren as TNode["kind"]]: VisitEachChildFunction<TNode> };
522522

523523
// NOTE: Before you can add a new method to `visitEachChildTable`, you must first ensure the `Node` subtype you
524524
// wish to add is defined in the `HasChildren` union in types.ts.

‎src/deprecatedCompat/5.0/identifierProperties.ts

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,40 @@
11
import {
22
addObjectAllocatorPatcher,
3+
hasProperty,
34
Identifier,
4-
idKeyword,
5+
identifierToKeywordKind,
56
NodeFlags,
67
} from "../_namespaces/ts";
78
import { deprecate } from "../deprecate";
89

10+
declare module "../../compiler/types" {
11+
export interface Identifier {
12+
/** @deprecated Use `idKeyword(identifier)` instead. */
13+
readonly originalKeywordKind?: SyntaxKind;
14+
15+
/** @deprecated Use `.parent` or the surrounding context to determine this instead. */
16+
readonly isInJSDocNamespace?: boolean;
17+
}
18+
}
19+
920
addObjectAllocatorPatcher(objectAllocator => {
1021
const Identifier = objectAllocator.getIdentifierConstructor();
1122

12-
const propertyNames = Object.getOwnPropertyNames(Identifier.prototype);
13-
if (!propertyNames.includes("originalKeywordKind")) {
23+
if (!hasProperty(Identifier.prototype, "originalKeywordKind")) {
1424
Object.defineProperty(Identifier.prototype, "originalKeywordKind", {
1525
get: deprecate(function (this: Identifier) {
16-
return idKeyword(this);
26+
return identifierToKeywordKind(this);
1727
}, {
1828
name: "originalKeywordKind",
1929
since: "5.0",
2030
warnAfter: "5.1",
2131
errorAfter: "5.2",
22-
message: "Use 'idKeyword(identifier)' instead."
32+
message: "Use 'identifierToKeywordKind(identifier)' instead."
2333
})
2434
});
2535
}
2636

27-
if (!propertyNames.includes("isInJSDocNamespace")) {
37+
if (!hasProperty(Identifier.prototype, "isInJSDocNamespace")) {
2838
Object.defineProperty(Identifier.prototype, "isInJSDocNamespace", {
2939
get: deprecate(function (this: Identifier) {
3040
// NOTE: Returns `true` or `undefined` to match previous possible values.
@@ -34,7 +44,7 @@ addObjectAllocatorPatcher(objectAllocator => {
3444
since: "5.0",
3545
warnAfter: "5.1",
3646
errorAfter: "5.2",
37-
message: "Use 'identifier.flags & NodeFlags.IdentifierIsInJSDocNamespace' instead."
47+
message: "Use '.parent' or the surrounding context to determine this instead."
3848
})
3949
});
4050
}

‎src/services/codefixes/convertToEsModule.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ import {
5050
isExportsOrModuleExportsOrAlias,
5151
isFunctionExpression,
5252
isIdentifier,
53-
isNonContextualKeyword,
53+
isIdentifierANonContextualKeyword,
5454
isObjectLiteralExpression,
5555
isPropertyAccessExpression,
5656
isRequireCall,
@@ -75,7 +75,6 @@ import {
7575
SourceFile,
7676
Statement,
7777
StringLiteralLike,
78-
stringToToken,
7978
SymbolFlags,
8079
SyntaxKind,
8180
textChanges,
@@ -163,8 +162,7 @@ function collectExportRenames(sourceFile: SourceFile, checker: TypeChecker, iden
163162
const res = new Map<string, string>();
164163
forEachExportReference(sourceFile, node => {
165164
const { text } = node.name;
166-
const originalKeywordKind = stringToToken(text);
167-
if (!res.has(text) && (originalKeywordKind !== undefined && isNonContextualKeyword(originalKeywordKind)
165+
if (!res.has(text) && (isIdentifierANonContextualKeyword(node.name)
168166
|| checker.resolveName(text, node, SymbolFlags.Value, /*excludeGlobals*/ true))) {
169167
// Unconditionally add an underscore in case `text` is a keyword.
170168
res.set(text, makeUniqueName(`_${text}`, identifiers));

‎src/services/completions.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ import {
107107
hasInitializer,
108108
hasType,
109109
Identifier,
110+
identifierToKeywordKind,
110111
ImportDeclaration,
111112
ImportEqualsDeclaration,
112113
ImportKind,
@@ -1691,7 +1692,7 @@ function isModifierLike(node: Node): ModifierSyntaxKind | undefined {
16911692
return node.kind;
16921693
}
16931694
if (isIdentifier(node)) {
1694-
const originalKeywordKind = stringToToken(node.escapedText as string);
1695+
const originalKeywordKind = identifierToKeywordKind(node);
16951696
if (originalKeywordKind && isModifierKind(originalKeywordKind)) {
16961697
return originalKeywordKind;
16971698
}
@@ -4723,7 +4724,7 @@ function isFunctionLikeBodyKeyword(kind: SyntaxKind) {
47234724
}
47244725

47254726
function keywordForNode(node: Node): SyntaxKind {
4726-
return isIdentifier(node) ? stringToToken(node.escapedText as string) || SyntaxKind.Unknown : node.kind;
4727+
return isIdentifier(node) ? identifierToKeywordKind(node) ?? SyntaxKind.Unknown : node.kind;
47274728
}
47284729

47294730
function getContextualKeywords(
@@ -4827,8 +4828,8 @@ function tryGetObjectTypeDeclarationCompletionContainer(sourceFile: SourceFile,
48274828
}
48284829
break;
48294830
case SyntaxKind.Identifier: {
4830-
const originalKeywordKind = stringToToken((location as Identifier).text);
4831-
if (originalKeywordKind && isKeyword(originalKeywordKind)) {
4831+
const originalKeywordKind = identifierToKeywordKind(location as Identifier);
4832+
if (originalKeywordKind) {
48324833
return undefined;
48334834
}
48344835
// class c { public prop = c| }
@@ -4873,7 +4874,7 @@ function tryGetObjectTypeDeclarationCompletionContainer(sourceFile: SourceFile,
48734874
return undefined;
48744875
}
48754876
const isValidKeyword = isClassLike(contextToken.parent.parent) ? isClassMemberCompletionKeyword : isInterfaceOrTypeLiteralCompletionKeyword;
4876-
return (isValidKeyword(contextToken.kind) || contextToken.kind === SyntaxKind.AsteriskToken || isIdentifier(contextToken) && isValidKeyword(stringToToken(contextToken.text)!)) // TODO: GH#18217
4877+
return (isValidKeyword(contextToken.kind) || contextToken.kind === SyntaxKind.AsteriskToken || isIdentifier(contextToken) && isValidKeyword(identifierToKeywordKind(contextToken) ?? SyntaxKind.Unknown))
48774878
? contextToken.parent.parent as ObjectTypeDeclaration : undefined;
48784879
}
48794880
}

‎src/services/refactors/extractSymbol.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
hasEffectiveModifier,
6262
hasSyntacticModifier,
6363
Identifier,
64+
identifierToKeywordKind,
6465
isArray,
6566
isArrowFunction,
6667
isAssignmentExpression,
@@ -87,7 +88,6 @@ import {
8788
isJsxElement,
8889
isJsxFragment,
8990
isJsxSelfClosingElement,
90-
isKeyword,
9191
isModuleBlock,
9292
isParenthesizedTypeNode,
9393
isPartOfTypeNode,
@@ -136,7 +136,6 @@ import {
136136
SourceFile,
137137
Statement,
138138
StringLiteral,
139-
stringToToken,
140139
suppressLeadingAndTrailingTrivia,
141140
Symbol,
142141
SymbolFlags,
@@ -1352,7 +1351,7 @@ function extractConstantInScope(
13521351

13531352
// Make a unique name for the extracted variable
13541353
const file = scope.getSourceFile();
1355-
const localNameText = isPropertyAccessExpression(node) && !isClassLike(scope) && !checker.resolveName(node.name.text, node, SymbolFlags.Value, /*excludeGlobals*/ false) && !isPrivateIdentifier(node.name) && !isKeyword(stringToToken(node.name.escapedText as string) || SyntaxKind.Unknown)
1354+
const localNameText = isPropertyAccessExpression(node) && !isClassLike(scope) && !checker.resolveName(node.name.text, node, SymbolFlags.Value, /*excludeGlobals*/ false) && !isPrivateIdentifier(node.name) && !identifierToKeywordKind(node.name)
13561355
? node.name.text
13571356
: getUniqueName(isClassLike(scope) ? "newProperty" : "newLocal", file);
13581357
const isJS = isInJSFile(scope);

0 commit comments

Comments
 (0)