Skip to content

Add undefined to Symbol.valueDeclaration #43033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 2, 2021
Merged

Add undefined to Symbol.valueDeclaration #43033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3ff11af
About halfway through the checker
sandersn Mar 1, 2021
6198101
Merge branch 'master' into add-undefined-to-valueDeclaration
sandersn Mar 1, 2021
7468f7e
Add undefined to Symbol.valueDeclaration
sandersn Mar 2, 2021
5360cc7
Fix lint from code review
sandersn Mar 2, 2021
c14118d
remove obsoleted fix from inferFromUsage
sandersn Mar 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/compiler/factory/nodeFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how I feel about changing the public API on these, but maybe it's fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine: they just make the function easier to call, and they're on functions that already return undefined or boolean, which implies (correctly) that they pass through undefined if you call them.

/**
* 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
View file
Open in desktop
Original file line number Diff line number Diff line change
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