Skip to content
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

Improve references search and quick info on properties with type errors within nullable contextual types #61383

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve references search and quick info on properties with type errors within nullable contextual types #61383

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions src/services/services.ts
View file
Original file line number Diff line number Diff line change
@@ -3558,6 +3558,7 @@ function getSymbolAtLocationForQuickInfo(node: Node, checker: TypeChecker): Symb
* @internal
*/
export function getPropertySymbolsFromContextualType(node: ObjectLiteralElementWithName, checker: TypeChecker, contextualType: Type, unionSymbolOk: boolean): readonly Symbol[] {
contextualType = contextualType.getNonNullableType();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just fine-tunes the algorithm for a common case. By discarding nullable (that can't provide anything useful here as they don't have properties anyway) the !contextualType.isUnion() "fast path" is taken here. This allows for better symbol result to be returned from here - despite those locations having errors. Without this the symbol of the anonymous object literal type (the one with error) is picked and that's less useful.

const name = getNameFromPropertyName(node.name);
if (!name) return emptyArray;
if (!contextualType.isUnion()) {
121 changes: 121 additions & 0 deletions tests/baselines/reference/findAllRefsFromContextualUnionType1.baseline.jsonc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// === findAllReferences ===
// === /tests/cases/fourslash/findAllRefsFromContextualUnionType1.ts ===
// function test1(arg: { <|[|prop|]: "foo"|> }) {}
// test1({ /*FIND ALL REFS*/<|[|{| isWriteAccess: true, isDefinition: true |}prop|]: "bar"|> });
//
// function test2(arg: { prop: "foo" } | undefined) {}
// test2({ prop: "bar" });

// === Definitions ===
// === /tests/cases/fourslash/findAllRefsFromContextualUnionType1.ts ===
// function test1(arg: { <|[|prop|]: "foo"|> }) {}
// test1({ /*FIND ALL REFS*/prop: "bar" });
//
// function test2(arg: { prop: "foo" } | undefined) {}
// test2({ prop: "bar" });

// === Details ===
[
{
"containerKind": "",
"containerName": "",
"kind": "property",
"name": "(property) prop: \"foo\"",
"displayParts": [
{
"text": "(",
"kind": "punctuation"
},
{
"text": "property",
"kind": "text"
},
{
"text": ")",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "prop",
"kind": "propertyName"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "\"foo\"",
"kind": "stringLiteral"
}
]
}
]



// === findAllReferences ===
// === /tests/cases/fourslash/findAllRefsFromContextualUnionType1.ts ===
// function test1(arg: { prop: "foo" }) {}
// test1({ prop: "bar" });
//
// function test2(arg: { <|[|prop|]: "foo"|> } | undefined) {}
// test2({ /*FIND ALL REFS*/<|[|{| isWriteAccess: true, isDefinition: true |}prop|]: "bar"|> });

// === Definitions ===
// === /tests/cases/fourslash/findAllRefsFromContextualUnionType1.ts ===
// function test1(arg: { prop: "foo" }) {}
// test1({ prop: "bar" });
//
// function test2(arg: { <|[|prop|]: "foo"|> } | undefined) {}
// test2({ /*FIND ALL REFS*/prop: "bar" });

// === Details ===
[
{
"containerKind": "",
"containerName": "",
"kind": "property",
"name": "(property) prop: \"foo\"",
"displayParts": [
{
"text": "(",
"kind": "punctuation"
},
{
"text": "property",
"kind": "text"
},
{
"text": ")",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "prop",
"kind": "propertyName"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "\"foo\"",
"kind": "stringLiteral"
}
]
}
]
10 changes: 10 additions & 0 deletions tests/cases/fourslash/findAllRefsFromContextualUnionType1.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @strict: true
//// function test1(arg: { prop: "foo" }) {}
//// test1({ /*1*/prop: "bar" });
////
//// function test2(arg: { prop: "foo" } | undefined) {}
//// test2({ /*2*/prop: "bar" });

verify.baselineFindAllReferences("1", "2");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/quickInfoFromContextualUnionType2.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

// @strict: true
//// function test1(arg: { prop: "foo" }) {}
//// test1({ /*1*/prop: "bar" });
////
//// function test2(arg: { prop: "foo" } | undefined) {}
//// test2({ /*2*/prop: "bar" });

verify.quickInfoAt("1", '(property) prop: "foo"');
verify.quickInfoAt("2", '(property) prop: "foo"');
33 changes: 33 additions & 0 deletions tests/cases/fourslash/quickInfoFromContextualUnionType3.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/// <reference path='fourslash.ts' />

// https://github.com/microsoft/TypeScript/issues/61382

// @strict: true
//// declare const foo1: <D extends Foo1<D>>(definition: D) => D;
////
//// type Foo1<D, Bar = Prop<D, "bar">> = {
//// bar: {
//// [K in keyof Bar]: Bar[K] extends boolean
//// ? Bar[K]
//// : "Error: bar should be boolean";
//// };
//// };
////
//// declare const foo2: <D extends Foo2<D>>(definition: D) => D;
////
//// type Foo2<D, Bar = Prop<D, "bar">> = {
//// bar?: {
//// [K in keyof Bar]: Bar[K] extends boolean
//// ? Bar[K]
//// : "Error: bar should be boolean";
//// };
//// };
////
//// type Prop<T, K> = K extends keyof T ? T[K] : never;
////
//// foo1({ bar: { /*1*/X: "test" } });
////
//// foo2({ bar: { /*2*/X: "test" } });

verify.quickInfoAt("1", '(property) X: "Error: bar should be boolean"');
verify.quickInfoAt("2", '(property) X: "Error: bar should be boolean"');