diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index afac9b5a1ae85..8f252f80f8c53 100644 --- a/src/compiler/checker.ts +++ b/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.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 | 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 diff --git a/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.errors.txt b/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.errors.txt new file mode 100644 index 0000000000000..2bba10dded19c --- /dev/null +++ b/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 = void; + + type Broken> = { + readonly [P in keyof V]: V[P] extends Foo ? Nothing : 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` 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. + \ No newline at end of file diff --git a/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.js b/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.js new file mode 100644 index 0000000000000..3c2e4f45cc5d4 --- /dev/null +++ b/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.js @@ -0,0 +1,65 @@ +//// [unionPropertyOfProtectedAndIntersectionProperty.ts] +class Foo { + protected foo = 0; +} + +class Bar { + protected foo = 0; +} + +type Nothing = void; + +type Broken> = { + readonly [P in keyof V]: V[P] extends Foo ? Nothing : 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` 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` 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. diff --git a/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.symbols b/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.symbols new file mode 100644 index 0000000000000..311bd2168bf56 --- /dev/null +++ b/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 = 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> = { +>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 : 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` 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. + diff --git a/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.types b/tests/baselines/reference/unionPropertyOfProtectedAndIntersectionProperty.types new file mode 100644 index 0000000000000..793146315c911 --- /dev/null +++ b/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 = void; +>Nothing : void + +type Broken> = { +>Broken : Broken + + readonly [P in keyof V]: V[P] extends Foo ? Nothing : 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` 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. + diff --git a/tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts b/tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts new file mode 100644 index 0000000000000..58671da1b4265 --- /dev/null +++ b/tests/cases/compiler/unionPropertyOfProtectedAndIntersectionProperty.ts @@ -0,0 +1,35 @@ +class Foo { + protected foo = 0; +} + +class Bar { + protected foo = 0; +} + +type Nothing = void; + +type Broken> = { + readonly [P in keyof V]: V[P] extends Foo ? Nothing : 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` 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.