Make go to definition go to the constraint's properties for object literals in argument positions #62361
+206
−13
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { | |
| CallLikeExpression, | ||
| canHaveSymbol, | ||
| concatenate, | ||
| ContextFlags, | ||
| createTextSpan, | ||
| createTextSpanFromBounds, | ||
| createTextSpanFromNode, | ||
|
|
@@ -72,6 +73,8 @@ import { | |
| isNameOfFunctionDeclaration, | ||
| isNewExpressionTarget, | ||
| isObjectBindingPattern, | ||
| isObjectLiteralElementLike, | ||
| isObjectLiteralExpression, | ||
| isPropertyName, | ||
| isRightSideOfPropertyAccess, | ||
| isStaticModifier, | ||
|
|
@@ -312,7 +315,20 @@ function getDefinitionFromObjectLiteralElement(typeChecker: TypeChecker, node: N | |
| if (element) { | ||
| const contextualType = element && typeChecker.getContextualType(element.parent); | ||
| if (contextualType) { | ||
| let properties = getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false); | ||
| if (properties.length === 1) { | ||
|
Member
There was a problem hiding this comment. Is there no way that this would ever apply if there were more than one symbol here? Like, can the definition be both the constraint but also something else via a union?
Contributor
Author
There was a problem hiding this comment. It could come from multiple sources. This is just me using a conservative approach that covers the majority of use cases. I don't see a reason why it wouldn't be OK to change the algorithm to:
This should leave all potential meaningful definitions in the result. WDYT? A simple playground for experimenting with how TypeScript behaves today in presence of unions: TS playground
Member
There was a problem hiding this comment. I feel like it must be better to not hardcode "exactly one property", but then again I don't know what the change I suggested looks like in code. |
||
| const declaration = properties[0].valueDeclaration; | ||
| const withoutNodeInferencesType = declaration && isObjectLiteralExpression(declaration.parent) && isObjectLiteralElementLike(declaration) && declaration.name === node ? | ||
| typeChecker.getContextualType(element.parent, ContextFlags.IgnoreNodeInferences) : | ||
| undefined; | ||
| if (withoutNodeInferencesType) { | ||
| const withoutNodeInferencesProperties = getPropertySymbolsFromContextualType(element, typeChecker, withoutNodeInferencesType, /*unionSymbolOk*/ false); | ||
| if (withoutNodeInferencesProperties.length) { | ||
| properties = withoutNodeInferencesProperties; | ||
| } | ||
| } | ||
| } | ||
| return flatMap(properties, propertySymbol => getDefinitionFromSymbol(typeChecker, propertySymbol, node)); | ||
| } | ||
| } | ||
| return emptyArray; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| // === goToDefinition === | ||
| // === /tests/cases/fourslash/goToDefinitionObjectLiteralProperties2.ts === | ||
| // type C = { | ||
| // <|[|foo|]: string;|> | ||
| // bar: number; | ||
| // }; | ||
| // | ||
| // declare function fn<T extends C>(arg: T): T; | ||
| // | ||
| // fn({ | ||
| // foo/*GOTO DEF*/: "", | ||
| // bar: true, | ||
| // }); | ||
| // | ||
| // --- (line: 13) skipped --- | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "foo", | ||
| "containerName": "__type", | ||
| "isLocal": false, | ||
| "isAmbient": false, | ||
| "unverified": false | ||
| } | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| // === goToDefinition === | ||
| // === /tests/cases/fourslash/goToDefinitionObjectLiteralProperties2.ts === | ||
| // type C = { | ||
| // foo: string; | ||
| // <|[|bar|]: number;|> | ||
| // }; | ||
| // | ||
| // declare function fn<T extends C>(arg: T): T; | ||
| // | ||
| // fn({ | ||
| // foo: "", | ||
| // bar/*GOTO DEF*/: true, | ||
| // }); | ||
| // | ||
| // const result = fn({ | ||
| // --- (line: 14) skipped --- | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "bar", | ||
| "containerName": "__type", | ||
| "isLocal": false, | ||
| "isAmbient": false, | ||
| "unverified": false | ||
| } | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| // === goToDefinition === | ||
| // === /tests/cases/fourslash/goToDefinitionObjectLiteralProperties2.ts === | ||
| // type C = { | ||
| // <|[|foo|]: string;|> | ||
| // bar: number; | ||
| // }; | ||
| // | ||
| // --- (line: 6) skipped --- | ||
|
|
||
| // --- (line: 10) skipped --- | ||
| // }); | ||
| // | ||
| // const result = fn({ | ||
| // foo/*GOTO DEF*/: "", | ||
| // bar: 1, | ||
| // }); | ||
| // | ||
| // // this one shouldn't go to the constraint type | ||
| // result.foo; | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "foo", | ||
| "containerName": "__type", | ||
| "isLocal": false, | ||
| "isAmbient": false, | ||
| "unverified": false | ||
| } | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| // === goToDefinition === | ||
| // === /tests/cases/fourslash/goToDefinitionObjectLiteralProperties2.ts === | ||
| // type C = { | ||
| // foo: string; | ||
| // <|[|bar|]: number;|> | ||
| // }; | ||
| // | ||
| // declare function fn<T extends C>(arg: T): T; | ||
| // --- (line: 7) skipped --- | ||
|
|
||
| // --- (line: 11) skipped --- | ||
| // | ||
| // const result = fn({ | ||
| // foo: "", | ||
| // bar/*GOTO DEF*/: 1, | ||
| // }); | ||
| // | ||
| // // this one shouldn't go to the constraint type | ||
| // result.foo; | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "bar", | ||
| "containerName": "__type", | ||
| "isLocal": false, | ||
| "isAmbient": false, | ||
| "unverified": false | ||
| } | ||
| ] | ||
|
|
||
|
|
||
|
|
||
| // === goToDefinition === | ||
| // === /tests/cases/fourslash/goToDefinitionObjectLiteralProperties2.ts === | ||
| // --- (line: 10) skipped --- | ||
| // }); | ||
| // | ||
| // const result = fn({ | ||
| // <|[|foo|]: ""|>, | ||
| // bar: 1, | ||
| // }); | ||
| // | ||
| // // this one shouldn't go to the constraint type | ||
| // result.foo/*GOTO DEF*/; | ||
|
|
||
| // === Details === | ||
| [ | ||
| { | ||
| "kind": "property", | ||
| "name": "foo", | ||
| "containerName": "__object", | ||
| "isLocal": false, | ||
| "isAmbient": false, | ||
| "unverified": false, | ||
| "failedAliasResolution": false | ||
| } | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /// <reference path='fourslash.ts'/> | ||
|
|
||
| //// type C = { | ||
| //// foo: string; | ||
| //// bar: number; | ||
| //// }; | ||
| //// | ||
| //// declare function fn<T extends C>(arg: T): T; | ||
| //// | ||
| //// fn({ | ||
| //// foo/*1*/: "", | ||
| //// bar/*2*/: true, | ||
| //// }); | ||
| //// | ||
| //// const result = fn({ | ||
| //// foo/*3*/: "", | ||
| //// bar/*4*/: 1, | ||
| //// }); | ||
| //// | ||
| //// // this one shouldn't go to the constraint type | ||
| //// result.foo/*5*/; | ||
|
|
||
| verify.baselineGoToDefinition("1", "2", "3", "4", "5"); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As shown by this PR, this mechanism is handy for other things than just completions - so I decided to rename it.