Add undefined to Symbol.declarations' type (#42975)

* Add undefined to Symbol.declarations' type Symbol.declarations now has type `Declaration[] | undefined`. I made a mistake somewhere in the checker related to JS checking, so there are quite a few test failures right now. * undo clever change to getDeclaringConstructor * Address PR comments 1. More early-returns. 2. More line breaks.
microsoft · Mar 1, 2021 · aa67b16 · aa67b16
1 parent 2a49cf7
commit aa67b16
Show file tree

Hide file tree

Showing 25 changed files with 364 additions and 309 deletions.
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -3266,7 +3266,7 @@ namespace ts {
                 if (node.name) {
                     setParent(node.name, node);
                 }
-                file.bindDiagnostics.push(createDiagnosticForNode(symbolExport.declarations[0], Diagnostics.Duplicate_identifier_0, symbolName(prototypeSymbol)));
+                file.bindDiagnostics.push(createDiagnosticForNode(symbolExport.declarations![0], Diagnostics.Duplicate_identifier_0, symbolName(prototypeSymbol)));
             }
             symbol.exports!.set(prototypeSymbol.escapedName, prototypeSymbol);
             prototypeSymbol.parent = symbol;

diff --git a/src/compiler/builderState.ts b/src/compiler/builderState.ts
@@ -166,14 +166,17 @@ namespace ts {
 
             // From ambient modules
             for (const ambientModule of program.getTypeChecker().getAmbientModules()) {
-                if (ambientModule.declarations.length > 1) {
+                if (ambientModule.declarations && ambientModule.declarations.length > 1) {
                     addReferenceFromAmbientModule(ambientModule);
                 }
             }
 
             return referencedFiles;
 
             function addReferenceFromAmbientModule(symbol: Symbol) {
+                if (!symbol.declarations) {
+                    return;
+                }
                 // Add any file other than our own as reference
                 for (const declaration of symbol.declarations) {
                     const declarationSourceFile = getSourceFileOfNode(declaration);

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
diff --git a/src/compiler/core.ts b/src/compiler/core.ts
@@ -292,7 +292,7 @@ namespace ts {
         return -1;
     }
 
-    export function countWhere<T>(array: readonly T[], predicate: (x: T, i: number) => boolean): number {
+    export function countWhere<T>(array: readonly T[] | undefined, predicate: (x: T, i: number) => boolean): number {
         let count = 0;
         if (array) {
             for (let i = 0; i < array.length; i++) {

diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts
@@ -382,7 +382,7 @@ namespace ts.moduleSpecifiers {
     }
 
     function tryGetModuleNameFromAmbientModule(moduleSymbol: Symbol, checker: TypeChecker): string | undefined {
-        const decl = find(moduleSymbol.declarations,
+        const decl = moduleSymbol.declarations?.find(
             d => isNonGlobalAmbientModule(d) && (!isExternalModuleAugmentation(d) || !isExternalModuleNameRelative(getTextOfIdentifierOrLiteral(d.name)))
         ) as (ModuleDeclaration & { name: StringLiteral }) | undefined;
         if (decl) {

diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts
@@ -206,13 +206,15 @@ namespace ts {
         }
 
         function reportNonlocalAugmentation(containingFile: SourceFile, parentSymbol: Symbol, symbol: Symbol) {
-            const primaryDeclaration = find(parentSymbol.declarations, d => getSourceFileOfNode(d) === containingFile)!;
+            const primaryDeclaration = parentSymbol.declarations?.find(d => getSourceFileOfNode(d) === containingFile)!;
             const augmentingDeclarations = filter(symbol.declarations, d => getSourceFileOfNode(d) !== containingFile);
-            for (const augmentations of augmentingDeclarations) {
-                context.addDiagnostic(addRelatedInfo(
-                    createDiagnosticForNode(augmentations, Diagnostics.Declaration_augments_declaration_in_another_file_This_cannot_be_serialized),
-                    createDiagnosticForNode(primaryDeclaration, Diagnostics.This_is_the_declaration_being_augmented_Consider_moving_the_augmenting_declaration_into_the_same_file)
-                ));
+            if (augmentingDeclarations) {
+                for (const augmentations of augmentingDeclarations) {
+                    context.addDiagnostic(addRelatedInfo(
+                        createDiagnosticForNode(augmentations, Diagnostics.Declaration_augments_declaration_in_another_file_This_cannot_be_serialized),
+                        createDiagnosticForNode(primaryDeclaration, Diagnostics.This_is_the_declaration_being_augmented_Consider_moving_the_augmenting_declaration_into_the_same_file)
+                    ));
+                }
             }
         }
 

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -4714,7 +4714,7 @@ namespace ts {
     export interface Symbol {
         flags: SymbolFlags;                     // Symbol flags
         escapedName: __String;                  // Name of symbol
-        declarations: Declaration[];            // Declarations associated with this symbol
+        declarations?: Declaration[];           // Declarations associated with this symbol
         valueDeclaration: Declaration;          // First value declaration of the symbol
         members?: SymbolTable;                  // Class, interface or object literal instance members
         exports?: SymbolTable;                  // Module exports

diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -767,7 +767,7 @@ namespace ts {
     }
 
     export function getNonAugmentationDeclaration(symbol: Symbol) {
-        return find(symbol.declarations, d => !isExternalModuleAugmentation(d) && !(isModuleDeclaration(d) && isGlobalScopeAugmentation(d)));
+        return symbol.declarations?.find(d => !isExternalModuleAugmentation(d) && !(isModuleDeclaration(d) && isGlobalScopeAugmentation(d)));
     }
 
     export function isEffectiveExternalModule(node: SourceFile, compilerOptions: CompilerOptions) {
@@ -4888,15 +4888,15 @@ namespace ts {
     }
 
     export function getLocalSymbolForExportDefault(symbol: Symbol) {
-        if (!isExportDefaultSymbol(symbol)) return undefined;
+        if (!isExportDefaultSymbol(symbol) || !symbol.declarations) return undefined;
         for (const decl of symbol.declarations) {
             if (decl.localSymbol) return decl.localSymbol;
         }
         return undefined;
     }
 
     function isExportDefaultSymbol(symbol: Symbol): boolean {
-        return symbol && length(symbol.declarations) > 0 && hasSyntacticModifier(symbol.declarations[0], ModifierFlags.Default);
+        return symbol && length(symbol.declarations) > 0 && hasSyntacticModifier(symbol.declarations![0], ModifierFlags.Default);
     }
 
     /** Return ".ts", ".d.ts", or ".tsx", if that is the extension. */
@@ -5445,7 +5445,7 @@ namespace ts {
     }
 
     export function getClassLikeDeclarationOfSymbol(symbol: Symbol): ClassLikeDeclaration | undefined {
-        return find(symbol.declarations, isClassLike);
+        return symbol.declarations?.find(isClassLike);
     }
 
     export function getObjectFlags(type: Type): ObjectFlags {

diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts
@@ -1003,8 +1003,8 @@ namespace FourSlash {
 
         private verifySymbol(symbol: ts.Symbol, declarationRanges: Range[]) {
             const { declarations } = symbol;
-            if (declarations.length !== declarationRanges.length) {
-                this.raiseError(`Expected to get ${declarationRanges.length} declarations, got ${declarations.length}`);
+            if (declarations?.length !== declarationRanges.length) {
+                this.raiseError(`Expected to get ${declarationRanges.length} declarations, got ${declarations?.length}`);
             }
 
             ts.zipWith(declarations, declarationRanges, (decl, range) => {

diff --git a/src/services/callHierarchy.ts b/src/services/callHierarchy.ts
@@ -181,7 +181,7 @@ namespace ts.CallHierarchy {
             const indices = indicesOf(symbol.declarations);
             const keys = map(symbol.declarations, decl => ({ file: decl.getSourceFile().fileName, pos: decl.pos }));
             indices.sort((a, b) => compareStringsCaseSensitive(keys[a].file, keys[b].file) || keys[a].pos - keys[b].pos);
-            const sortedDeclarations = map(indices, i => symbol.declarations[i]);
+            const sortedDeclarations = map(indices, i => symbol.declarations![i]);
             let lastDecl: CallHierarchyDeclaration | undefined;
             for (const decl of sortedDeclarations) {
                 if (isValidCallHierarchyDeclaration(decl)) {

diff --git a/src/services/codefixes/convertFunctionToEs6Class.ts b/src/services/codefixes/convertFunctionToEs6Class.ts
@@ -61,7 +61,7 @@ namespace ts.codefix {
             // all static members are stored in the "exports" array of symbol
             if (symbol.exports) {
                 symbol.exports.forEach(member => {
-                    if (member.name === "prototype") {
+                    if (member.name === "prototype" && member.declarations) {
                         const firstDeclaration = member.declarations[0];
                         // only one "x.prototype = { ... }" will pass
                         if (member.declarations.length === 1 &&

diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts
@@ -262,7 +262,7 @@ namespace ts.codefix {
             const superSymbol = superElement && checker.getSymbolAtLocation(superElement.expression);
             if (!superSymbol) break;
             const symbol = superSymbol.flags & SymbolFlags.Alias ? checker.getAliasedSymbol(superSymbol) : superSymbol;
-            const superDecl = find(symbol.declarations, isClassLike);
+            const superDecl = symbol.declarations && find(symbol.declarations, isClassLike);
             if (!superDecl) break;
             res.push(superDecl);
             decl = superDecl;

diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -890,7 +890,7 @@ namespace ts.Completions {
     }
 
     function isModuleSymbol(symbol: Symbol): boolean {
-        return symbol.declarations.some(d => d.kind === SyntaxKind.SourceFile);
+        return !!symbol.declarations?.some(d => d.kind === SyntaxKind.SourceFile);
     }
 
     function getCompletionData(
@@ -1240,7 +1240,7 @@ namespace ts.Completions {
                         const isValidAccess: (symbol: Symbol) => boolean =
                             isNamespaceName
                                 // At `namespace N.M/**/`, if this is the only declaration of `M`, don't include `M` as a completion.
-                                ? symbol => !!(symbol.flags & SymbolFlags.Namespace) && !symbol.declarations.every(d => d.parent === node.parent)
+                                ? symbol => !!(symbol.flags & SymbolFlags.Namespace) && !symbol.declarations?.every(d => d.parent === node.parent)
                                 : isRhsOfImportDeclaration ?
                                     // Any kind is allowed when dotting off namespace in internal import equals declaration
                                     symbol => isValidTypeAccess(symbol) || isValidValueAccess(symbol) :

diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts
@@ -839,7 +839,7 @@ namespace ts.FindAllReferences {
             }
 
             const exported = symbol.exports!.get(InternalSymbolName.ExportEquals);
-            if (exported) {
+            if (exported?.declarations) {
                 for (const decl of exported.declarations) {
                     const sourceFile = decl.getSourceFile();
                     if (sourceFilesSet.has(sourceFile.fileName)) {
@@ -916,7 +916,7 @@ namespace ts.FindAllReferences {
             const result: SymbolAndEntries[] = [];
             const state = new State(sourceFiles, sourceFilesSet, node ? getSpecialSearchKind(node) : SpecialSearchKind.None, checker, cancellationToken, searchMeaning, options, result);
 
-            const exportSpecifier = !isForRenameWithPrefixAndSuffixText(options) ? undefined : find(symbol.declarations, isExportSpecifier);
+            const exportSpecifier = !isForRenameWithPrefixAndSuffixText(options) || !symbol.declarations ? undefined : find(symbol.declarations, isExportSpecifier);
             if (exportSpecifier) {
                 // When renaming at an export specifier, rename the export and not the thing being exported.
                 getReferencesAtExportSpecifier(exportSpecifier.name, symbol, exportSpecifier, state.createSearch(node, originalSymbol, /*comingFrom*/ undefined), state, /*addReferencesHere*/ true, /*alwaysGetReferences*/ true);

diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts
@@ -146,7 +146,7 @@ namespace ts {
                 importLiteral => {
                     const importedModuleSymbol = program.getTypeChecker().getSymbolAtLocation(importLiteral);
                     // No need to update if it's an ambient module^M
-                    if (importedModuleSymbol && importedModuleSymbol.declarations.some(d => isAmbientModule(d))) return undefined;
+                    if (importedModuleSymbol?.declarations && importedModuleSymbol.declarations.some(d => isAmbientModule(d))) return undefined;
 
                     const toImport = oldFromNew !== undefined
                         // If we're at the new location (file was already renamed), need to redo module resolution starting from the old location.
@@ -185,7 +185,7 @@ namespace ts {
     ): ToImport | undefined {
         if (importedModuleSymbol) {
             // `find` should succeed because we checked for ambient modules before calling this function.
-            const oldFileName = find(importedModuleSymbol.declarations, isSourceFile)!.fileName;
+            const oldFileName = find(importedModuleSymbol.declarations!, isSourceFile)!.fileName;
             const newFileName = oldToNew(oldFileName);
             return newFileName === undefined ? { newFileName: oldFileName, updated: false } : { newFileName, updated: true };
         }

diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts
@@ -51,7 +51,7 @@ namespace ts.GoToDefinition {
         // assignment. This case and others are handled by the following code.
         if (node.parent.kind === SyntaxKind.ShorthandPropertyAssignment) {
             const shorthandSymbol = typeChecker.getShorthandAssignmentValueSymbol(symbol.valueDeclaration);
-            const definitions = shorthandSymbol ? shorthandSymbol.declarations.map(decl => createDefinitionInfo(decl, typeChecker, shorthandSymbol, node)) : emptyArray;
+            const definitions = shorthandSymbol?.declarations ? shorthandSymbol.declarations.map(decl => createDefinitionInfo(decl, typeChecker, shorthandSymbol, node)) : emptyArray;
             return concatenate(definitions, getDefinitionFromObjectLiteralElement(typeChecker, node) || emptyArray);
         }
 
@@ -206,7 +206,7 @@ namespace ts.GoToDefinition {
         // get the aliased symbol instead. This allows for goto def on an import e.g.
         //   import {A, B} from "mod";
         // to jump to the implementation directly.
-        if (symbol && symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) {
+        if (symbol?.declarations && symbol.flags & SymbolFlags.Alias && shouldSkipAlias(node, symbol.declarations[0])) {
             const aliased = checker.getAliasedSymbol(symbol);
             if (aliased.declarations) {
                 return aliased;
@@ -254,7 +254,7 @@ namespace ts.GoToDefinition {
             // Applicable only if we are in a new expression, or we are on a constructor declaration
             // and in either case the symbol has a construct signature definition, i.e. class
             if (symbol.flags & SymbolFlags.Class && !(symbol.flags & (SymbolFlags.Function | SymbolFlags.Variable)) && (isNewExpressionTarget(node) || node.kind === SyntaxKind.ConstructorKeyword)) {
-                const cls = find(filteredDeclarations, isClassLike) || Debug.fail("Expected declaration to have at least one class-like declaration");
+                const cls = find(filteredDeclarations!, isClassLike) || Debug.fail("Expected declaration to have at least one class-like declaration");
                 return getSignatureDefinition(cls.members, /*selectConstructors*/ true);
             }
         }

diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts
@@ -62,9 +62,11 @@ namespace ts.FindAllReferences {
             }
 
             // Module augmentations may use this module's exports without importing it.
-            for (const decl of exportingModuleSymbol.declarations) {
-                if (isExternalModuleAugmentation(decl) && sourceFilesSet.has(decl.getSourceFile().fileName)) {
-                    addIndirectUser(decl);
+            if (exportingModuleSymbol.declarations) {
+                for (const decl of exportingModuleSymbol.declarations) {
+                    if (isExternalModuleAugmentation(decl) && sourceFilesSet.has(decl.getSourceFile().fileName)) {
+                        addIndirectUser(decl);
+                    }
                 }
             }
 
@@ -468,7 +470,7 @@ namespace ts.FindAllReferences {
                 if (parent.kind === SyntaxKind.PropertyAccessExpression) {
                     // When accessing an export of a JS module, there's no alias. The symbol will still be flagged as an export even though we're at the use.
                     // So check that we are at the declaration.
-                    return symbol.declarations.some(d => d === parent) && isBinaryExpression(grandParent)
+                    return symbol.declarations?.some(d => d === parent) && isBinaryExpression(grandParent)
                         ? getSpecialPropertyExport(grandParent, /*useLhsSymbol*/ false)
                         : undefined;
                 }

diff --git a/src/services/refactors/convertOverloadListToSingleSignature.ts b/src/services/refactors/convertOverloadListToSingleSignature.ts
@@ -201,10 +201,10 @@ ${newComment.split("\n").map(c => ` * ${c}`).join("\n")}
         if (!every(decls, d => getSourceFileOfNode(d) === file)) {
             return;
         }
-        if (!isConvertableSignatureDeclaration(decls[0])) {
+        if (!isConvertableSignatureDeclaration(decls![0])) {
             return;
         }
-        const kindOne = decls[0].kind;
+        const kindOne = decls![0].kind;
         if (!every(decls, d => d.kind === kindOne)) {
             return;
         }

diff --git a/src/services/refactors/convertParamsToDestructuredObject.ts b/src/services/refactors/convertParamsToDestructuredObject.ts
@@ -360,7 +360,7 @@ namespace ts.refactor.convertParamsToDestructuredObject {
                 if (isObjectLiteralExpression(functionDeclaration.parent)) {
                     const contextualSymbol = getSymbolForContextualType(functionDeclaration.name, checker);
                     // don't offer the refactor when there are multiple signatures since we won't know which ones the user wants to change
-                    return contextualSymbol?.declarations.length === 1 && isSingleImplementation(functionDeclaration, checker);
+                    return contextualSymbol?.declarations?.length === 1 && isSingleImplementation(functionDeclaration, checker);
                 }
                 return isSingleImplementation(functionDeclaration, checker);
             case SyntaxKind.Constructor:

diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts
@@ -145,7 +145,7 @@ namespace ts.refactor {
             if (isTypeReferenceNode(node)) {
                 if (isIdentifier(node.typeName)) {
                     const symbol = checker.resolveName(node.typeName.text, node.typeName, SymbolFlags.TypeParameter, /* excludeGlobals */ true);
-                    if (symbol) {
+                    if (symbol?.declarations) {
                         const declaration = cast(first(symbol.declarations), isTypeParameterDeclaration);
                         if (rangeContainsSkipTrivia(statement, declaration, file) && !rangeContainsSkipTrivia(selection, declaration, file)) {
                             pushIfUnique(result, declaration);

diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts
@@ -443,6 +443,9 @@ namespace ts.refactor {
         const oldFileNamedImports: string[] = [];
         const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`.
         newFileImportsFromOldFile.forEach(symbol => {
+            if (!symbol.declarations) {
+                return;
+            }
             for (const decl of symbol.declarations) {
                 if (!isTopLevelDeclaration(decl)) continue;
                 const name = nameOfTopLevelDeclaration(decl);

diff --git a/src/services/rename.ts b/src/services/rename.ts
@@ -60,7 +60,7 @@ namespace ts.Rename {
             return getRenameInfoError(Diagnostics.You_cannot_rename_a_module_via_a_global_import);
         }
 
-        const moduleSourceFile = find(moduleSymbol.declarations, isSourceFile);
+        const moduleSourceFile = moduleSymbol.declarations && find(moduleSymbol.declarations, isSourceFile);
         if (!moduleSourceFile) return undefined;
         const withoutIndex = endsWith(node.text, "/index") || endsWith(node.text, "/index.js") ? undefined : tryRemoveSuffix(removeFileExtension(moduleSourceFile.fileName), "/index");
         const name = withoutIndex === undefined ? moduleSourceFile.fileName : withoutIndex;