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

Check nested weak types in intersections on target side of relation #51140

Merged
merged 5 commits into from Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
70 changes: 33 additions & 37 deletions src/compiler/checker.ts
Expand Up @@ -194,8 +194,6 @@ namespace ts {
None = 0,
Source = 1 << 0,
Target = 1 << 1,
PropertyCheck = 1 << 2,
InPropertyCheck = 1 << 3,
}

const enum RecursionFlags {
Expand Down Expand Up @@ -19112,7 +19110,7 @@ namespace ts {
}
}

const isPerformingCommonPropertyChecks = (relation !== comparableRelation || !(source.flags & TypeFlags.Union) && isLiteralType(source)) &&
const isPerformingCommonPropertyChecks = (relation !== comparableRelation || isUnitType(source)) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the problem with isUnitType is that it doesn't consider enum literal types.

https://github.com/microsoft/TypeScript/pull/50423/files#r954112029

So we won't catch assignability from an enum literal to a weak type with no overlap.

enum E { A = "A" }

// these will differ
let x: { nope?: any } = E.A;
let y: { nope?: any } = "A";

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enum literal types are definitely considered unit types. It's a bit subtle, isUnitType checks for TypeFlags.Unit, which includes TypeFlags.Literal, which includes TypeFlags.StringLiteral and TypeFlags.NumberLiteral. For enum literal types, the TypeFlags.EnumLiteral flag is always combined with one of the latter two, so isUnitType will always be true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure, I verified both of your examples generate errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Can you add that as a test case?

!(intersectionState & IntersectionState.Target) &&
source.flags & (TypeFlags.Primitive | TypeFlags.Object | TypeFlags.Intersection) && source !== globalObjectType &&
target.flags & (TypeFlags.Object | TypeFlags.Intersection) && isWeakType(target) &&
Expand All @@ -19139,31 +19137,9 @@ namespace ts {

const skipCaching = source.flags & TypeFlags.Union && (source as UnionType).types.length < 4 && !(target.flags & TypeFlags.Union) ||
target.flags & TypeFlags.Union && (target as UnionType).types.length < 4 && !(source.flags & TypeFlags.StructuredOrInstantiable);
let result = skipCaching ?
const result = skipCaching ?
unionOrIntersectionRelatedTo(source, target, reportErrors, intersectionState) :
recursiveTypeRelatedTo(source, target, reportErrors, intersectionState, recursionFlags);
// For certain combinations involving intersections and optional, excess, or mismatched properties we need
// an extra property check where the intersection is viewed as a single object. The following are motivating
// examples that all should be errors, but aren't without this extra property check:
//
// let obj: { a: { x: string } } & { c: number } = { a: { x: 'hello', y: 2 }, c: 5 }; // Nested excess property
//
// declare let wrong: { a: { y: string } };
// let weak: { a?: { x?: number } } & { c?: string } = wrong; // Nested weak object type
//
// function foo<T extends object>(x: { a?: string }, y: T & { a: boolean }) {
// x = y; // Mismatched property in source intersection
// }
//
// We suppress recursive intersection property checks because they can generate lots of work when relating
// recursive intersections that are structurally similar but not exactly identical. See #37854.
if (result && !inPropertyCheck && (
target.flags & TypeFlags.Intersection && (isPerformingExcessPropertyChecks || isPerformingCommonPropertyChecks) ||
isNonGenericObjectType(target) && !isArrayOrTupleType(target) && source.flags & TypeFlags.Intersection && getApparentType(source).flags & TypeFlags.StructuredType && !some((source as IntersectionType).types, t => !!(getObjectFlags(t) & ObjectFlags.NonInferrableType)))) {
inPropertyCheck = true;
result &= recursiveTypeRelatedTo(source, target, reportErrors, IntersectionState.PropertyCheck, recursionFlags);
inPropertyCheck = false;
}
if (result) {
return result;
}
Expand Down Expand Up @@ -19567,8 +19543,7 @@ namespace ts {
if (overflow) {
return Ternary.False;
}
const keyIntersectionState = intersectionState | (inPropertyCheck ? IntersectionState.InPropertyCheck : 0);
const id = getRelationKey(source, target, keyIntersectionState, relation, /*ingnoreConstraints*/ false);
const id = getRelationKey(source, target, intersectionState, relation, /*ingnoreConstraints*/ false);
const entry = relation.get(id);
if (entry !== undefined) {
if (reportErrors && entry & RelationComparisonResult.Failed && !(entry & RelationComparisonResult.Reported)) {
Expand Down Expand Up @@ -19598,7 +19573,7 @@ namespace ts {
// A key that starts with "*" is an indication that we have type references that reference constrained
// type parameters. For such keys we also check against the key we would have gotten if all type parameters
// were unconstrained.
const broadestEquivalentId = id.startsWith("*") ? getRelationKey(source, target, keyIntersectionState, relation, /*ignoreConstraints*/ true) : undefined;
const broadestEquivalentId = id.startsWith("*") ? getRelationKey(source, target, intersectionState, relation, /*ignoreConstraints*/ true) : undefined;
for (let i = 0; i < maybeCount; i++) {
// If source and target are already being compared, consider them related with assumptions
if (id === maybeKeys[i] || broadestEquivalentId && broadestEquivalentId === maybeKeys[i]) {
Expand Down Expand Up @@ -19686,7 +19661,7 @@ namespace ts {
function structuredTypeRelatedTo(source: Type, target: Type, reportErrors: boolean, intersectionState: IntersectionState): Ternary {
const saveErrorInfo = captureErrorCalculationState();
let result = structuredTypeRelatedToWorker(source, target, reportErrors, intersectionState, saveErrorInfo);
if (!result && (source.flags & TypeFlags.Intersection || source.flags & TypeFlags.TypeParameter && target.flags & TypeFlags.Union)) {
if (relation !== identityRelation) {
// The combined constraint of an intersection type is the intersection of the constraints of
// the constituents. When an intersection type contains instantiable types with union type
// constraints, there are situations where we need to examine the combined constraint. One is
Expand All @@ -19700,10 +19675,34 @@ namespace ts {
// needs to have its constraint hoisted into an intersection with said type parameter, this way
// the type param can be compared with itself in the target (with the influence of its constraint to match other parts)
// For example, if `T extends 1 | 2` and `U extends 2 | 3` and we compare `T & U` to `T & U & (1 | 2 | 3)`
const constraint = getEffectiveConstraintOfIntersection(source.flags & TypeFlags.Intersection ? (source as IntersectionType).types: [source], !!(target.flags & TypeFlags.Union));
if (constraint && !(constraint.flags & TypeFlags.Never) && everyType(constraint, c => c !== source)) { // Skip comparison if expansion contains the source itself
// TODO: Stack errors so we get a pyramid for the "normal" comparison above, _and_ a second for this
result = isRelatedTo(constraint, target, RecursionFlags.Source, /*reportErrors*/ false, /*headMessage*/ undefined, intersectionState);
if (!result && (source.flags & TypeFlags.Intersection || source.flags & TypeFlags.TypeParameter && target.flags & TypeFlags.Union)) {
const constraint = getEffectiveConstraintOfIntersection(source.flags & TypeFlags.Intersection ? (source as IntersectionType).types: [source], !!(target.flags & TypeFlags.Union));
if (constraint && !(constraint.flags & TypeFlags.Never) && everyType(constraint, c => c !== source)) { // Skip comparison if expansion contains the source itself
// TODO: Stack errors so we get a pyramid for the "normal" comparison above, _and_ a second for this
result = isRelatedTo(constraint, target, RecursionFlags.Source, /*reportErrors*/ false, /*headMessage*/ undefined, intersectionState);
}
}
// For certain combinations involving intersections and optional, excess, or mismatched properties we need
// an extra property check where the intersection is viewed as a single object. The following are motivating
// examples that all should be errors, but aren't without this extra property check:
//
// let obj: { a: { x: string } } & { c: number } = { a: { x: 'hello', y: 2 }, c: 5 }; // Nested excess property
//
// declare let wrong: { a: { y: string } };
// let weak: { a?: { x?: number } } & { c?: string } = wrong; // Nested weak object type
//
// function foo<T extends object>(x: { a?: string }, y: T & { a: boolean }) {
// x = y; // Mismatched property in source intersection
// }
//
// We suppress recursive intersection property checks because they can generate lots of work when relating
// recursive intersections that are structurally similar but not exactly identical. See #37854.
if (result && !inPropertyCheck && (
target.flags & TypeFlags.Intersection && !isGenericObjectType(target) && source.flags & (TypeFlags.Object | TypeFlags.Intersection) ||
isNonGenericObjectType(target) && !isArrayOrTupleType(target) && source.flags & TypeFlags.Intersection && getApparentType(source).flags & TypeFlags.StructuredType && !some((source as IntersectionType).types, t => !!(getObjectFlags(t) & ObjectFlags.NonInferrableType)))) {
inPropertyCheck = true;
result &= propertiesRelatedTo(source, target, reportErrors, /*excludedProperties*/ undefined, IntersectionState.None);
inPropertyCheck = false;
}
}
if (result) {
Expand All @@ -19713,9 +19712,6 @@ namespace ts {
}

function structuredTypeRelatedToWorker(source: Type, target: Type, reportErrors: boolean, intersectionState: IntersectionState, saveErrorInfo: ReturnType<typeof captureErrorCalculationState>): Ternary {
if (intersectionState & IntersectionState.PropertyCheck) {
return propertiesRelatedTo(source, target, reportErrors, /*excludedProperties*/ undefined, IntersectionState.None);
}
let result: Ternary;
let originalErrorInfo: DiagnosticMessageChain | undefined;
let varianceCheckFailed = false;
Expand Down
59 changes: 59 additions & 0 deletions tests/baselines/reference/nestedExcessPropertyChecking.errors.txt
@@ -0,0 +1,59 @@
tests/cases/compiler/nestedExcessPropertyChecking.ts(6,7): error TS2322: Type 'C1' is not assignable to type 'A1 & B1'.
Types of property 'x' are incompatible.
Type '{ c: string; }' has no properties in common with type '{ a?: string | undefined; } & { b?: string | undefined; }'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(13,7): error TS2559: Type 'C2' has no properties in common with type 'A2 & B2'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(25,22): error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(35,9): error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.


==== tests/cases/compiler/nestedExcessPropertyChecking.ts (4 errors) ====
type A1 = { x: { a?: string } };
type B1 = { x: { b?: string } };

type C1 = { x: { c: string } };

const ab1: A1 & B1 = {} as C1; // Error
~~~
!!! error TS2322: Type 'C1' is not assignable to type 'A1 & B1'.
!!! error TS2322: Types of property 'x' are incompatible.
!!! error TS2322: Type '{ c: string; }' has no properties in common with type '{ a?: string | undefined; } & { b?: string | undefined; }'.

type A2 = { a?: string };
type B2 = { b?: string };

type C2 = { c: string };

const ab2: A2 & B2 = {} as C2; // Error
~~~
!!! error TS2559: Type 'C2' has no properties in common with type 'A2 & B2'.

// Repros from #51043

type OverridesInput = {
someProp?: 'A' | 'B'
}

const foo1: Partial<{ something: any }> & { variables: {
overrides?: OverridesInput;
} & Partial<{
overrides?: OverridesInput;
}>} = { variables: { overrides: false } }; // Error
~~~~~~~~~
!!! error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.
!!! related TS6500 tests/cases/compiler/nestedExcessPropertyChecking.ts:22:5: The expected type comes from property 'overrides' which is declared here on type '{ overrides?: OverridesInput | undefined; } & Partial<{ overrides?: OverridesInput | undefined; }>'


interface Unrelated { _?: any }

interface VariablesA { overrides?: OverridesInput; }
interface VariablesB { overrides?: OverridesInput; }

const foo2: Unrelated & { variables: VariablesA & VariablesB } = {
variables: {
overrides: false // Error
~~~~~~~~~
!!! error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.
!!! related TS6500 tests/cases/compiler/nestedExcessPropertyChecking.ts:30:24: The expected type comes from property 'overrides' which is declared here on type 'VariablesA & VariablesB'
}
};

50 changes: 50 additions & 0 deletions tests/baselines/reference/nestedExcessPropertyChecking.js
@@ -0,0 +1,50 @@
//// [nestedExcessPropertyChecking.ts]
type A1 = { x: { a?: string } };
type B1 = { x: { b?: string } };

type C1 = { x: { c: string } };

const ab1: A1 & B1 = {} as C1; // Error

type A2 = { a?: string };
type B2 = { b?: string };

type C2 = { c: string };

const ab2: A2 & B2 = {} as C2; // Error

// Repros from #51043

type OverridesInput = {
someProp?: 'A' | 'B'
}

const foo1: Partial<{ something: any }> & { variables: {
overrides?: OverridesInput;
} & Partial<{
overrides?: OverridesInput;
}>} = { variables: { overrides: false } }; // Error


interface Unrelated { _?: any }

interface VariablesA { overrides?: OverridesInput; }
interface VariablesB { overrides?: OverridesInput; }

const foo2: Unrelated & { variables: VariablesA & VariablesB } = {
variables: {
overrides: false // Error
}
};


//// [nestedExcessPropertyChecking.js]
"use strict";
var ab1 = {}; // Error
var ab2 = {}; // Error
var foo1 = { variables: { overrides: false } }; // Error
var foo2 = {
variables: {
overrides: false // Error
}
};
100 changes: 100 additions & 0 deletions tests/baselines/reference/nestedExcessPropertyChecking.symbols
@@ -0,0 +1,100 @@
=== tests/cases/compiler/nestedExcessPropertyChecking.ts ===
type A1 = { x: { a?: string } };
>A1 : Symbol(A1, Decl(nestedExcessPropertyChecking.ts, 0, 0))
>x : Symbol(x, Decl(nestedExcessPropertyChecking.ts, 0, 11))
>a : Symbol(a, Decl(nestedExcessPropertyChecking.ts, 0, 16))

type B1 = { x: { b?: string } };
>B1 : Symbol(B1, Decl(nestedExcessPropertyChecking.ts, 0, 32))
>x : Symbol(x, Decl(nestedExcessPropertyChecking.ts, 1, 11))
>b : Symbol(b, Decl(nestedExcessPropertyChecking.ts, 1, 16))

type C1 = { x: { c: string } };
>C1 : Symbol(C1, Decl(nestedExcessPropertyChecking.ts, 1, 32))
>x : Symbol(x, Decl(nestedExcessPropertyChecking.ts, 3, 11))
>c : Symbol(c, Decl(nestedExcessPropertyChecking.ts, 3, 16))

const ab1: A1 & B1 = {} as C1; // Error
>ab1 : Symbol(ab1, Decl(nestedExcessPropertyChecking.ts, 5, 5))
>A1 : Symbol(A1, Decl(nestedExcessPropertyChecking.ts, 0, 0))
>B1 : Symbol(B1, Decl(nestedExcessPropertyChecking.ts, 0, 32))
>C1 : Symbol(C1, Decl(nestedExcessPropertyChecking.ts, 1, 32))

type A2 = { a?: string };
>A2 : Symbol(A2, Decl(nestedExcessPropertyChecking.ts, 5, 30))
>a : Symbol(a, Decl(nestedExcessPropertyChecking.ts, 7, 11))

type B2 = { b?: string };
>B2 : Symbol(B2, Decl(nestedExcessPropertyChecking.ts, 7, 25))
>b : Symbol(b, Decl(nestedExcessPropertyChecking.ts, 8, 11))

type C2 = { c: string };
>C2 : Symbol(C2, Decl(nestedExcessPropertyChecking.ts, 8, 25))
>c : Symbol(c, Decl(nestedExcessPropertyChecking.ts, 10, 11))

const ab2: A2 & B2 = {} as C2; // Error
>ab2 : Symbol(ab2, Decl(nestedExcessPropertyChecking.ts, 12, 5))
>A2 : Symbol(A2, Decl(nestedExcessPropertyChecking.ts, 5, 30))
>B2 : Symbol(B2, Decl(nestedExcessPropertyChecking.ts, 7, 25))
>C2 : Symbol(C2, Decl(nestedExcessPropertyChecking.ts, 8, 25))

// Repros from #51043

type OverridesInput = {
>OverridesInput : Symbol(OverridesInput, Decl(nestedExcessPropertyChecking.ts, 12, 30))

someProp?: 'A' | 'B'
>someProp : Symbol(someProp, Decl(nestedExcessPropertyChecking.ts, 16, 23))
}

const foo1: Partial<{ something: any }> & { variables: {
>foo1 : Symbol(foo1, Decl(nestedExcessPropertyChecking.ts, 20, 5))
>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))
>something : Symbol(something, Decl(nestedExcessPropertyChecking.ts, 20, 21))
>variables : Symbol(variables, Decl(nestedExcessPropertyChecking.ts, 20, 43))

overrides?: OverridesInput;
>overrides : Symbol(overrides, Decl(nestedExcessPropertyChecking.ts, 20, 56))
>OverridesInput : Symbol(OverridesInput, Decl(nestedExcessPropertyChecking.ts, 12, 30))

} & Partial<{
>Partial : Symbol(Partial, Decl(lib.es5.d.ts, --, --))

overrides?: OverridesInput;
>overrides : Symbol(overrides, Decl(nestedExcessPropertyChecking.ts, 22, 13))
>OverridesInput : Symbol(OverridesInput, Decl(nestedExcessPropertyChecking.ts, 12, 30))

}>} = { variables: { overrides: false } }; // Error
>variables : Symbol(variables, Decl(nestedExcessPropertyChecking.ts, 24, 7))
>overrides : Symbol(overrides, Decl(nestedExcessPropertyChecking.ts, 24, 20))


interface Unrelated { _?: any }
>Unrelated : Symbol(Unrelated, Decl(nestedExcessPropertyChecking.ts, 24, 42))
>_ : Symbol(Unrelated._, Decl(nestedExcessPropertyChecking.ts, 27, 21))

interface VariablesA { overrides?: OverridesInput; }
>VariablesA : Symbol(VariablesA, Decl(nestedExcessPropertyChecking.ts, 27, 31))
>overrides : Symbol(VariablesA.overrides, Decl(nestedExcessPropertyChecking.ts, 29, 22))
>OverridesInput : Symbol(OverridesInput, Decl(nestedExcessPropertyChecking.ts, 12, 30))

interface VariablesB { overrides?: OverridesInput; }
>VariablesB : Symbol(VariablesB, Decl(nestedExcessPropertyChecking.ts, 29, 52))
>overrides : Symbol(VariablesB.overrides, Decl(nestedExcessPropertyChecking.ts, 30, 22))
>OverridesInput : Symbol(OverridesInput, Decl(nestedExcessPropertyChecking.ts, 12, 30))

const foo2: Unrelated & { variables: VariablesA & VariablesB } = {
>foo2 : Symbol(foo2, Decl(nestedExcessPropertyChecking.ts, 32, 5))
>Unrelated : Symbol(Unrelated, Decl(nestedExcessPropertyChecking.ts, 24, 42))
>variables : Symbol(variables, Decl(nestedExcessPropertyChecking.ts, 32, 25))
>VariablesA : Symbol(VariablesA, Decl(nestedExcessPropertyChecking.ts, 27, 31))
>VariablesB : Symbol(VariablesB, Decl(nestedExcessPropertyChecking.ts, 29, 52))

variables: {
>variables : Symbol(variables, Decl(nestedExcessPropertyChecking.ts, 32, 66))

overrides: false // Error
>overrides : Symbol(overrides, Decl(nestedExcessPropertyChecking.ts, 33, 16))
}
};