Skip to content

Commit d60f498

Browse files
author
Andy
authored
In getPropertySymbolsFromContextualType, use union discriminant to filter types (microsoft#25914)
1 parent 9658b47 commit d60f498

9 files changed

+137
-57
lines changed

‎src/services/findAllReferences.ts

+2-12
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,8 @@ namespace ts.FindAllReferences.Core {
13941394
// If the location is in a context sensitive location (i.e. in an object literal) try
13951395
// to get a contextual type for it, and add the property symbol from the contextual
13961396
// type to the search set
1397-
const res = firstDefined(getPropertySymbolsFromContextualType(containingObjectLiteralElement, checker), fromRoot);
1397+
const contextualType = checker.getContextualType(containingObjectLiteralElement.parent);
1398+
const res = contextualType && firstDefined(getPropertySymbolsFromContextualType(containingObjectLiteralElement, checker, contextualType, /*unionSymbolOk*/ true), fromRoot);
13981399
if (res) return res;
13991400

14001401
// If the location is name of property symbol from object literal destructuring pattern
@@ -1462,17 +1463,6 @@ namespace ts.FindAllReferences.Core {
14621463
!(search.parents && !search.parents.some(parent => explicitlyInheritsFrom(rootSymbol.parent!, parent, state.inheritsFromCache, checker))));
14631464
}
14641465

1465-
/** Gets all symbols for one property. Does not get symbols for every property. */
1466-
function getPropertySymbolsFromContextualType(node: ObjectLiteralElementWithName, checker: TypeChecker): ReadonlyArray<Symbol> {
1467-
const contextualType = checker.getContextualType(<ObjectLiteralExpression>node.parent);
1468-
if (!contextualType) return emptyArray;
1469-
const name = getNameFromPropertyName(node.name);
1470-
if (!name) return emptyArray;
1471-
const symbol = contextualType.getProperty(name);
1472-
return symbol ? [symbol] :
1473-
contextualType.isUnion() ? mapDefined(contextualType.types, t => t.getProperty(name)) : emptyArray;
1474-
}
1475-
14761466
/**
14771467
* Given an initial searchMeaning, extracted from a location, widen the search scope based on the declarations
14781468
* of the corresponding symbol. e.g. if we are searching for "Foo" in value position, but "Foo" references a class

‎src/services/goToDefinition.ts

+9-8
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,9 @@ namespace ts.GoToDefinition {
6969
if (isPropertyName(node) && isBindingElement(parent) && isObjectBindingPattern(parent.parent) &&
7070
(node === (parent.propertyName || parent.name))) {
7171
const type = typeChecker.getTypeAtLocation(parent.parent);
72-
if (type) {
73-
const propSymbols = getPropertySymbolsFromType(type, node);
74-
if (propSymbols) {
75-
return flatMap(propSymbols, propSymbol => getDefinitionFromSymbol(typeChecker, propSymbol, node));
76-
}
72+
const propSymbols = getPropertySymbolsFromType(type, node);
73+
if (propSymbols) {
74+
return flatMap(propSymbols, propSymbol => getDefinitionFromSymbol(typeChecker, propSymbol, node));
7775
}
7876
}
7977

@@ -87,9 +85,12 @@ namespace ts.GoToDefinition {
8785
// function Foo(arg: Props) {}
8886
// Foo( { pr/*1*/op1: 10, prop2: true })
8987
const element = getContainingObjectLiteralElement(node);
90-
if (element && typeChecker.getContextualType(element.parent as Expression)) {
91-
return flatMap(getPropertySymbolsFromContextualType(typeChecker, element), propertySymbol =>
92-
getDefinitionFromSymbol(typeChecker, propertySymbol, node));
88+
if (element) {
89+
const contextualType = element && typeChecker.getContextualType(element.parent);
90+
if (contextualType) {
91+
return flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol =>
92+
getDefinitionFromSymbol(typeChecker, propertySymbol, node));
93+
}
9394
}
9495
return getDefinitionFromSymbol(typeChecker, symbol, node);
9596
}

‎src/services/services.ts

+39-20
Original file line numberDiff line numberDiff line change
@@ -1489,19 +1489,6 @@ namespace ts {
14891489
}
14901490
}
14911491

1492-
function getSymbolAtLocationForQuickInfo(node: Node, checker: TypeChecker): Symbol | undefined {
1493-
if ((isIdentifier(node) || isStringLiteral(node))
1494-
&& isPropertyAssignment(node.parent)
1495-
&& node.parent.name === node) {
1496-
const type = checker.getContextualType(node.parent.parent);
1497-
const property = type && checker.getPropertyOfType(type, getTextOfIdentifierOrLiteral(node));
1498-
if (property) {
1499-
return property;
1500-
}
1501-
}
1502-
return checker.getSymbolAtLocation(node);
1503-
}
1504-
15051492
/// Goto definition
15061493
function getDefinitionAtPosition(fileName: string, position: number): DefinitionInfo[] | undefined {
15071494
synchronizeHostData();
@@ -2187,28 +2174,60 @@ namespace ts {
21872174
*/
21882175
/* @internal */
21892176
export function getContainingObjectLiteralElement(node: Node): ObjectLiteralElementWithName | undefined {
2177+
const element = getContainingObjectLiteralElementWorker(node);
2178+
return element && (isObjectLiteralExpression(element.parent) || isJsxAttributes(element.parent)) ? element as ObjectLiteralElementWithName : undefined;
2179+
}
2180+
function getContainingObjectLiteralElementWorker(node: Node): ObjectLiteralElement | undefined {
21902181
switch (node.kind) {
21912182
case SyntaxKind.StringLiteral:
21922183
case SyntaxKind.NumericLiteral:
21932184
if (node.parent.kind === SyntaxKind.ComputedPropertyName) {
2194-
return isObjectLiteralElement(node.parent.parent) ? node.parent.parent as ObjectLiteralElementWithName : undefined;
2185+
return isObjectLiteralElement(node.parent.parent) ? node.parent.parent : undefined;
21952186
}
21962187
// falls through
21972188
case SyntaxKind.Identifier:
21982189
return isObjectLiteralElement(node.parent) &&
21992190
(node.parent.parent.kind === SyntaxKind.ObjectLiteralExpression || node.parent.parent.kind === SyntaxKind.JsxAttributes) &&
2200-
node.parent.name === node ? node.parent as ObjectLiteralElementWithName : undefined;
2191+
node.parent.name === node ? node.parent : undefined;
22012192
}
22022193
return undefined;
22032194
}
2195+
22042196
/* @internal */
2205-
export type ObjectLiteralElementWithName = ObjectLiteralElement & { name: PropertyName };
2197+
export type ObjectLiteralElementWithName = ObjectLiteralElement & { name: PropertyName; parent: ObjectLiteralExpression | JsxAttributes };
2198+
2199+
function getSymbolAtLocationForQuickInfo(node: Node, checker: TypeChecker): Symbol | undefined {
2200+
const object = getContainingObjectLiteralElement(node);
2201+
if (object) {
2202+
const contextualType = checker.getContextualType(object.parent);
2203+
const properties = contextualType && getPropertySymbolsFromContextualType(object, checker, contextualType, /*unionSymbolOk*/ false);
2204+
if (properties && properties.length === 1) {
2205+
return first(properties);
2206+
}
2207+
}
2208+
return checker.getSymbolAtLocation(node);
2209+
}
22062210

2211+
/** Gets all symbols for one property. Does not get symbols for every property. */
22072212
/* @internal */
2208-
export function getPropertySymbolsFromContextualType(typeChecker: TypeChecker, node: ObjectLiteralElement): Symbol[] {
2209-
const objectLiteral = <ObjectLiteralExpression | JsxAttributes>node.parent;
2210-
const contextualType = typeChecker.getContextualType(objectLiteral)!; // TODO: GH#18217
2211-
return getPropertySymbolsFromType(contextualType, node.name!)!; // TODO: GH#18217
2213+
export function getPropertySymbolsFromContextualType(node: ObjectLiteralElementWithName, checker: TypeChecker, contextualType: Type, unionSymbolOk: boolean): ReadonlyArray<Symbol> {
2214+
const name = getNameFromPropertyName(node.name);
2215+
if (!name) return emptyArray;
2216+
if (!contextualType.isUnion()) {
2217+
const symbol = contextualType.getProperty(name);
2218+
return symbol ? [symbol] : emptyArray;
2219+
}
2220+
2221+
const discriminatedPropertySymbols = mapDefined(contextualType.types, t => isObjectLiteralExpression(node.parent) && checker.isTypeInvalidDueToUnionDiscriminant(t, node.parent) ? undefined : t.getProperty(name));
2222+
if (unionSymbolOk && (discriminatedPropertySymbols.length === 0 || discriminatedPropertySymbols.length === contextualType.types.length)) {
2223+
const symbol = contextualType.getProperty(name);
2224+
if (symbol) return [symbol];
2225+
}
2226+
if (discriminatedPropertySymbols.length === 0) {
2227+
// Bad discriminant -- do again without discriminating
2228+
return mapDefined(contextualType.types, t => t.getProperty(name));
2229+
}
2230+
return discriminatedPropertySymbols;
22122231
}
22132232

22142233
/* @internal */

‎tests/cases/fourslash/findAllRefsUnionProperty.ts

+17-9
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,27 @@
11
/// <reference path='fourslash.ts'/>
22

33
////type T =
4-
//// | { [|{| "isWriteAccess": true, "isDefinition": true |}type|]: "a" }
5-
//// | { [|{| "isWriteAccess": true, "isDefinition": true |}type|]: "b" };
4+
//// | { [|{| "isWriteAccess": true, "isDefinition": true |}type|]: "a", [|{| "isWriteAccess": true, "isDefinition": true |}prop|]: number }
5+
//// | { [|{| "isWriteAccess": true, "isDefinition": true |}type|]: "b", [|{| "isWriteAccess": true, "isDefinition": true |}prop|]: string };
6+
////const tt: T = {
7+
//// [|{| "isWriteAccess": true, "isDefinition": true |}type|]: "a",
8+
//// [|{| "isWriteAccess": true, "isDefinition": true |}prop|]: 0,
9+
////};
610
////declare const t: T;
711
////if (t.[|type|] === "a") {
812
//// t.[|type|];
913
////} else {
1014
//// t.[|type|];
1115
////}
1216

13-
const ranges = test.ranges();
14-
const [r0, r1, r2, r3, r4] = ranges;
15-
verify.referenceGroups(ranges, [
16-
{ definition: { text: '(property) type: "a"', range: r0 }, ranges: [r0, r3] },
17-
{ definition: { text: '(property) type: "b"', range: r1 }, ranges: [r1, r4] },
18-
{ definition: { text: '(property) type: "a" | "b"', range: r0 }, ranges: [r2] },
19-
]);
17+
const [t0, p0, t1, p1, t2, p2, t3, t4, t5] = test.ranges();
18+
19+
const a = { definition: { text: '(property) type: "a"', range: t0 }, ranges: [t0, t2, t4] };
20+
const b = { definition: { text: '(property) type: "b"', range: t1 }, ranges: [t1, t5] };
21+
const ab = { definition: { text: '(property) type: "a" | "b"', range: t0 }, ranges: [t3] };
22+
verify.referenceGroups([t0, t1, t3, t4, t5], [a, b, ab]);
23+
verify.referenceGroups(t2, [a, ab]);
24+
25+
const p = { definition: "(property) prop: number", ranges: [p0, p2] };
26+
verify.referenceGroups([p0, p1], [p, { definition: "(property) prop: string", ranges: [p1] }]);
27+
verify.referenceGroups(p2, [p]);

‎tests/cases/fourslash/goToDefinitionUnionTypeProperty_discriminated.ts

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////type U = A | B;
4+
////
5+
////interface A {
6+
//// /*aKind*/kind: "a";
7+
//// /*aProp*/prop: number;
8+
////};
9+
////
10+
////interface B {
11+
//// /*bKind*/kind: "b";
12+
//// /*bProp*/prop: string;
13+
////}
14+
////
15+
////const u: U = {
16+
//// [|/*kind*/kind|]: "a",
17+
//// [|/*prop*/prop|]: 0,
18+
////};
19+
////const u2: U = {
20+
//// [|/*kindBogus*/kind|]: "bogus",
21+
//// [|/*propBogus*/prop|]: 0,
22+
////};
23+
24+
verify.goToDefinition({
25+
kind: "aKind",
26+
prop: "aProp",
27+
kindBogus: ["aKind", "bKind"],
28+
propBogus: ["aProp", "bProp"],
29+
});

‎tests/cases/fourslash/jsxGenericQuickInfo.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,5 @@
1818
//// var c = <Component items={[0, 1, 2]} render/*2*/Item={it/*3*/em => item.toFixed()}
1919
verify.quickInfoAt("0", "(property) Props<number>.renderItem: (item: number) => string");
2020
verify.quickInfoAt("1", "(parameter) item: number");
21-
verify.quickInfoAt("2", "(JSX attribute) renderItem: (item: number) => string");
21+
verify.quickInfoAt("2", "(JSX attribute) Props<number>.renderItem: (item: number) => string");
2222
verify.quickInfoAt("3", "(parameter) item: number");

‎tests/cases/fourslash/quickInfoUnion_discriminated.ts

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
// @Filename: quickInfoJsDocTags.ts
4+
////type U = A | B;
5+
////
6+
////interface A {
7+
//// /** Kind A */
8+
//// kind: "a";
9+
//// /** Prop A */
10+
//// prop: number;
11+
////}
12+
////
13+
////interface B {
14+
//// /** Kind B */
15+
//// kind: "b";
16+
//// /** Prop B */
17+
//// prop: string;
18+
////}
19+
////
20+
////const u: U = {
21+
//// /*uKind*/kind: "a",
22+
//// /*uProp*/prop: 0,
23+
////}
24+
////const u2: U = {
25+
//// /*u2Kind*/kind: "bogus",
26+
//// /*u2Prop*/prop: 1,
27+
////};
28+
29+
verify.quickInfos({
30+
uKind: ['(property) A.kind: "a"', "Kind A "],
31+
uProp: ["(property) A.prop: number", "Prop A "],
32+
u2Kind: '(property) kind: "bogus"',
33+
u2Prop: "(property) prop: number",
34+
});

‎tests/cases/fourslash/referencesForContextuallyTypedUnionProperties.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@
3434
////var u1 = { a: 0, b: 0, common: "" };
3535
////var u2 = { b: 0, common: 0 };
3636

37-
const all = test.ranges();
38-
const [aCommon, bCommon, ...unionRefs] = all;
37+
const [aCommon, bCommon, ...unionRefs] = test.ranges();
3938
verify.referenceGroups(aCommon, [
4039
{ definition: "(property) A.common: string", ranges: [aCommon] },
4140
{ definition: "(property) common: string | number", ranges: unionRefs },

‎tests/cases/fourslash/tsxQuickInfo4.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
//// }
2929

3030
//// function _buildMainButton({ onClick, children, className }: ButtonProps): JSX.Element {
31-
//// return(<button className={className} onClick={onClick}>{ children || 'MAIN BUTTON'}</button>);
31+
//// return(<button className={className} onClick={onClick}>{ children || 'MAIN BUTTON'}</button>);
3232
//// }
3333

3434
//// declare function buildMainLink({ to, children, className }: LinkProps): JSX.Element;
@@ -39,17 +39,17 @@
3939
//// );
4040
//// }
4141

42-
//// function buildSomeElement2(): JSX.Element {
42+
//// function buildSomeElement2(): JSX.Element {
4343
//// return (
4444
//// <MainB/*3*/utton onC/*4*/lick={()=>{}}>GO</MainButton>;
4545
//// );
4646
//// }
47-
//// let componenet = <MainButton onClick={()=>{}} ext/*5*/ra-prop>GO</MainButton>;
47+
//// let componenet = <MainButton onClick={()=>{}} ext/*5*/ra-prop>GO</MainButton>;
4848

4949
verify.quickInfos({
5050
1: "function MainButton(linkProps: LinkProps): any (+1 overload)",
51-
2: "(JSX attribute) to: string",
51+
2: "(JSX attribute) LinkProps.to: string",
5252
3: "function MainButton(buttonProps: ButtonProps): any (+1 overload)",
53-
4: "(JSX attribute) onClick: () => void",
53+
4: "(method) ButtonProps.onClick(event?: any): void",
5454
5: "(JSX attribute) extra-prop: true"
5555
});

0 commit comments

Comments
 (0)