Skip to content

Commit 43d0794

Browse files
authored
Fix crash when binding jsdoc-style inner namepaths (microsoft#25106)
* getDeclarationIdentifier handles undefined name getNameOfDeclaration actually doesn't handle all declarations, only those that produce names that could be reasonably used as an identifier. Until now, getDeclarationIdentifier assumed that getNameOfDeclaration always returned a name. This caused crashes whenever we tried to get the name of something like a Constructor. * Add test and baselines * getNameOfDeclaration can return undefined This requires all callers to handle it, which turns out now to be too disruptive. * Fix lint
1 parent 40899ea commit 43d0794

14 files changed

Lines changed: 113 additions & 43 deletions

src/compiler/checker.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22807,11 +22807,7 @@ namespace ts {
2280722807
addDiagnostic(UnusedKind.Local, createDiagnosticForNode(node, message, name));
2280822808
}
2280922809

22810-
function parameterNameStartsWithUnderscore(parameterName: DeclarationName) {
22811-
return parameterName && isIdentifierThatStartsWithUnderScore(parameterName);
22812-
}
22813-
22814-
function isIdentifierThatStartsWithUnderScore(node: Node) {
22810+
function isIdentifierThatStartsWithUnderscore(node: Node) {
2281522811
return isIdentifier(node) && idText(node).charCodeAt(0) === CharacterCodes._;
2281622812
}
2281722813

@@ -22859,7 +22855,7 @@ namespace ts {
2285922855
const typeParameters = getEffectiveTypeParameterDeclarations(node);
2286022856
if (!(node.flags & NodeFlags.Ambient) && last(getSymbolOfNode(node).declarations) === node) {
2286122857
for (const typeParameter of typeParameters) {
22862-
if (!(getMergedSymbol(typeParameter.symbol).isReferenced! & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {
22858+
if (!(getMergedSymbol(typeParameter.symbol).isReferenced! & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderscore(typeParameter.name)) {
2286322859
addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(typeParameter.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(typeParameter.symbol)));
2286422860
}
2286522861
}
@@ -22897,7 +22893,7 @@ namespace ts {
2289722893

2289822894
for (const declaration of local.declarations) {
2289922895
if (isAmbientModule(declaration) ||
22900-
(isVariableDeclaration(declaration) && isForInOrOfStatement(declaration.parent.parent) || isImportedDeclaration(declaration)) && isIdentifierThatStartsWithUnderScore(declaration.name!)) {
22896+
(isVariableDeclaration(declaration) && isForInOrOfStatement(declaration.parent.parent) || isImportedDeclaration(declaration)) && isIdentifierThatStartsWithUnderscore(declaration.name!)) {
2290122897
continue;
2290222898
}
2290322899

@@ -22916,9 +22912,9 @@ namespace ts {
2291622912
}
2291722913
else {
2291822914
const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
22919-
if (parameter) {
22920-
const name = getNameOfDeclaration(local.valueDeclaration);
22921-
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !parameterNameStartsWithUnderscore(name)) {
22915+
const name = getNameOfDeclaration(local.valueDeclaration);
22916+
if (parameter && name) {
22917+
if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
2292222918
addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
2292322919
}
2292422920
}
@@ -24213,18 +24209,19 @@ namespace ts {
2421324209
}
2421424210

2421524211
const propDeclaration = prop.valueDeclaration;
24212+
const name = getNameOfDeclaration(propDeclaration);
2421624213

2421724214
// index is numeric and property name is not valid numeric literal
24218-
if (indexKind === IndexKind.Number && !(propDeclaration ? isNumericName(getNameOfDeclaration(propDeclaration)) : isNumericLiteralName(prop.escapedName))) {
24215+
if (indexKind === IndexKind.Number && !(name ? isNumericName(name) : isNumericLiteralName(prop.escapedName))) {
2421924216
return;
2422024217
}
2422124218

2422224219
// perform property check if property or indexer is declared in 'type'
2422324220
// this allows us to rule out cases when both property and indexer are inherited from the base class
2422424221
let errorNode: Node | undefined;
24225-
if (propDeclaration &&
24222+
if (propDeclaration && name &&
2422624223
(propDeclaration.kind === SyntaxKind.BinaryExpression ||
24227-
getNameOfDeclaration(propDeclaration).kind === SyntaxKind.ComputedPropertyName ||
24224+
name.kind === SyntaxKind.ComputedPropertyName ||
2422824225
prop.parent === containingType.symbol)) {
2422924226
errorNode = propDeclaration;
2423024227
}

src/compiler/factory.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,10 @@ namespace ts {
194194
}
195195

196196
/** Create a unique name generated for a node. */
197-
export function getGeneratedNameForNode(node: Node): Identifier;
198-
/* @internal */ export function getGeneratedNameForNode(node: Node, flags: GeneratedIdentifierFlags): Identifier; // tslint:disable-line unified-signatures
199-
export function getGeneratedNameForNode(node: Node, flags?: GeneratedIdentifierFlags): Identifier {
200-
const name = createIdentifier(isIdentifier(node) ? idText(node) : "");
197+
export function getGeneratedNameForNode(node: Node | undefined): Identifier;
198+
/* @internal */ export function getGeneratedNameForNode(node: Node | undefined, flags: GeneratedIdentifierFlags): Identifier; // tslint:disable-line unified-signatures
199+
export function getGeneratedNameForNode(node: Node | undefined, flags?: GeneratedIdentifierFlags): Identifier {
200+
const name = createIdentifier(node && isIdentifier(node) ? idText(node) : "");
201201
name.autoGenerateFlags = GeneratedIdentifierFlags.Node | flags!;
202202
name.autoGenerateId = nextAutoGenerateId;
203203
name.original = node;

src/compiler/utilities.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -745,8 +745,8 @@ namespace ts {
745745
// Return display name of an identifier
746746
// Computed property names will just be emitted as "[<expr>]", where <expr> is the source
747747
// text of the expression in the computed property.
748-
export function declarationNameToString(name: DeclarationName | QualifiedName) {
749-
return getFullWidth(name) === 0 ? "(Missing)" : getTextOfNode(name);
748+
export function declarationNameToString(name: DeclarationName | QualifiedName | undefined) {
749+
return !name || getFullWidth(name) === 0 ? "(Missing)" : getTextOfNode(name);
750750
}
751751

752752
export function getNameFromIndexInfo(info: IndexInfo): string | undefined {
@@ -4858,7 +4858,7 @@ namespace ts {
48584858

48594859
function getDeclarationIdentifier(node: Declaration | Expression): Identifier | undefined {
48604860
const name = getNameOfDeclaration(node);
4861-
return isIdentifier(name) ? name : undefined;
4861+
return name && isIdentifier(name) ? name : undefined;
48624862
}
48634863

48644864
export function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | undefined {
@@ -4870,16 +4870,15 @@ namespace ts {
48704870
return !!(node as NamedDeclaration).name; // A 'name' property should always be a DeclarationName.
48714871
}
48724872

4873-
// TODO: GH#18217 This is often used as if it returns a defined result
4874-
export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName {
4873+
export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
48754874
if (!declaration) {
4876-
return undefined!;
4875+
return undefined;
48774876
}
48784877
switch (declaration.kind) {
48794878
case SyntaxKind.ClassExpression:
48804879
case SyntaxKind.FunctionExpression:
48814880
if (!(declaration as ClassExpression | FunctionExpression).name) {
4882-
return getAssignedName(declaration)!;
4881+
return getAssignedName(declaration);
48834882
}
48844883
break;
48854884
case SyntaxKind.Identifier:
@@ -4901,19 +4900,19 @@ namespace ts {
49014900
case SpecialPropertyAssignmentKind.PrototypeProperty:
49024901
return (expr.left as PropertyAccessExpression).name;
49034902
default:
4904-
return undefined!;
4903+
return undefined;
49054904
}
49064905
}
49074906
case SyntaxKind.JSDocCallbackTag:
4908-
return (declaration as JSDocCallbackTag).name!;
4907+
return (declaration as JSDocCallbackTag).name;
49094908
case SyntaxKind.JSDocTypedefTag:
4910-
return getNameOfJSDocTypedef(declaration as JSDocTypedefTag)!;
4909+
return getNameOfJSDocTypedef(declaration as JSDocTypedefTag);
49114910
case SyntaxKind.ExportAssignment: {
49124911
const { expression } = declaration as ExportAssignment;
4913-
return isIdentifier(expression) ? expression : undefined!;
4912+
return isIdentifier(expression) ? expression : undefined;
49144913
}
49154914
}
4916-
return (declaration as NamedDeclaration).name!;
4915+
return (declaration as NamedDeclaration).name;
49174916
}
49184917

49194918
function getAssignedName(node: Node): DeclarationName | undefined {

src/services/codefixes/inferFromUsage.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ namespace ts.codefix {
3333
const token = getTokenAtPosition(sourceFile, start, /*includeJsDocComment*/ false);
3434
let declaration!: Declaration | undefined;
3535
const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeenseen*/ returnTrue); });
36-
return changes.length === 0 ? undefined
37-
: [createCodeFixAction(fixId, changes, [getDiagnostic(errorCode, token), getNameOfDeclaration(declaration!).getText(sourceFile)], fixId, Diagnostics.Infer_all_types_from_usage)];
36+
const name = getNameOfDeclaration(declaration!);
37+
return !name || changes.length === 0 ? undefined
38+
: [createCodeFixAction(fixId, changes, [getDiagnostic(errorCode, token), name.getText(sourceFile)], fixId, Diagnostics.Infer_all_types_from_usage)];
3839
},
3940
fixIds: [fixId],
4041
getAllCodeActions(context) {

src/services/completions.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ namespace ts.Completions {
11221122
// If this is e.g. [Symbol.iterator], add a completion for `Symbol`.
11231123
const symbolSymbol = firstDefined(symbol.declarations, decl => {
11241124
const name = getNameOfDeclaration(decl);
1125-
const leftName = name.kind === SyntaxKind.ComputedPropertyName ? getLeftMostName(name.expression) : undefined;
1125+
const leftName = name && name.kind === SyntaxKind.ComputedPropertyName ? getLeftMostName(name.expression) : undefined;
11261126
return leftName && typeChecker.getSymbolAtLocation(leftName);
11271127
});
11281128
if (symbolSymbol) {
@@ -1966,7 +1966,7 @@ namespace ts.Completions {
19661966
// NOTE: if one only performs this step when m.name is an identifier,
19671967
// things like '__proto__' are not filtered out.
19681968
const name = getNameOfDeclaration(m);
1969-
existingName = isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
1969+
existingName = name && isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
19701970
}
19711971

19721972
existingMemberNames.set(existingName!, true); // TODO: GH#18217

src/services/findAllReferences.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -981,15 +981,16 @@ namespace ts.FindAllReferences.Core {
981981

982982
function getReferenceForShorthandProperty({ flags, valueDeclaration }: Symbol, search: Search, state: State): void {
983983
const shorthandValueSymbol = state.checker.getShorthandAssignmentValueSymbol(valueDeclaration)!;
984+
const name = getNameOfDeclaration(valueDeclaration);
984985
/*
985986
* Because in short-hand property assignment, an identifier which stored as name of the short-hand property assignment
986987
* has two meanings: property name and property value. Therefore when we do findAllReference at the position where
987988
* an identifier is declared, the language service should return the position of the variable declaration as well as
988989
* the position in short-hand property assignment excluding property accessing. However, if we do findAllReference at the
989990
* position of property accessing, the referenceEntry of such position will be handled in the first case.
990991
*/
991-
if (!(flags & SymbolFlags.Transient) && search.includes(shorthandValueSymbol)) {
992-
addReference(getNameOfDeclaration(valueDeclaration), shorthandValueSymbol, state);
992+
if (!(flags & SymbolFlags.Transient) && name && search.includes(shorthandValueSymbol)) {
993+
addReference(name, shorthandValueSymbol, state);
993994
}
994995
}
995996

src/services/formatting/formatting.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,10 @@ namespace ts.formatting {
512512
// falls through
513513
case SyntaxKind.PropertyDeclaration:
514514
case SyntaxKind.Parameter:
515-
return getNameOfDeclaration(<Declaration>node).kind;
515+
const name = getNameOfDeclaration(<Declaration>node);
516+
if (name) {
517+
return name.kind;
518+
}
516519
}
517520
}
518521

src/services/navigateTo.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ namespace ts.NavigateTo {
113113
// First, if we started with a computed property name, then add all but the last
114114
// portion into the container array.
115115
const name = getNameOfDeclaration(declaration);
116-
if (name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
116+
if (name && name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
117117
return undefined;
118118
}
119119

tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6106,7 +6106,7 @@ declare namespace ts {
61066106
function isLateVisibilityPaintedStatement(node: Node): node is LateVisibilityPaintedStatement;
61076107
function isAnyImportOrReExport(node: Node): node is AnyImportOrReExport;
61086108
function getEnclosingBlockScopeContainer(node: Node): Node;
6109-
function declarationNameToString(name: DeclarationName | QualifiedName): string;
6109+
function declarationNameToString(name: DeclarationName | QualifiedName | undefined): string;
61106110
function getNameFromIndexInfo(info: IndexInfo): string | undefined;
61116111
function getTextOfPropertyName(name: PropertyName): __String;
61126112
function entityNameToString(name: EntityNameOrEntityNameExpression): string;
@@ -6685,7 +6685,7 @@ declare namespace ts {
66856685
function isNamedDeclaration(node: Node): node is NamedDeclaration & {
66866686
name: DeclarationName;
66876687
};
6688-
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName;
6688+
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
66896689
/**
66906690
* Gets the JSDoc parameter tags for the node if present.
66916691
*
@@ -7702,8 +7702,8 @@ declare namespace ts {
77027702
/** Create a unique name based on the supplied text. This does not consider names injected by the transformer. */
77037703
function createFileLevelUniqueName(text: string): Identifier;
77047704
/** Create a unique name generated for a node. */
7705-
function getGeneratedNameForNode(node: Node): Identifier;
7706-
function getGeneratedNameForNode(node: Node, flags: GeneratedIdentifierFlags): Identifier;
7705+
function getGeneratedNameForNode(node: Node | undefined): Identifier;
7706+
function getGeneratedNameForNode(node: Node | undefined, flags: GeneratedIdentifierFlags): Identifier;
77077707
function createToken<TKind extends SyntaxKind>(token: TKind): Token<TKind>;
77087708
function createSuper(): SuperExpression;
77097709
function createThis(): ThisExpression & Token<SyntaxKind.ThisKeyword>;

tests/baselines/reference/api/typescript.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3188,7 +3188,7 @@ declare namespace ts {
31883188
*/
31893189
function unescapeIdentifier(id: string): string;
31903190
function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | undefined;
3191-
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName;
3191+
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
31923192
/**
31933193
* Gets the JSDoc parameter tags for the node if present.
31943194
*
@@ -3623,7 +3623,7 @@ declare namespace ts {
36233623
/** Create a unique name based on the supplied text. This does not consider names injected by the transformer. */
36243624
function createFileLevelUniqueName(text: string): Identifier;
36253625
/** Create a unique name generated for a node. */
3626-
function getGeneratedNameForNode(node: Node): Identifier;
3626+
function getGeneratedNameForNode(node: Node | undefined): Identifier;
36273627
function createToken<TKind extends SyntaxKind>(token: TKind): Token<TKind>;
36283628
function createSuper(): SuperExpression;
36293629
function createThis(): ThisExpression & Token<SyntaxKind.ThisKeyword>;

0 commit comments

Comments
 (0)