Skip to content

Commit 168186f

Browse files
andrewbranchandrewbranch
authoredSep 20, 2022
Allow a union property of a private/protected member and an intersection property including that same member (#50328)
·
v5.8.2v4.9-beta
1 parent 812ebcf commit 168186f

6 files changed

+297
-2
lines changed
 

‎src/compiler/checker.ts

+29-2
Original file line numberDiff line numberDiff line change
@@ -12531,7 +12531,7 @@ namespace ts {
1253112531
const isInstantiation = (getTargetSymbol(prop) || prop) === (getTargetSymbol(singleProp) || singleProp);
1253212532
// If the symbols are instances of one another with identical types - consider the symbols
1253312533
// equivalent and just use the first one, which thus allows us to avoid eliding private
12534-
// members when intersecting a (this-)instantiations of a class with it's raw base or another instance
12534+
// members when intersecting a (this-)instantiations of a class with its raw base or another instance
1253512535
if (isInstantiation && compareProperties(singleProp, prop, (a, b) => a === b ? Ternary.True : Ternary.False) === Ternary.True) {
1253612536
// If we merged instantiations of a generic type, we replicate the symbol parent resetting behavior we used
1253712537
// to do when we recorded multiple distinct symbols so that we still get, eg, `Array<T>.length` printed
@@ -12579,7 +12579,12 @@ namespace ts {
1257912579
}
1258012580
}
1258112581
}
12582-
if (!singleProp || isUnion && (propSet || checkFlags & CheckFlags.Partial) && checkFlags & (CheckFlags.ContainsPrivate | CheckFlags.ContainsProtected)) {
12582+
if (!singleProp ||
12583+
isUnion &&
12584+
(propSet || checkFlags & CheckFlags.Partial) &&
12585+
checkFlags & (CheckFlags.ContainsPrivate | CheckFlags.ContainsProtected) &&
12586+
!(propSet && getCommonDeclarationsOfSymbols(arrayFrom(propSet.values())))
12587+
) {
1258312588
// No property was found, or, in a union, a property has a private or protected declaration in one
1258412589
// constituent, but is missing or has a different declaration in another constituent.
1258512590
return undefined;
@@ -12685,6 +12690,28 @@ namespace ts {
1268512690
return property;
1268612691
}
1268712692

12693+
function getCommonDeclarationsOfSymbols(symbols: readonly Symbol[]) {
12694+
let commonDeclarations: Set<Node> | undefined;
12695+
for (const symbol of symbols) {
12696+
if (!symbol.declarations) {
12697+
return undefined;
12698+
}
12699+
if (!commonDeclarations) {
12700+
commonDeclarations = new Set(symbol.declarations);
12701+
continue;
12702+
}
12703+
commonDeclarations.forEach(declaration => {
12704+
if (!contains(symbol.declarations, declaration)) {
12705+
commonDeclarations!.delete(declaration);
12706+
}
12707+
});
12708+
if (commonDeclarations.size === 0) {
12709+
return undefined;
12710+
}
12711+
}
12712+
return commonDeclarations;
12713+
}
12714+
1268812715
function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
1268912716
const property = getUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment);
1269012717
// We need to filter out partial properties in union types

‎tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.errors.txt

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts(19,23): error TS2339: Property 'foo' does not exist on type 'Foo | Bar'.
2+
3+
4+
==== tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts (1 errors) ====
5+
class Foo {
6+
protected foo = 0;
7+
}
8+
9+
class Bar {
10+
protected foo = 0;
11+
}
12+
13+
type Nothing<V extends Foo> = void;
14+
15+
type Broken<V extends Array<Foo | Bar>> = {
16+
readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
17+
};
18+
19+
// The issue above, #49517, is fixed very indirectly. Here's some code
20+
// that shows the direct result of the change:
21+
22+
type _3 = (Foo & Bar)['foo']; // Ok
23+
type _4 = (Foo | Bar)['foo']; // Error
24+
~~~~~
25+
!!! error TS2339: Property 'foo' does not exist on type 'Foo | Bar'.
26+
type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
27+
28+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
29+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
30+
// it passes the regular assignability check but then goes into intersection
31+
// property checks. To pull `foo` from the substitution type, it gets the
32+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
33+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
34+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
35+
// is a synthesized intersection property with a declaration in `Foo` and a
36+
// declaration in `Bar`. Because the former was marked as protected and the
37+
// latter was a different symbol, we previously thought the two symbols were
38+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
39+
// check not that the two property symbols are identical, but that they share
40+
// some common declaration. The change is directly observable by seeing whether
41+
// `(Foo | (Foo & Bar))['foo']` is allowed.
42+

‎tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.js

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
//// [unionPropertyOfProtectedAndIntersectionProperty.ts]
2+
class Foo {
3+
protected foo = 0;
4+
}
5+
6+
class Bar {
7+
protected foo = 0;
8+
}
9+
10+
type Nothing<V extends Foo> = void;
11+
12+
type Broken<V extends Array<Foo | Bar>> = {
13+
readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
14+
};
15+
16+
// The issue above, #49517, is fixed very indirectly. Here's some code
17+
// that shows the direct result of the change:
18+
19+
type _3 = (Foo & Bar)['foo']; // Ok
20+
type _4 = (Foo | Bar)['foo']; // Error
21+
type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
22+
23+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
24+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
25+
// it passes the regular assignability check but then goes into intersection
26+
// property checks. To pull `foo` from the substitution type, it gets the
27+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
28+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
29+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
30+
// is a synthesized intersection property with a declaration in `Foo` and a
31+
// declaration in `Bar`. Because the former was marked as protected and the
32+
// latter was a different symbol, we previously thought the two symbols were
33+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
34+
// check not that the two property symbols are identical, but that they share
35+
// some common declaration. The change is directly observable by seeing whether
36+
// `(Foo | (Foo & Bar))['foo']` is allowed.
37+
38+
39+
//// [unionPropertyOfProtectedAndIntersectionProperty.js]
40+
var Foo = /** @class */ (function () {
41+
function Foo() {
42+
this.foo = 0;
43+
}
44+
return Foo;
45+
}());
46+
var Bar = /** @class */ (function () {
47+
function Bar() {
48+
this.foo = 0;
49+
}
50+
return Bar;
51+
}());
52+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
53+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
54+
// it passes the regular assignability check but then goes into intersection
55+
// property checks. To pull `foo` from the substitution type, it gets the
56+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
57+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
58+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
59+
// is a synthesized intersection property with a declaration in `Foo` and a
60+
// declaration in `Bar`. Because the former was marked as protected and the
61+
// latter was a different symbol, we previously thought the two symbols were
62+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
63+
// check not that the two property symbols are identical, but that they share
64+
// some common declaration. The change is directly observable by seeing whether
65+
// `(Foo | (Foo & Bar))['foo']` is allowed.

‎tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.symbols

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
=== tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts ===
2+
class Foo {
3+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
4+
5+
protected foo = 0;
6+
>foo : Symbol(Foo.foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 11))
7+
}
8+
9+
class Bar {
10+
>Bar : Symbol(Bar, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 2, 1))
11+
12+
protected foo = 0;
13+
>foo : Symbol(Bar.foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 4, 11))
14+
}
15+
16+
type Nothing<V extends Foo> = void;
17+
>Nothing : Symbol(Nothing, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 6, 1))
18+
>V : Symbol(V, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 8, 13))
19+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
20+
21+
type Broken<V extends Array<Foo | Bar>> = {
22+
>Broken : Symbol(Broken, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 8, 35))
23+
>V : Symbol(V, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 10, 12))
24+
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
25+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
26+
>Bar : Symbol(Bar, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 2, 1))
27+
28+
readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
29+
>P : Symbol(P, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 11, 12))
30+
>V : Symbol(V, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 10, 12))
31+
>V : Symbol(V, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 10, 12))
32+
>P : Symbol(P, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 11, 12))
33+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
34+
>Nothing : Symbol(Nothing, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 6, 1))
35+
>V : Symbol(V, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 10, 12))
36+
>P : Symbol(P, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 11, 12))
37+
38+
};
39+
40+
// The issue above, #49517, is fixed very indirectly. Here's some code
41+
// that shows the direct result of the change:
42+
43+
type _3 = (Foo & Bar)['foo']; // Ok
44+
>_3 : Symbol(_3, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 12, 2))
45+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
46+
>Bar : Symbol(Bar, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 2, 1))
47+
48+
type _4 = (Foo | Bar)['foo']; // Error
49+
>_4 : Symbol(_4, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 17, 29))
50+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
51+
>Bar : Symbol(Bar, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 2, 1))
52+
53+
type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
54+
>_5 : Symbol(_5, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 18, 29))
55+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
56+
>Foo : Symbol(Foo, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 0, 0))
57+
>Bar : Symbol(Bar, Decl(unionPropertyOfProtectedAndIntersectionProperty.ts, 2, 1))
58+
59+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
60+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
61+
// it passes the regular assignability check but then goes into intersection
62+
// property checks. To pull `foo` from the substitution type, it gets the
63+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
64+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
65+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
66+
// is a synthesized intersection property with a declaration in `Foo` and a
67+
// declaration in `Bar`. Because the former was marked as protected and the
68+
// latter was a different symbol, we previously thought the two symbols were
69+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
70+
// check not that the two property symbols are identical, but that they share
71+
// some common declaration. The change is directly observable by seeing whether
72+
// `(Foo | (Foo & Bar))['foo']` is allowed.
73+

‎tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.types

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
=== tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts ===
2+
class Foo {
3+
>Foo : Foo
4+
5+
protected foo = 0;
6+
>foo : number
7+
>0 : 0
8+
}
9+
10+
class Bar {
11+
>Bar : Bar
12+
13+
protected foo = 0;
14+
>foo : number
15+
>0 : 0
16+
}
17+
18+
type Nothing<V extends Foo> = void;
19+
>Nothing : void
20+
21+
type Broken<V extends Array<Foo | Bar>> = {
22+
>Broken : Broken<V>
23+
24+
readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
25+
};
26+
27+
// The issue above, #49517, is fixed very indirectly. Here's some code
28+
// that shows the direct result of the change:
29+
30+
type _3 = (Foo & Bar)['foo']; // Ok
31+
>_3 : number
32+
33+
type _4 = (Foo | Bar)['foo']; // Error
34+
>_4 : any
35+
36+
type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
37+
>_5 : number
38+
39+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
40+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
41+
// it passes the regular assignability check but then goes into intersection
42+
// property checks. To pull `foo` from the substitution type, it gets the
43+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
44+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
45+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
46+
// is a synthesized intersection property with a declaration in `Foo` and a
47+
// declaration in `Bar`. Because the former was marked as protected and the
48+
// latter was a different symbol, we previously thought the two symbols were
49+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
50+
// check not that the two property symbols are identical, but that they share
51+
// some common declaration. The change is directly observable by seeing whether
52+
// `(Foo | (Foo & Bar))['foo']` is allowed.
53+

‎tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
class Foo {
2+
protected foo = 0;
3+
}
4+
5+
class Bar {
6+
protected foo = 0;
7+
}
8+
9+
type Nothing<V extends Foo> = void;
10+
11+
type Broken<V extends Array<Foo | Bar>> = {
12+
readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
13+
};
14+
15+
// The issue above, #49517, is fixed very indirectly. Here's some code
16+
// that shows the direct result of the change:
17+
18+
type _3 = (Foo & Bar)['foo']; // Ok
19+
type _4 = (Foo | Bar)['foo']; // Error
20+
type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
21+
22+
// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
23+
// checking if that's assignable to `Foo` in the constraint of `Nothing`,
24+
// it passes the regular assignability check but then goes into intersection
25+
// property checks. To pull `foo` from the substitution type, it gets the
26+
// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
27+
// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
28+
// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
29+
// is a synthesized intersection property with a declaration in `Foo` and a
30+
// declaration in `Bar`. Because the former was marked as protected and the
31+
// latter was a different symbol, we previously thought the two symbols were
32+
// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
33+
// check not that the two property symbols are identical, but that they share
34+
// some common declaration. The change is directly observable by seeing whether
35+
// `(Foo | (Foo & Bar))['foo']` is allowed.

0 commit comments

Comments
 (0)
Please sign in to comment.