Fix crash when binding jsdoc-style inner namepaths (#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

src/compiler/checker.ts

@@ -22807,11 +22807,7 @@ namespace ts {
             addDiagnostic(UnusedKind.Local, createDiagnosticForNode(node, message, name));
         }
 
-        function parameterNameStartsWithUnderscore(parameterName: DeclarationName) {
-            return parameterName && isIdentifierThatStartsWithUnderScore(parameterName);
-        }
-
-        function isIdentifierThatStartsWithUnderScore(node: Node) {
+        function isIdentifierThatStartsWithUnderscore(node: Node) {
             return isIdentifier(node) && idText(node).charCodeAt(0) === CharacterCodes._;
         }
 
@@ -22859,7 +22855,7 @@ namespace ts {
             const typeParameters = getEffectiveTypeParameterDeclarations(node);
             if (!(node.flags & NodeFlags.Ambient) && last(getSymbolOfNode(node).declarations) === node) {
                 for (const typeParameter of typeParameters) {
-                    if (!(getMergedSymbol(typeParameter.symbol).isReferenced! & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {
+                    if (!(getMergedSymbol(typeParameter.symbol).isReferenced! & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderscore(typeParameter.name)) {
                         addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(typeParameter.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(typeParameter.symbol)));
                     }
                 }
@@ -22897,7 +22893,7 @@ namespace ts {
 
                 for (const declaration of local.declarations) {
                     if (isAmbientModule(declaration) ||
-                        (isVariableDeclaration(declaration) && isForInOrOfStatement(declaration.parent.parent) || isImportedDeclaration(declaration)) && isIdentifierThatStartsWithUnderScore(declaration.name!)) {
+                        (isVariableDeclaration(declaration) && isForInOrOfStatement(declaration.parent.parent) || isImportedDeclaration(declaration)) && isIdentifierThatStartsWithUnderscore(declaration.name!)) {
                         continue;
                     }
 
@@ -22916,9 +22912,9 @@ namespace ts {
                     }
                     else {
                         const parameter = local.valueDeclaration && tryGetRootParameterDeclaration(local.valueDeclaration);
-                        if (parameter) {
-                            const name = getNameOfDeclaration(local.valueDeclaration);
-                            if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !parameterNameStartsWithUnderscore(name)) {
+                        const name = getNameOfDeclaration(local.valueDeclaration);
+                        if (parameter && name) {
+                            if (!isParameterPropertyDeclaration(parameter) && !parameterIsThisKeyword(parameter) && !isIdentifierThatStartsWithUnderscore(name)) {
                                 addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local)));
                             }
                         }
@@ -24213,18 +24209,19 @@ namespace ts {
                 }
 
                 const propDeclaration = prop.valueDeclaration;
+                const name = getNameOfDeclaration(propDeclaration);
 
                 // index is numeric and property name is not valid numeric literal
-                if (indexKind === IndexKind.Number && !(propDeclaration ? isNumericName(getNameOfDeclaration(propDeclaration)) : isNumericLiteralName(prop.escapedName))) {
+                if (indexKind === IndexKind.Number && !(name ? isNumericName(name) : isNumericLiteralName(prop.escapedName))) {
                     return;
                 }
 
                 // perform property check if property or indexer is declared in 'type'
                 // this allows us to rule out cases when both property and indexer are inherited from the base class
                 let errorNode: Node | undefined;
-                if (propDeclaration &&
+                if (propDeclaration && name &&
                     (propDeclaration.kind === SyntaxKind.BinaryExpression ||
-                     getNameOfDeclaration(propDeclaration).kind === SyntaxKind.ComputedPropertyName ||
+                     name.kind === SyntaxKind.ComputedPropertyName ||
                      prop.parent === containingType.symbol)) {
                     errorNode = propDeclaration;
                 }

src/compiler/factory.ts

@@ -194,10 +194,10 @@ namespace ts {
     }
 
     /** Create a unique name generated for a node. */
-    export function getGeneratedNameForNode(node: Node): Identifier;
-    /* @internal */ export function getGeneratedNameForNode(node: Node, flags: GeneratedIdentifierFlags): Identifier; // tslint:disable-line unified-signatures
-    export function getGeneratedNameForNode(node: Node, flags?: GeneratedIdentifierFlags): Identifier {
-        const name = createIdentifier(isIdentifier(node) ? idText(node) : "");
+    export function getGeneratedNameForNode(node: Node | undefined): Identifier;
+    /* @internal */ export function getGeneratedNameForNode(node: Node | undefined, flags: GeneratedIdentifierFlags): Identifier; // tslint:disable-line unified-signatures
+    export function getGeneratedNameForNode(node: Node | undefined, flags?: GeneratedIdentifierFlags): Identifier {
+        const name = createIdentifier(node && isIdentifier(node) ? idText(node) : "");
         name.autoGenerateFlags = GeneratedIdentifierFlags.Node | flags!;
         name.autoGenerateId = nextAutoGenerateId;
         name.original = node;

src/compiler/utilities.ts

@@ -745,8 +745,8 @@ namespace ts {
     // Return display name of an identifier
     // Computed property names will just be emitted as "[<expr>]", where <expr> is the source
     // text of the expression in the computed property.
-    export function declarationNameToString(name: DeclarationName | QualifiedName) {
-        return getFullWidth(name) === 0 ? "(Missing)" : getTextOfNode(name);
+    export function declarationNameToString(name: DeclarationName | QualifiedName | undefined) {
+        return !name || getFullWidth(name) === 0 ? "(Missing)" : getTextOfNode(name);
     }
 
     export function getNameFromIndexInfo(info: IndexInfo): string | undefined {
@@ -4858,7 +4858,7 @@ namespace ts {
 
     function getDeclarationIdentifier(node: Declaration | Expression): Identifier | undefined {
         const name = getNameOfDeclaration(node);
-        return isIdentifier(name) ? name : undefined;
+        return name && isIdentifier(name) ? name : undefined;
     }
 
     export function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | undefined {
@@ -4870,16 +4870,15 @@ namespace ts {
         return !!(node as NamedDeclaration).name; // A 'name' property should always be a DeclarationName.
     }
 
-    // TODO: GH#18217 This is often used as if it returns a defined result
-    export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName {
+    export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
         if (!declaration) {
-            return undefined!;
+            return undefined;
         }
         switch (declaration.kind) {
             case SyntaxKind.ClassExpression:
             case SyntaxKind.FunctionExpression:
                 if (!(declaration as ClassExpression | FunctionExpression).name) {
-                    return getAssignedName(declaration)!;
+                    return getAssignedName(declaration);
                 }
                 break;
             case SyntaxKind.Identifier:
@@ -4901,19 +4900,19 @@ namespace ts {
                     case SpecialPropertyAssignmentKind.PrototypeProperty:
                         return (expr.left as PropertyAccessExpression).name;
                     default:
-                        return undefined!;
+                        return undefined;
                 }
             }
             case SyntaxKind.JSDocCallbackTag:
-                return (declaration as JSDocCallbackTag).name!;
+                return (declaration as JSDocCallbackTag).name;
             case SyntaxKind.JSDocTypedefTag:
-                return getNameOfJSDocTypedef(declaration as JSDocTypedefTag)!;
+                return getNameOfJSDocTypedef(declaration as JSDocTypedefTag);
             case SyntaxKind.ExportAssignment: {
                 const { expression } = declaration as ExportAssignment;
-                return isIdentifier(expression) ? expression : undefined!;
+                return isIdentifier(expression) ? expression : undefined;
             }
         }
-        return (declaration as NamedDeclaration).name!;
+        return (declaration as NamedDeclaration).name;
     }
 
     function getAssignedName(node: Node): DeclarationName | undefined {

src/services/codefixes/inferFromUsage.ts

@@ -33,8 +33,9 @@ namespace ts.codefix {
             const token = getTokenAtPosition(sourceFile, start, /*includeJsDocComment*/ false);
             let declaration!: Declaration | undefined;
             const changes = textChanges.ChangeTracker.with(context, changes => { declaration = doChange(changes, sourceFile, token, errorCode, program, cancellationToken, /*markSeenseen*/ returnTrue); });
-            return changes.length === 0 ? undefined
-                : [createCodeFixAction(fixId, changes, [getDiagnostic(errorCode, token), getNameOfDeclaration(declaration!).getText(sourceFile)], fixId, Diagnostics.Infer_all_types_from_usage)];
+            const name = getNameOfDeclaration(declaration!);
+            return !name || changes.length === 0 ? undefined
+                : [createCodeFixAction(fixId, changes, [getDiagnostic(errorCode, token), name.getText(sourceFile)], fixId, Diagnostics.Infer_all_types_from_usage)];
         },
         fixIds: [fixId],
         getAllCodeActions(context) {

src/services/completions.ts

@@ -1122,7 +1122,7 @@ namespace ts.Completions {
             // If this is e.g. [Symbol.iterator], add a completion for `Symbol`.
             const symbolSymbol = firstDefined(symbol.declarations, decl => {
                 const name = getNameOfDeclaration(decl);
-                const leftName = name.kind === SyntaxKind.ComputedPropertyName ? getLeftMostName(name.expression) : undefined;
+                const leftName = name && name.kind === SyntaxKind.ComputedPropertyName ? getLeftMostName(name.expression) : undefined;
                 return leftName && typeChecker.getSymbolAtLocation(leftName);
             });
             if (symbolSymbol) {
@@ -1966,7 +1966,7 @@ namespace ts.Completions {
                     // NOTE: if one only performs this step when m.name is an identifier,
                     // things like '__proto__' are not filtered out.
                     const name = getNameOfDeclaration(m);
-                    existingName = isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
+                    existingName = name && isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
                 }
 
                 existingMemberNames.set(existingName!, true); // TODO: GH#18217

src/services/findAllReferences.ts

@@ -981,6 +981,7 @@ namespace ts.FindAllReferences.Core {
 
     function getReferenceForShorthandProperty({ flags, valueDeclaration }: Symbol, search: Search, state: State): void {
         const shorthandValueSymbol = state.checker.getShorthandAssignmentValueSymbol(valueDeclaration)!;
+        const name = getNameOfDeclaration(valueDeclaration);
         /*
          * Because in short-hand property assignment, an identifier which stored as name of the short-hand property assignment
          * has two meanings: property name and property value. Therefore when we do findAllReference at the position where
@@ -988,8 +989,8 @@ namespace ts.FindAllReferences.Core {
          * the position in short-hand property assignment excluding property accessing. However, if we do findAllReference at the
          * position of property accessing, the referenceEntry of such position will be handled in the first case.
          */
-        if (!(flags & SymbolFlags.Transient) && search.includes(shorthandValueSymbol)) {
-            addReference(getNameOfDeclaration(valueDeclaration), shorthandValueSymbol, state);
+        if (!(flags & SymbolFlags.Transient) && name && search.includes(shorthandValueSymbol)) {
+            addReference(name, shorthandValueSymbol, state);
         }
     }
 

src/services/formatting/formatting.ts

@@ -512,7 +512,10 @@ namespace ts.formatting {
                 // falls through
                 case SyntaxKind.PropertyDeclaration:
                 case SyntaxKind.Parameter:
-                    return getNameOfDeclaration(<Declaration>node).kind;
+                    const name = getNameOfDeclaration(<Declaration>node);
+                    if (name) {
+                        return name.kind;
+                    }
             }
         }
 

src/services/navigateTo.ts

@@ -113,7 +113,7 @@ namespace ts.NavigateTo {
         // First, if we started with a computed property name, then add all but the last
         // portion into the container array.
         const name = getNameOfDeclaration(declaration);
-        if (name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
+        if (name && name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
             return undefined;
         }
 

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

@@ -6106,7 +6106,7 @@ declare namespace ts {
     function isLateVisibilityPaintedStatement(node: Node): node is LateVisibilityPaintedStatement;
     function isAnyImportOrReExport(node: Node): node is AnyImportOrReExport;
     function getEnclosingBlockScopeContainer(node: Node): Node;
-    function declarationNameToString(name: DeclarationName | QualifiedName): string;
+    function declarationNameToString(name: DeclarationName | QualifiedName | undefined): string;
     function getNameFromIndexInfo(info: IndexInfo): string | undefined;
     function getTextOfPropertyName(name: PropertyName): __String;
     function entityNameToString(name: EntityNameOrEntityNameExpression): string;
@@ -6685,7 +6685,7 @@ declare namespace ts {
     function isNamedDeclaration(node: Node): node is NamedDeclaration & {
         name: DeclarationName;
     };
-    function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName;
+    function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
     /**
      * Gets the JSDoc parameter tags for the node if present.
      *
@@ -7702,8 +7702,8 @@ declare namespace ts {
     /** Create a unique name based on the supplied text. This does not consider names injected by the transformer. */
     function createFileLevelUniqueName(text: string): Identifier;
     /** Create a unique name generated for a node. */
-    function getGeneratedNameForNode(node: Node): Identifier;
-    function getGeneratedNameForNode(node: Node, flags: GeneratedIdentifierFlags): Identifier;
+    function getGeneratedNameForNode(node: Node | undefined): Identifier;
+    function getGeneratedNameForNode(node: Node | undefined, flags: GeneratedIdentifierFlags): Identifier;
     function createToken<TKind extends SyntaxKind>(token: TKind): Token<TKind>;
     function createSuper(): SuperExpression;
     function createThis(): ThisExpression & Token<SyntaxKind.ThisKeyword>;

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

@@ -3188,7 +3188,7 @@ declare namespace ts {
      */
     function unescapeIdentifier(id: string): string;
     function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | undefined;
-    function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName;
+    function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
     /**
      * Gets the JSDoc parameter tags for the node if present.
      *
@@ -3623,7 +3623,7 @@ declare namespace ts {
     /** Create a unique name based on the supplied text. This does not consider names injected by the transformer. */
     function createFileLevelUniqueName(text: string): Identifier;
     /** Create a unique name generated for a node. */
-    function getGeneratedNameForNode(node: Node): Identifier;
+    function getGeneratedNameForNode(node: Node | undefined): Identifier;
     function createToken<TKind extends SyntaxKind>(token: TKind): Token<TKind>;
     function createSuper(): SuperExpression;
     function createThis(): ThisExpression & Token<SyntaxKind.ThisKeyword>;

tests/baselines/reference/typedefInnerNamepaths.errors.txt

@@ -0,0 +1,28 @@
+tests/cases/conformance/jsdoc/bug25104.js(1,7): error TS2300: Duplicate identifier 'C'.
+tests/cases/conformance/jsdoc/bug25104.js(3,19): error TS1005: '}' expected.
+tests/cases/conformance/jsdoc/bug25104.js(4,26): error TS2300: Duplicate identifier 'C'.
+tests/cases/conformance/jsdoc/bug25104.js(6,18): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
+tests/cases/conformance/jsdoc/bug25104.js(6,18): error TS1005: '}' expected.
+
+
+==== tests/cases/conformance/jsdoc/bug25104.js (5 errors) ====
+    class C {
+          ~
+!!! error TS2300: Duplicate identifier 'C'.
+        /**
+         * @typedef {C~A} C~B
+                      ~
+!!! error TS1005: '}' expected.
+         * @typedef {object} C~A
+                             ~
+!!! error TS2300: Duplicate identifier 'C'.
+         */
+        /** @param {C~A} o */
+                     
+!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
+                     ~
+!!! error TS1005: '}' expected.
+        constructor(o) {
+        }
+    }
+    

tests/baselines/reference/typedefInnerNamepaths.symbols

@@ -0,0 +1,14 @@
+=== tests/cases/conformance/jsdoc/bug25104.js ===
+class C {
+>C : Symbol(C, Decl(bug25104.js, 0, 0))
+
+    /**
+     * @typedef {C~A} C~B
+     * @typedef {object} C~A
+     */
+    /** @param {C~A} o */
+    constructor(o) {
+>o : Symbol(o, Decl(bug25104.js, 6, 16))
+    }
+}
+

tests/baselines/reference/typedefInnerNamepaths.types

@@ -0,0 +1,14 @@
+=== tests/cases/conformance/jsdoc/bug25104.js ===
+class C {
+>C : C
+
+    /**
+     * @typedef {C~A} C~B
+     * @typedef {object} C~A
+     */
+    /** @param {C~A} o */
+    constructor(o) {
+>o : any
+    }
+}
+

tests/cases/conformance/jsdoc/typedefInnerNamepaths.ts

@@ -0,0 +1,13 @@
+// @noEmit: true
+// @allowJs: true
+// @checkJs: true
+// @Filename: bug25104.js
+class C {
+    /**
+     * @typedef {C~A} C~B
+     * @typedef {object} C~A
+     */
+    /** @param {C~A} o */
+    constructor(o) {
+    }
+}