Skip to content

Commit

Permalink
Check nested weak types in intersections on target side of relation (#…
Browse files Browse the repository at this point in the history
…51140)

* Check nested weak types in intersections on target side of relation

* Add regression tests

* Move logic from isRelatedTo to structuredTypeRelatedTo

* Fix lint error

* Add additional test
  • Loading branch information
ahejlsberg committed Oct 13, 2022
1 parent 9f49f9c commit 37317a2
Show file tree
Hide file tree
Showing 6 changed files with 431 additions and 37 deletions.
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)) &&
!(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
70 changes: 70 additions & 0 deletions tests/baselines/reference/nestedExcessPropertyChecking.errors.txt
@@ -0,0 +1,70 @@
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(17,5): error TS2559: Type 'E' has no properties in common with type '{ nope?: any; }'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(18,5): error TS2559: Type '"A"' has no properties in common with type '{ nope?: any; }'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(30,22): error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.
tests/cases/compiler/nestedExcessPropertyChecking.ts(40,9): error TS2559: Type 'false' has no properties in common with type 'OverridesInput'.


==== tests/cases/compiler/nestedExcessPropertyChecking.ts (6 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'.

enum E { A = "A" }

let x: { nope?: any } = E.A; // Error
~
!!! error TS2559: Type 'E' has no properties in common with type '{ nope?: any; }'.
let y: { nope?: any } = "A"; // Error
~
!!! error TS2559: Type '"A"' has no properties in common with type '{ nope?: any; }'.

// 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:27: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:35:24: The expected type comes from property 'overrides' which is declared here on type 'VariablesA & VariablesB'
}
};

61 changes: 61 additions & 0 deletions tests/baselines/reference/nestedExcessPropertyChecking.js
@@ -0,0 +1,61 @@
//// [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

enum E { A = "A" }

let x: { nope?: any } = E.A; // Error
let y: { nope?: any } = "A"; // 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 E;
(function (E) {
E["A"] = "A";
})(E || (E = {}));
var x = E.A; // Error
var y = "A"; // Error
var foo1 = { variables: { overrides: false } }; // Error
var foo2 = {
variables: {
overrides: false // Error
}
};

0 comments on commit 37317a2

Please sign in to comment.