Skip to content

Commit

Permalink
Add undefined to Symbol.valueDeclaration (#43033)
Browse files Browse the repository at this point in the history
* About halfway through the checker

I'm going to merge with master to avoid clashing with the declaration
fix.

* Add undefined to Symbol.valueDeclaration

Also add undefined to a number of utility functions that have always
accepted it, but never added it to their type.

* Fix lint from code review

Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>

* remove obsoleted fix from inferFromUsage

Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
  • Loading branch information
sandersn and DanielRosenwasser committed Mar 2, 2021
1 parent 3d1c6e8 commit c497b48
Show file tree
Hide file tree
Showing 18 changed files with 98 additions and 72 deletions.
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
Expand Up @@ -3187,7 +3187,7 @@ namespace ts {
undefined;
init = init && getRightMostAssignedExpression(init);
if (init) {
const isPrototypeAssignment = isPrototypeAccess(isVariableDeclaration(node) ? node.name : isBinaryExpression(node) ? node.left : node);
const isPrototypeAssignment = isPrototypeAccess(isVariableDeclaration(node!) ? node.name : isBinaryExpression(node!) ? node.left : node!);
return !!getExpandoInitializer(isBinaryExpression(init) && (init.operatorToken.kind === SyntaxKind.BarBarToken || init.operatorToken.kind === SyntaxKind.QuestionQuestionToken) ? init.right : init, isPrototypeAssignment);
}
return false;
Expand Down
99 changes: 59 additions & 40 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/compiler/factory/nodeFactory.ts
Expand Up @@ -5507,7 +5507,7 @@ namespace ts {
: reduceLeft(expressions, factory.createComma)!;
}

function getName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean, emitFlags: EmitFlags = 0) {
function getName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean, emitFlags: EmitFlags = 0) {
const nodeName = getNameOfDeclaration(node);
if (nodeName && isIdentifier(nodeName) && !isGeneratedIdentifier(nodeName)) {
// TODO(rbuckton): Does this need to be parented?
Expand Down Expand Up @@ -5571,7 +5571,7 @@ namespace ts {
* @param allowComments A value indicating whether comments may be emitted for the name.
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getDeclarationName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean) {
function getDeclarationName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean) {
return getName(node, allowComments, allowSourceMaps);
}

Expand Down
3 changes: 3 additions & 0 deletions src/compiler/moduleSpecifiers.ts
Expand Up @@ -103,6 +103,9 @@ namespace ts.moduleSpecifiers {

const info = getInfo(importingSourceFile.path, host);
const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration || getNonAugmentationDeclaration(moduleSymbol));
if (!moduleSourceFile) {
return [];
}
const modulePaths = getAllModulePaths(importingSourceFile.path, moduleSourceFile.originalFileName, host);
const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/declarations.ts
Expand Up @@ -1202,7 +1202,7 @@ namespace ts {
fakespace.symbol = props[0].parent!;
const exportMappings: [Identifier, string][] = [];
let declarations: (VariableStatement | ExportDeclaration)[] = mapDefined(props, p => {
if (!isPropertyAccessExpression(p.valueDeclaration)) {
if (!p.valueDeclaration || !isPropertyAccessExpression(p.valueDeclaration)) {
return undefined; // TODO GH#33569: Handle element access expressions that created late bound names (rather than silently omitting them)
}
getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(p.valueDeclaration);
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/types.ts
Expand Up @@ -4046,7 +4046,7 @@ namespace ts {
* The function returns the value (local variable) symbol of an identifier in the short-hand property assignment.
* This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value.
*/
getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined;
getShorthandAssignmentValueSymbol(location: Node | undefined): Symbol | undefined;

getExportSpecifierLocalTargetSymbol(location: ExportSpecifier | Identifier): Symbol | undefined;
/**
Expand Down Expand Up @@ -4715,7 +4715,7 @@ namespace ts {
flags: SymbolFlags; // Symbol flags
escapedName: __String; // Name of symbol
declarations?: Declaration[]; // Declarations associated with this symbol
valueDeclaration: Declaration; // First value declaration of the symbol
valueDeclaration?: Declaration; // First value declaration of the symbol
members?: SymbolTable; // Class, interface or object literal instance members
exports?: SymbolTable; // Module exports
globalExports?: SymbolTable; // Conditional global UMD exports
Expand Down Expand Up @@ -7394,7 +7394,7 @@ namespace ts {
* @param allowComments A value indicating whether comments may be emitted for the name.
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
/* @internal */ getDeclarationName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean): Identifier;
/* @internal */ getDeclarationName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean): Identifier;
/**
* Gets a namespace-qualified name for use in expressions.
*
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/utilities.ts
Expand Up @@ -734,9 +734,9 @@ namespace ts {
return isShorthandAmbientModule(moduleSymbol.valueDeclaration);
}

function isShorthandAmbientModule(node: Node): boolean {
function isShorthandAmbientModule(node: Node | undefined): boolean {
// The only kind of module that can be missing a body is a shorthand ambient module.
return node && node.kind === SyntaxKind.ModuleDeclaration && (!(<ModuleDeclaration>node).body);
return !!node && node.kind === SyntaxKind.ModuleDeclaration && (!(<ModuleDeclaration>node).body);
}

export function isBlockScopedContainerTopLevel(node: Node): boolean {
Expand Down
6 changes: 3 additions & 3 deletions src/compiler/utilitiesPublic.ts
Expand Up @@ -614,7 +614,7 @@ namespace ts {
return (declaration as NamedDeclaration).name;
}

export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
export function getNameOfDeclaration(declaration: Declaration | Expression | undefined): DeclarationName | undefined {
if (declaration === undefined) return undefined;
return getNonAssignedNameOfDeclaration(declaration) ||
(isFunctionExpression(declaration) || isClassExpression(declaration) ? getAssignedName(declaration) : undefined);
Expand Down Expand Up @@ -1208,8 +1208,8 @@ namespace ts {

// Functions

export function isFunctionLike(node: Node): node is SignatureDeclaration {
return node && isFunctionLikeKind(node.kind);
export function isFunctionLike(node: Node | undefined): node is SignatureDeclaration {
return !!node && isFunctionLikeKind(node.kind);
}

/* @internal */
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/convertConstToLet.ts
Expand Up @@ -18,7 +18,7 @@ namespace ts.codefix {
const token = getTokenAtPosition(sourceFile, pos);
const checker = program.getTypeChecker();
const symbol = checker.getSymbolAtLocation(token);
if (symbol) {
if (symbol?.valueDeclaration) {
return symbol.valueDeclaration.parent.parent as VariableStatement;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/services/codefixes/convertFunctionToEs6Class.ts
Expand Up @@ -16,7 +16,7 @@ namespace ts.codefix {

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, checker: TypeChecker, preferences: UserPreferences, compilerOptions: CompilerOptions): void {
const ctorSymbol = checker.getSymbolAtLocation(getTokenAtPosition(sourceFile, position))!;
if (!ctorSymbol || !(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) {
if (!ctorSymbol || !ctorSymbol.valueDeclaration || !(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) {
// Bad input
return undefined;
}
Expand Down Expand Up @@ -46,7 +46,7 @@ namespace ts.codefix {
// all instance members are stored in the "member" array of symbol
if (symbol.members) {
symbol.members.forEach((member, key) => {
if (key === "constructor") {
if (key === "constructor" && member.valueDeclaration) {
// fn.prototype.constructor = fn
changes.delete(sourceFile, member.valueDeclaration.parent);
return;
Expand Down
5 changes: 4 additions & 1 deletion src/services/codefixes/convertToAsyncFunction.ts
Expand Up @@ -168,7 +168,10 @@ namespace ts.codefix {
// so we push an entry for 'response'.
if (lastCallSignature && !isParameter(node.parent) && !isFunctionLikeDeclaration(node.parent) && !synthNamesMap.has(symbolIdString)) {
const firstParameter = firstOrUndefined(lastCallSignature.parameters);
const ident = firstParameter && isParameter(firstParameter.valueDeclaration) && tryCast(firstParameter.valueDeclaration.name, isIdentifier) || factory.createUniqueName("result", GeneratedIdentifierFlags.Optimistic);
const ident = firstParameter?.valueDeclaration
&& isParameter(firstParameter.valueDeclaration)
&& tryCast(firstParameter.valueDeclaration.name, isIdentifier)
|| factory.createUniqueName("result", GeneratedIdentifierFlags.Optimistic);
const synthName = getNewNameIfConflict(ident, collidingSymbolMap);
synthNamesMap.set(symbolIdString, synthName);
collidingSymbolMap.add(ident.text, symbol);
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/fixSpelling.ts
Expand Up @@ -87,7 +87,7 @@ namespace ts.codefix {
const suggestion = symbolName(suggestedSymbol);
if (!isIdentifierText(suggestion, target) && isPropertyAccessExpression(node.parent)) {
const valDecl = suggestedSymbol.valueDeclaration;
if (isNamedDeclaration(valDecl) && isPrivateIdentifier(valDecl.name)) {
if (valDecl && isNamedDeclaration(valDecl) && isPrivateIdentifier(valDecl.name)) {
changes.replaceNode(sourceFile, node, factory.createIdentifier(suggestion));
}
else {
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/importFixes.ts
Expand Up @@ -1025,7 +1025,7 @@ namespace ts.codefix {
}

function allowsImportingAmbientModule(moduleSymbol: Symbol): boolean {
if (!packageJsons.length) {
if (!packageJsons.length || !moduleSymbol.valueDeclaration) {
return true;
}

Expand Down
7 changes: 4 additions & 3 deletions src/services/codefixes/inferFromUsage.ts
Expand Up @@ -950,7 +950,7 @@ namespace ts.codefix {
const props = createMultiMap<Type>();
for (const anon of anons) {
for (const p of checker.getPropertiesOfType(anon)) {
props.add(p.name, checker.getTypeOfSymbolAtLocation(p, p.valueDeclaration));
props.add(p.name, p.valueDeclaration ? checker.getTypeOfSymbolAtLocation(p, p.valueDeclaration) : checker.getAnyType());
}
calls.push(...checker.getSignaturesOfType(anon, SignatureKind.Call));
constructs.push(...checker.getSignaturesOfType(anon, SignatureKind.Construct));
Expand Down Expand Up @@ -1104,12 +1104,13 @@ namespace ts.codefix {
if (!usageParam) {
break;
}
let genericParamType = checker.getTypeOfSymbolAtLocation(genericParam, genericParam.valueDeclaration);
let genericParamType = genericParam.valueDeclaration ? checker.getTypeOfSymbolAtLocation(genericParam, genericParam.valueDeclaration) : checker.getAnyType();
const elementType = isRest && checker.getElementTypeOfArrayType(genericParamType);
if (elementType) {
genericParamType = elementType;
}
const targetType = (usageParam as SymbolLinks).type || checker.getTypeOfSymbolAtLocation(usageParam, usageParam.valueDeclaration);
const targetType = (usageParam as SymbolLinks).type
|| (usageParam.valueDeclaration ? checker.getTypeOfSymbolAtLocation(usageParam, usageParam.valueDeclaration) : checker.getAnyType());
types.push(...inferTypeParameters(genericParamType, targetType, typeParameter));
}
const genericReturn = checker.getReturnTypeOfSignature(genericSig);
Expand Down
4 changes: 2 additions & 2 deletions src/services/importTracker.ts
Expand Up @@ -360,7 +360,7 @@ namespace ts.FindAllReferences {
const checker = program.getTypeChecker();
for (const referencingFile of sourceFiles) {
const searchSourceFile = searchModuleSymbol.valueDeclaration;
if (searchSourceFile.kind === SyntaxKind.SourceFile) {
if (searchSourceFile?.kind === SyntaxKind.SourceFile) {
for (const ref of referencingFile.referencedFiles) {
if (program.getSourceFileFromReference(referencingFile, ref) === searchSourceFile) {
refs.push({ kind: "reference", referencingFile, ref });
Expand Down Expand Up @@ -582,7 +582,7 @@ namespace ts.FindAllReferences {
return Debug.checkDefined(checker.getImmediateAliasedSymbol(importedSymbol));
}

const decl = importedSymbol.valueDeclaration;
const decl = Debug.checkDefined(importedSymbol.valueDeclaration);
if (isExportAssignment(decl)) { // `export = class {}`
return Debug.checkDefined(decl.expression.symbol);
}
Expand Down
2 changes: 1 addition & 1 deletion src/services/refactors/extractType.ts
Expand Up @@ -168,7 +168,7 @@ namespace ts.refactor {
else if (isTypeQueryNode(node)) {
if (isIdentifier(node.exprName)) {
const symbol = checker.resolveName(node.exprName.text, node.exprName, SymbolFlags.Value, /* excludeGlobals */ false);
if (symbol && rangeContainsSkipTrivia(statement, symbol.valueDeclaration, file) && !rangeContainsSkipTrivia(selection, symbol.valueDeclaration, file)) {
if (symbol?.valueDeclaration && rangeContainsSkipTrivia(statement, symbol.valueDeclaration, file) && !rangeContainsSkipTrivia(selection, symbol.valueDeclaration, file)) {
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Expand Up @@ -2187,7 +2187,7 @@ declare namespace ts {
* The function returns the value (local variable) symbol of an identifier in the short-hand property assignment.
* This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value.
*/
getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined;
getShorthandAssignmentValueSymbol(location: Node | undefined): Symbol | undefined;
getExportSpecifierLocalTargetSymbol(location: ExportSpecifier | Identifier): Symbol | undefined;
/**
* If a symbol is a local symbol with an associated exported symbol, returns the exported symbol.
Expand Down Expand Up @@ -2409,7 +2409,7 @@ declare namespace ts {
flags: SymbolFlags;
escapedName: __String;
declarations?: Declaration[];
valueDeclaration: Declaration;
valueDeclaration?: Declaration;
members?: SymbolTable;
exports?: SymbolTable;
globalExports?: SymbolTable;
Expand Down Expand Up @@ -4107,7 +4107,7 @@ declare namespace ts {
function idText(identifierOrPrivateName: Identifier | PrivateIdentifier): string;
function symbolName(symbol: Symbol): string;
function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | PrivateIdentifier | undefined;
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
function getNameOfDeclaration(declaration: Declaration | Expression | undefined): DeclarationName | undefined;
/**
* Gets the JSDoc parameter tags for the node if present.
*
Expand Down Expand Up @@ -4233,7 +4233,7 @@ declare namespace ts {
function isEntityName(node: Node): node is EntityName;
function isPropertyName(node: Node): node is PropertyName;
function isBindingName(node: Node): node is BindingName;
function isFunctionLike(node: Node): node is SignatureDeclaration;
function isFunctionLike(node: Node | undefined): node is SignatureDeclaration;
function isClassElement(node: Node): node is ClassElement;
function isClassLike(node: Node): node is ClassLikeDeclaration;
function isAccessor(node: Node): node is AccessorDeclaration;
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/api/typescript.d.ts
Expand Up @@ -2187,7 +2187,7 @@ declare namespace ts {
* The function returns the value (local variable) symbol of an identifier in the short-hand property assignment.
* This is necessary as an identifier in short-hand property assignment can contains two meaning: property name and property value.
*/
getShorthandAssignmentValueSymbol(location: Node): Symbol | undefined;
getShorthandAssignmentValueSymbol(location: Node | undefined): Symbol | undefined;
getExportSpecifierLocalTargetSymbol(location: ExportSpecifier | Identifier): Symbol | undefined;
/**
* If a symbol is a local symbol with an associated exported symbol, returns the exported symbol.
Expand Down Expand Up @@ -2409,7 +2409,7 @@ declare namespace ts {
flags: SymbolFlags;
escapedName: __String;
declarations?: Declaration[];
valueDeclaration: Declaration;
valueDeclaration?: Declaration;
members?: SymbolTable;
exports?: SymbolTable;
globalExports?: SymbolTable;
Expand Down Expand Up @@ -4107,7 +4107,7 @@ declare namespace ts {
function idText(identifierOrPrivateName: Identifier | PrivateIdentifier): string;
function symbolName(symbol: Symbol): string;
function getNameOfJSDocTypedef(declaration: JSDocTypedefTag): Identifier | PrivateIdentifier | undefined;
function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined;
function getNameOfDeclaration(declaration: Declaration | Expression | undefined): DeclarationName | undefined;
/**
* Gets the JSDoc parameter tags for the node if present.
*
Expand Down Expand Up @@ -4233,7 +4233,7 @@ declare namespace ts {
function isEntityName(node: Node): node is EntityName;
function isPropertyName(node: Node): node is PropertyName;
function isBindingName(node: Node): node is BindingName;
function isFunctionLike(node: Node): node is SignatureDeclaration;
function isFunctionLike(node: Node | undefined): node is SignatureDeclaration;
function isClassElement(node: Node): node is ClassElement;
function isClassLike(node: Node): node is ClassLikeDeclaration;
function isAccessor(node: Node): node is AccessorDeclaration;
Expand Down

0 comments on commit c497b48

Please sign in to comment.