Allow a union property of a private/protected member and an intersection property including that same member (#50328)

Author: andrewbranch

src/compiler/checker.ts

@@ -12531,7 +12531,7 @@ namespace ts {
                             const isInstantiation = (getTargetSymbol(prop) || prop) === (getTargetSymbol(singleProp) || singleProp);
                             // If the symbols are instances of one another with identical types - consider the symbols
                             // equivalent and just use the first one, which thus allows us to avoid eliding private
-                            // members when intersecting a (this-)instantiations of a class with it's raw base or another instance
+                            // members when intersecting a (this-)instantiations of a class with its raw base or another instance
                             if (isInstantiation && compareProperties(singleProp, prop, (a, b) => a === b ? Ternary.True : Ternary.False) === Ternary.True) {
                                 // If we merged instantiations of a generic type, we replicate the symbol parent resetting behavior we used
                                 // to do when we recorded multiple distinct symbols so that we still get, eg, `Array<T>.length` printed
@@ -12579,7 +12579,12 @@ namespace ts {
                     }
                 }
             }
-            if (!singleProp || isUnion && (propSet || checkFlags & CheckFlags.Partial) && checkFlags & (CheckFlags.ContainsPrivate | CheckFlags.ContainsProtected)) {
+            if (!singleProp ||
+                isUnion &&
+                (propSet || checkFlags & CheckFlags.Partial) &&
+                checkFlags & (CheckFlags.ContainsPrivate | CheckFlags.ContainsProtected) &&
+                !(propSet && getCommonDeclarationsOfSymbols(arrayFrom(propSet.values())))
+            ) {
                 // No property was found, or, in a union, a property has a private or protected declaration in one
                 // constituent, but is missing or has a different declaration in another constituent.
                 return undefined;
@@ -12685,6 +12690,28 @@ namespace ts {
             return property;
         }
 
+        function getCommonDeclarationsOfSymbols(symbols: readonly Symbol[]) {
+            let commonDeclarations: Set<Node> | undefined;
+            for (const symbol of symbols) {
+                if (!symbol.declarations) {
+                    return undefined;
+                }
+                if (!commonDeclarations) {
+                    commonDeclarations = new Set(symbol.declarations);
+                    continue;
+                }
+                commonDeclarations.forEach(declaration => {
+                    if (!contains(symbol.declarations, declaration)) {
+                        commonDeclarations!.delete(declaration);
+                    }
+                });
+                if (commonDeclarations.size === 0) {
+                    return undefined;
+                }
+            }
+            return commonDeclarations;
+        }
+
         function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
             const property = getUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment);
             // We need to filter out partial properties in union types

tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.errors.txt

@@ -0,0 +1,42 @@
+tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts(19,23): error TS2339: Property 'foo' does not exist on type 'Foo | Bar'.
+
+
+==== tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts (1 errors) ====
+    class Foo {
+      protected foo = 0;
+    }
+    
+    class Bar {
+      protected foo = 0;
+    }
+    
+    type Nothing<V extends Foo> = void;
+    
+    type Broken<V extends Array<Foo | Bar>> = {
+      readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
+    };
+    
+    // The issue above, #49517, is fixed very indirectly. Here's some code
+    // that shows the direct result of the change:
+    
+    type _3 = (Foo & Bar)['foo'];         // Ok
+    type _4 = (Foo | Bar)['foo'];         // Error
+                          ~~~~~
+!!! error TS2339: Property 'foo' does not exist on type 'Foo | Bar'.
+    type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
+    
+    // V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
+    // checking if that's assignable to `Foo` in the constraint of `Nothing`,
+    // it passes the regular assignability check but then goes into intersection
+    // property checks. To pull `foo` from the substitution type, it gets the
+    // apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
+    // where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
+    // Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
+    // is a synthesized intersection property with a declaration in `Foo` and a
+    // declaration in `Bar`. Because the former was marked as protected and the
+    // latter was a different symbol, we previously thought the two symbols were
+    // totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
+    // check not that the two property symbols are identical, but that they share
+    // some common declaration. The change is directly observable by seeing whether
+    // `(Foo | (Foo & Bar))['foo']` is allowed.
+    

tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.js

@@ -0,0 +1,65 @@
+//// [unionPropertyOfProtectedAndIntersectionProperty.ts]
+class Foo {
+  protected foo = 0;
+}
+
+class Bar {
+  protected foo = 0;
+}
+
+type Nothing<V extends Foo> = void;
+
+type Broken<V extends Array<Foo | Bar>> = {
+  readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
+};
+
+// The issue above, #49517, is fixed very indirectly. Here's some code
+// that shows the direct result of the change:
+
+type _3 = (Foo & Bar)['foo'];         // Ok
+type _4 = (Foo | Bar)['foo'];         // Error
+type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
+
+// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
+// checking if that's assignable to `Foo` in the constraint of `Nothing`,
+// it passes the regular assignability check but then goes into intersection
+// property checks. To pull `foo` from the substitution type, it gets the
+// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
+// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
+// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
+// is a synthesized intersection property with a declaration in `Foo` and a
+// declaration in `Bar`. Because the former was marked as protected and the
+// latter was a different symbol, we previously thought the two symbols were
+// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
+// check not that the two property symbols are identical, but that they share
+// some common declaration. The change is directly observable by seeing whether
+// `(Foo | (Foo & Bar))['foo']` is allowed.
+
+
+//// [unionPropertyOfProtectedAndIntersectionProperty.js]
+var Foo = /** @class */ (function () {
+    function Foo() {
+        this.foo = 0;
+    }
+    return Foo;
+}());
+var Bar = /** @class */ (function () {
+    function Bar() {
+        this.foo = 0;
+    }
+    return Bar;
+}());
+// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
+// checking if that's assignable to `Foo` in the constraint of `Nothing`,
+// it passes the regular assignability check but then goes into intersection
+// property checks. To pull `foo` from the substitution type, it gets the
+// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
+// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
+// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
+// is a synthesized intersection property with a declaration in `Foo` and a
+// declaration in `Bar`. Because the former was marked as protected and the
+// latter was a different symbol, we previously thought the two symbols were
+// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
+// check not that the two property symbols are identical, but that they share
+// some common declaration. The change is directly observable by seeing whether
+// `(Foo | (Foo & Bar))['foo']` is allowed.

tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.symbols

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

tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.types

@@ -0,0 +1,53 @@
+=== tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts ===
+class Foo {
+>Foo : Foo
+
+  protected foo = 0;
+>foo : number
+>0 : 0
+}
+
+class Bar {
+>Bar : Bar
+
+  protected foo = 0;
+>foo : number
+>0 : 0
+}
+
+type Nothing<V extends Foo> = void;
+>Nothing : void
+
+type Broken<V extends Array<Foo | Bar>> = {
+>Broken : Broken<V>
+
+  readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
+};
+
+// The issue above, #49517, is fixed very indirectly. Here's some code
+// that shows the direct result of the change:
+
+type _3 = (Foo & Bar)['foo'];         // Ok
+>_3 : number
+
+type _4 = (Foo | Bar)['foo'];         // Error
+>_4 : any
+
+type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
+>_5 : number
+
+// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
+// checking if that's assignable to `Foo` in the constraint of `Nothing`,
+// it passes the regular assignability check but then goes into intersection
+// property checks. To pull `foo` from the substitution type, it gets the
+// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
+// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
+// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
+// is a synthesized intersection property with a declaration in `Foo` and a
+// declaration in `Bar`. Because the former was marked as protected and the
+// latter was a different symbol, we previously thought the two symbols were
+// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
+// check not that the two property symbols are identical, but that they share
+// some common declaration. The change is directly observable by seeing whether
+// `(Foo | (Foo & Bar))['foo']` is allowed.
+

tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts

@@ -0,0 +1,35 @@
+class Foo {
+  protected foo = 0;
+}
+
+class Bar {
+  protected foo = 0;
+}
+
+type Nothing<V extends Foo> = void;
+
+type Broken<V extends Array<Foo | Bar>> = {
+  readonly [P in keyof V]: V[P] extends Foo ? Nothing<V[P]> : never;
+};
+
+// The issue above, #49517, is fixed very indirectly. Here's some code
+// that shows the direct result of the change:
+
+type _3 = (Foo & Bar)['foo'];         // Ok
+type _4 = (Foo | Bar)['foo'];         // Error
+type _5 = (Foo | (Foo & Bar))['foo']; // Prev error, now ok
+
+// V[P] in `Nothing<V[P]>` is the substitution type `V[P] & Foo`. When
+// checking if that's assignable to `Foo` in the constraint of `Nothing`,
+// it passes the regular assignability check but then goes into intersection
+// property checks. To pull `foo` from the substitution type, it gets the
+// apparent type, which turns out to be something like `(Foo & Foo') | (Foo & Bar)`
+// where `Foo` and `Foo'` are different this-type instantiations of `Foo`.
+// Those two instantiations have the same `foo` property, but then `(Foo & Bar)['foo']`
+// is a synthesized intersection property with a declaration in `Foo` and a
+// declaration in `Bar`. Because the former was marked as protected and the
+// latter was a different symbol, we previously thought the two symbols were
+// totally unrelated, as in `(Foo | Bar)['foo']`. The fix I implemented is to
+// check not that the two property symbols are identical, but that they share
+// some common declaration. The change is directly observable by seeing whether
+// `(Foo | (Foo & Bar))['foo']` is allowed.