Skip to content

Commit 0285755

Browse files
author
Andy Hanson
committed
In getPropertySymbolsFromContextualType, use union discriminant to filter types
1 parent 23eb591 commit 0285755

6 files changed

+85
-38
lines changed

‎src/services/findAllReferences.ts

Lines changed: 2 additions & 12 deletions
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

Lines changed: 9 additions & 8 deletions
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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2187,28 +2187,48 @@ namespace ts {
21872187
*/
21882188
/* @internal */
21892189
export function getContainingObjectLiteralElement(node: Node): ObjectLiteralElementWithName | undefined {
2190+
const element = getContainingObjectLiteralElementWorker(node);
2191+
return element && (isObjectLiteralExpression(element.parent) || isJsxAttributes(element.parent)) ? element as ObjectLiteralElementWithName : undefined;
2192+
}
2193+
function getContainingObjectLiteralElementWorker(node: Node): ObjectLiteralElement | undefined {
21902194
switch (node.kind) {
21912195
case SyntaxKind.StringLiteral:
21922196
case SyntaxKind.NumericLiteral:
21932197
if (node.parent.kind === SyntaxKind.ComputedPropertyName) {
2194-
return isObjectLiteralElement(node.parent.parent) ? node.parent.parent as ObjectLiteralElementWithName : undefined;
2198+
return isObjectLiteralElement(node.parent.parent) ? node.parent.parent : undefined;
21952199
}
21962200
// falls through
21972201
case SyntaxKind.Identifier:
21982202
return isObjectLiteralElement(node.parent) &&
21992203
(node.parent.parent.kind === SyntaxKind.ObjectLiteralExpression || node.parent.parent.kind === SyntaxKind.JsxAttributes) &&
2200-
node.parent.name === node ? node.parent as ObjectLiteralElementWithName : undefined;
2204+
node.parent.name === node ? node.parent : undefined;
22012205
}
22022206
return undefined;
22032207
}
2208+
22042209
/* @internal */
2205-
export type ObjectLiteralElementWithName = ObjectLiteralElement & { name: PropertyName };
2210+
export type ObjectLiteralElementWithName = ObjectLiteralElement & { name: PropertyName; parent: ObjectLiteralExpression | JsxAttributes };
22062211

2212+
/** Gets all symbols for one property. Does not get symbols for every property. */
22072213
/* @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
2214+
export function getPropertySymbolsFromContextualType(node: ObjectLiteralElementWithName, checker: TypeChecker, contextualType: Type, unionSymbolOk: boolean): ReadonlyArray<Symbol> {
2215+
const name = getNameFromPropertyName(node.name);
2216+
if (!name) return emptyArray;
2217+
if (!contextualType.isUnion()) {
2218+
const symbol = contextualType.getProperty(name);
2219+
return symbol ? [symbol] : emptyArray;
2220+
}
2221+
2222+
const discriminatedPropertySymbols = mapDefined(contextualType.types, t => isObjectLiteralExpression(node.parent) && checker.isTypeInvalidDueToUnionDiscriminant(t, node.parent) ? undefined : t.getProperty(name));
2223+
if (unionSymbolOk && (discriminatedPropertySymbols.length === 0 || discriminatedPropertySymbols.length === contextualType.types.length)) {
2224+
const symbol = contextualType.getProperty(name);
2225+
if (symbol) return [symbol];
2226+
}
2227+
if (discriminatedPropertySymbols.length === 0) {
2228+
// Bogus discriminant -- do again without discriminating
2229+
return mapDefined(contextualType.types, t => t.getProperty(name));
2230+
}
2231+
return discriminatedPropertySymbols;
22122232
}
22132233

22142234
/* @internal */

‎tests/cases/fourslash/findAllRefsUnionProperty.ts

Lines changed: 17 additions & 9 deletions
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

Lines changed: 29 additions & 0 deletions
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/referencesForContextuallyTypedUnionProperties.ts

Lines changed: 1 addition & 2 deletions
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 },

0 commit comments

Comments
 (0)