Skip to content

Commit dd933f4

Browse files
weswighamweswigham
authored
Fix get symbol at location to behave correctly for parameter assignments and jsx attributes (#20706)
* Fix get symbol at location to behave correctly got parameter assignments and jsx attributes * Handle all those edge cases that weren;t explicitly handled * Fix part of bug WRT getTypeAtLocation and for loops * Baseline corrections pursuant to #20710 * Restore jsdoc tag interpretation * Clean up some code, revert fourslash tests * Cleanup types used by PR feedback
1 parent a3ab1ff commit dd933f4

File tree

59 files changed

+513
-296
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+513
-296
lines changed

‎src/compiler/checker.ts

Lines changed: 64 additions & 41 deletions
Large diffs are not rendered by default.

‎src/compiler/declarationEmitter.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ namespace ts {
6464
let currentIdentifiers: Map<string>;
6565
let isCurrentFileExternalModule: boolean;
6666
let reportedDeclarationError = false;
67-
let errorNameNode: DeclarationName;
67+
let errorNameNode: DeclarationName | QualifiedName;
6868
const emitJsDocComments = compilerOptions.removeComments ? noop : writeJsDocComments;
6969
const emit = compilerOptions.stripInternal ? stripInternal : emitNode;
7070
let needsDeclare = true;
@@ -1372,7 +1372,7 @@ namespace ts {
13721372
// if this is property of type literal,
13731373
// or is parameter of method/call/construct/index signature of type literal
13741374
// emit only if type is specified
1375-
if (node.type) {
1375+
if (hasType(node)) {
13761376
write(": ");
13771377
emitType(node.type);
13781378
}

‎src/compiler/parser.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,20 +88,48 @@ namespace ts {
8888
case SyntaxKind.SpreadAssignment:
8989
return visitNode(cbNode, (<SpreadAssignment>node).expression);
9090
case SyntaxKind.Parameter:
91+
return visitNodes(cbNode, cbNodes, node.decorators) ||
92+
visitNodes(cbNode, cbNodes, node.modifiers) ||
93+
visitNode(cbNode, (<ParameterDeclaration>node).dotDotDotToken) ||
94+
visitNode(cbNode, (<ParameterDeclaration>node).name) ||
95+
visitNode(cbNode, (<ParameterDeclaration>node).questionToken) ||
96+
visitNode(cbNode, (<ParameterDeclaration>node).type) ||
97+
visitNode(cbNode, (<ParameterDeclaration>node).initializer);
9198
case SyntaxKind.PropertyDeclaration:
99+
return visitNodes(cbNode, cbNodes, node.decorators) ||
100+
visitNodes(cbNode, cbNodes, node.modifiers) ||
101+
visitNode(cbNode, (<PropertyDeclaration>node).name) ||
102+
visitNode(cbNode, (<PropertyDeclaration>node).questionToken) ||
103+
visitNode(cbNode, (<PropertyDeclaration>node).exclamationToken) ||
104+
visitNode(cbNode, (<PropertyDeclaration>node).type) ||
105+
visitNode(cbNode, (<PropertyDeclaration>node).initializer);
92106
case SyntaxKind.PropertySignature:
107+
return visitNodes(cbNode, cbNodes, node.decorators) ||
108+
visitNodes(cbNode, cbNodes, node.modifiers) ||
109+
visitNode(cbNode, (<PropertySignature>node).name) ||
110+
visitNode(cbNode, (<PropertySignature>node).questionToken) ||
111+
visitNode(cbNode, (<PropertySignature>node).type) ||
112+
visitNode(cbNode, (<PropertySignature>node).initializer);
93113
case SyntaxKind.PropertyAssignment:
114+
return visitNodes(cbNode, cbNodes, node.decorators) ||
115+
visitNodes(cbNode, cbNodes, node.modifiers) ||
116+
visitNode(cbNode, (<PropertyAssignment>node).name) ||
117+
visitNode(cbNode, (<PropertyAssignment>node).questionToken) ||
118+
visitNode(cbNode, (<PropertyAssignment>node).initializer);
94119
case SyntaxKind.VariableDeclaration:
120+
return visitNodes(cbNode, cbNodes, node.decorators) ||
121+
visitNodes(cbNode, cbNodes, node.modifiers) ||
122+
visitNode(cbNode, (<VariableDeclaration>node).name) ||
123+
visitNode(cbNode, (<VariableDeclaration>node).exclamationToken) ||
124+
visitNode(cbNode, (<VariableDeclaration>node).type) ||
125+
visitNode(cbNode, (<VariableDeclaration>node).initializer);
95126
case SyntaxKind.BindingElement:
96127
return visitNodes(cbNode, cbNodes, node.decorators) ||
97128
visitNodes(cbNode, cbNodes, node.modifiers) ||
98-
visitNode(cbNode, (<VariableLikeDeclaration>node).propertyName) ||
99-
visitNode(cbNode, (<VariableLikeDeclaration>node).dotDotDotToken) ||
100-
visitNode(cbNode, (<VariableLikeDeclaration>node).name) ||
101-
visitNode(cbNode, (<VariableLikeDeclaration>node).questionToken) ||
102-
visitNode(cbNode, (<VariableLikeDeclaration>node).exclamationToken) ||
103-
visitNode(cbNode, (<VariableLikeDeclaration>node).type) ||
104-
visitNode(cbNode, (<VariableLikeDeclaration>node).initializer);
129+
visitNode(cbNode, (<BindingElement>node).propertyName) ||
130+
visitNode(cbNode, (<BindingElement>node).dotDotDotToken) ||
131+
visitNode(cbNode, (<BindingElement>node).name) ||
132+
visitNode(cbNode, (<BindingElement>node).initializer);
105133
case SyntaxKind.FunctionType:
106134
case SyntaxKind.ConstructorType:
107135
case SyntaxKind.CallSignature:

‎src/compiler/types.ts

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,40 @@ namespace ts {
584584
| JSDocFunctionType
585585
| EndOfFileToken;
586586

587+
export type HasType =
588+
| SignatureDeclaration
589+
| VariableDeclaration
590+
| ParameterDeclaration
591+
| PropertySignature
592+
| PropertyDeclaration
593+
| TypePredicateNode
594+
| ParenthesizedTypeNode
595+
| TypeOperatorNode
596+
| MappedTypeNode
597+
| AssertionExpression
598+
| TypeAliasDeclaration
599+
| JSDocTypeExpression
600+
| JSDocNonNullableType
601+
| JSDocNullableType
602+
| JSDocOptionalType
603+
| JSDocVariadicType;
604+
605+
export type HasInitializer =
606+
| HasExpressionInitializer
607+
| ForStatement
608+
| ForInStatement
609+
| ForOfStatement
610+
| JsxAttribute;
611+
612+
export type HasExpressionInitializer =
613+
| VariableDeclaration
614+
| ParameterDeclaration
615+
| BindingElement
616+
| PropertySignature
617+
| PropertyDeclaration
618+
| PropertyAssignment
619+
| EnumMember;
620+
587621
/* @internal */
588622
export type MutableNodeArray<T extends Node> = NodeArray<T> & T[];
589623

@@ -793,6 +827,9 @@ namespace ts {
793827
initializer?: Expression; // Optional initializer
794828
}
795829

830+
/*@internal*/
831+
export type BindingElementGrandparent = BindingElement["parent"]["parent"];
832+
796833
export interface PropertySignature extends TypeElement, JSDocContainer {
797834
kind: SyntaxKind.PropertySignature;
798835
name: PropertyName; // Declared property name
@@ -848,25 +885,18 @@ namespace ts {
848885
expression: Expression;
849886
}
850887

851-
// SyntaxKind.VariableDeclaration
852-
// SyntaxKind.Parameter
853-
// SyntaxKind.BindingElement
854-
// SyntaxKind.Property
855-
// SyntaxKind.PropertyAssignment
856-
// SyntaxKind.JsxAttribute
857-
// SyntaxKind.ShorthandPropertyAssignment
858-
// SyntaxKind.EnumMember
859-
// SyntaxKind.JSDocPropertyTag
860-
// SyntaxKind.JSDocParameterTag
861-
export interface VariableLikeDeclaration extends NamedDeclaration {
862-
propertyName?: PropertyName;
863-
dotDotDotToken?: DotDotDotToken;
864-
name: DeclarationName;
865-
questionToken?: QuestionToken;
866-
exclamationToken?: ExclamationToken;
867-
type?: TypeNode;
868-
initializer?: Expression;
869-
}
888+
export type VariableLikeDeclaration =
889+
| VariableDeclaration
890+
| ParameterDeclaration
891+
| BindingElement
892+
| PropertyDeclaration
893+
| PropertyAssignment
894+
| PropertySignature
895+
| JsxAttribute
896+
| ShorthandPropertyAssignment
897+
| EnumMember
898+
| JSDocPropertyTag
899+
| JSDocParameterTag;
870900

871901
export interface PropertyLikeDeclaration extends NamedDeclaration {
872902
name: PropertyName;

‎src/compiler/utilities.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ namespace ts {
548548
// Return display name of an identifier
549549
// Computed property names will just be emitted as "[<expr>]", where <expr> is the source
550550
// text of the expression in the computed property.
551-
export function declarationNameToString(name: DeclarationName) {
551+
export function declarationNameToString(name: DeclarationName | QualifiedName) {
552552
return getFullWidth(name) === 0 ? "(Missing)" : getTextOfNode(name);
553553
}
554554

@@ -784,7 +784,7 @@ namespace ts {
784784
case SyntaxKind.PropertySignature:
785785
case SyntaxKind.Parameter:
786786
case SyntaxKind.VariableDeclaration:
787-
return node === (<VariableLikeDeclaration>parent).type;
787+
return node === (parent as HasType).type;
788788
case SyntaxKind.FunctionDeclaration:
789789
case SyntaxKind.FunctionExpression:
790790
case SyntaxKind.ArrowFunction:
@@ -1340,7 +1340,7 @@ namespace ts {
13401340
case SyntaxKind.EnumMember:
13411341
case SyntaxKind.PropertyAssignment:
13421342
case SyntaxKind.BindingElement:
1343-
return (<VariableLikeDeclaration>parent).initializer === node;
1343+
return (parent as HasInitializer).initializer === node;
13441344
case SyntaxKind.ExpressionStatement:
13451345
case SyntaxKind.IfStatement:
13461346
case SyntaxKind.DoStatement:
@@ -1650,7 +1650,7 @@ namespace ts {
16501650
result = addRange(result, getJSDocParameterTags(node as ParameterDeclaration));
16511651
}
16521652

1653-
if (isVariableLike(node) && node.initializer && hasJSDocNodes(node.initializer)) {
1653+
if (isVariableLike(node) && hasInitializer(node) && hasJSDocNodes(node.initializer)) {
16541654
result = addRange(result, node.initializer.jsDoc);
16551655
}
16561656

@@ -2816,8 +2816,8 @@ namespace ts {
28162816
* Gets the effective type annotation of a variable, parameter, or property. If the node was
28172817
* parsed in a JavaScript file, gets the type annotation from JSDoc.
28182818
*/
2819-
export function getEffectiveTypeAnnotationNode(node: VariableLikeDeclaration, checkJSDoc?: boolean): TypeNode | undefined {
2820-
if (node.type) {
2819+
export function getEffectiveTypeAnnotationNode(node: Node, checkJSDoc?: boolean): TypeNode | undefined {
2820+
if (hasType(node)) {
28212821
return node.type;
28222822
}
28232823
if (checkJSDoc || isInJavaScriptFile(node)) {
@@ -5812,4 +5812,22 @@ namespace ts {
58125812
export function hasJSDocNodes(node: Node): node is HasJSDoc {
58135813
return !!(node as JSDocContainer).jsDoc && (node as JSDocContainer).jsDoc.length > 0;
58145814
}
5815+
5816+
/** True if has type node attached to it. */
5817+
/* @internal */
5818+
export function hasType(node: Node): node is HasType {
5819+
return !!(node as HasType).type;
5820+
}
5821+
5822+
/** True if has initializer node attached to it. */
5823+
/* @internal */
5824+
export function hasInitializer(node: Node): node is HasInitializer {
5825+
return !!(node as HasInitializer).initializer;
5826+
}
5827+
5828+
/** True if has initializer node attached to it. */
5829+
/* @internal */
5830+
export function hasOnlyExpressionInitializer(node: Node): node is HasExpressionInitializer {
5831+
return hasInitializer(node) && !isForStatement(node) && !isForInStatement(node) && !isForOfStatement(node) && !isJsxAttribute(node);
5832+
}
58155833
}

‎src/services/completions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1325,7 +1325,7 @@ namespace ts.Completions {
13251325
// through type declaration or inference.
13261326
// Also proceed if rootDeclaration is a parameter and if its containing function expression/arrow function is contextually typed -
13271327
// type of parameter will flow in from the contextual type of the function
1328-
let canGetType = rootDeclaration.initializer || rootDeclaration.type || rootDeclaration.parent.parent.kind === SyntaxKind.ForOfStatement;
1328+
let canGetType = hasInitializer(rootDeclaration) || hasType(rootDeclaration) || rootDeclaration.parent.parent.kind === SyntaxKind.ForOfStatement;
13291329
if (!canGetType && rootDeclaration.kind === SyntaxKind.Parameter) {
13301330
if (isExpression(rootDeclaration.parent)) {
13311331
canGetType = !!typeChecker.getContextualType(<Expression>rootDeclaration.parent);

‎src/services/findAllReferences.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,7 +1060,7 @@ namespace ts.FindAllReferences.Core {
10601060
const containingTypeReference = getContainingTypeReference(refNode);
10611061
if (containingTypeReference && state.markSeenContainingTypeReference(containingTypeReference)) {
10621062
const parent = containingTypeReference.parent;
1063-
if (isVariableLike(parent) && parent.type === containingTypeReference && parent.initializer && isImplementationExpression(parent.initializer)) {
1063+
if (hasType(parent) && parent.type === containingTypeReference && hasInitializer(parent) && isImplementationExpression(parent.initializer)) {
10641064
addReference(parent.initializer);
10651065
}
10661066
else if (isFunctionLike(parent) && parent.type === containingTypeReference && (parent as FunctionLikeDeclaration).body) {
@@ -1670,14 +1670,12 @@ namespace ts.FindAllReferences.Core {
16701670
if (!node) {
16711671
return false;
16721672
}
1673-
else if (isVariableLike(node)) {
1674-
if (node.initializer) {
1675-
return true;
1676-
}
1677-
else if (node.kind === SyntaxKind.VariableDeclaration) {
1678-
const parentStatement = getParentStatementOfVariableDeclaration(<VariableDeclaration>node);
1679-
return parentStatement && hasModifier(parentStatement, ModifierFlags.Ambient);
1680-
}
1673+
else if (isVariableLike(node) && hasInitializer(node)) {
1674+
return true;
1675+
}
1676+
else if (node.kind === SyntaxKind.VariableDeclaration) {
1677+
const parentStatement = getParentStatementOfVariableDeclaration(<VariableDeclaration>node);
1678+
return parentStatement && hasModifier(parentStatement, ModifierFlags.Ambient);
16811679
}
16821680
else if (isFunctionLike(node)) {
16831681
return !!(node as FunctionLikeDeclaration).body || hasModifier(node, ModifierFlags.Ambient);

‎tests/baselines/reference/ES5For-of31.types

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ var a: string, b: number;
55

66
for ({ a: b = 1, b: a = ""} of []) {
77
>{ a: b = 1, b: a = ""} : { a?: number; b?: string; }
8-
>a : undefined
8+
>a : number
99
>b = 1 : 1
1010
>b : number
1111
>1 : 1
12-
>b : undefined
12+
>b : string
1313
>a = "" : ""
1414
>a : string
1515
>"" : ""

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,9 @@ declare namespace ts {
453453
interface JSDocContainer {
454454
}
455455
type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | EndOfFileToken;
456+
type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
457+
type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
458+
type HasExpressionInitializer = VariableDeclaration | ParameterDeclaration | BindingElement | PropertySignature | PropertyDeclaration | PropertyAssignment | EnumMember;
456459
interface NodeArray<T extends Node> extends ReadonlyArray<T>, TextRange {
457460
hasTrailingComma?: boolean;
458461
}
@@ -604,15 +607,7 @@ declare namespace ts {
604607
kind: SyntaxKind.SpreadAssignment;
605608
expression: Expression;
606609
}
607-
interface VariableLikeDeclaration extends NamedDeclaration {
608-
propertyName?: PropertyName;
609-
dotDotDotToken?: DotDotDotToken;
610-
name: DeclarationName;
611-
questionToken?: QuestionToken;
612-
exclamationToken?: ExclamationToken;
613-
type?: TypeNode;
614-
initializer?: Expression;
615-
}
610+
type VariableLikeDeclaration = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyDeclaration | PropertyAssignment | PropertySignature | JsxAttribute | ShorthandPropertyAssignment | EnumMember | JSDocPropertyTag | JSDocParameterTag;
616611
interface PropertyLikeDeclaration extends NamedDeclaration {
617612
name: PropertyName;
618613
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,9 @@ declare namespace ts {
453453
interface JSDocContainer {
454454
}
455455
type HasJSDoc = ParameterDeclaration | CallSignatureDeclaration | ConstructSignatureDeclaration | MethodSignature | PropertySignature | ArrowFunction | ParenthesizedExpression | SpreadAssignment | ShorthandPropertyAssignment | PropertyAssignment | FunctionExpression | LabeledStatement | ExpressionStatement | VariableStatement | FunctionDeclaration | ConstructorDeclaration | MethodDeclaration | PropertyDeclaration | AccessorDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | EnumMember | EnumDeclaration | ModuleDeclaration | ImportEqualsDeclaration | IndexSignatureDeclaration | FunctionTypeNode | ConstructorTypeNode | JSDocFunctionType | EndOfFileToken;
456+
type HasType = SignatureDeclaration | VariableDeclaration | ParameterDeclaration | PropertySignature | PropertyDeclaration | TypePredicateNode | ParenthesizedTypeNode | TypeOperatorNode | MappedTypeNode | AssertionExpression | TypeAliasDeclaration | JSDocTypeExpression | JSDocNonNullableType | JSDocNullableType | JSDocOptionalType | JSDocVariadicType;
457+
type HasInitializer = HasExpressionInitializer | ForStatement | ForInStatement | ForOfStatement | JsxAttribute;
458+
type HasExpressionInitializer = VariableDeclaration | ParameterDeclaration | BindingElement | PropertySignature | PropertyDeclaration | PropertyAssignment | EnumMember;
456459
interface NodeArray<T extends Node> extends ReadonlyArray<T>, TextRange {
457460
hasTrailingComma?: boolean;
458461
}
@@ -604,15 +607,7 @@ declare namespace ts {
604607
kind: SyntaxKind.SpreadAssignment;
605608
expression: Expression;
606609
}
607-
interface VariableLikeDeclaration extends NamedDeclaration {
608-
propertyName?: PropertyName;
609-
dotDotDotToken?: DotDotDotToken;
610-
name: DeclarationName;
611-
questionToken?: QuestionToken;
612-
exclamationToken?: ExclamationToken;
613-
type?: TypeNode;
614-
initializer?: Expression;
615-
}
610+
type VariableLikeDeclaration = VariableDeclaration | ParameterDeclaration | BindingElement | PropertyDeclaration | PropertyAssignment | PropertySignature | JsxAttribute | ShorthandPropertyAssignment | EnumMember | JSDocPropertyTag | JSDocParameterTag;
616611
interface PropertyLikeDeclaration extends NamedDeclaration {
617612
name: PropertyName;
618613
}

0 commit comments

Comments
 (0)