Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/compiler/checker.ts
Expand Up @@ -12440,7 +12440,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
Expand Down Expand Up @@ -12488,7 +12488,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;
Expand Down Expand Up @@ -12594,6 +12599,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
Expand Down
@@ -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.

@@ -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.
@@ -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.

@@ -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.

@@ -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.