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

Improve isValidSpreadType check #47010

Merged
merged 4 commits into from
Dec 4, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20857,7 +20857,7 @@ namespace ts {
// flags for the string, number, boolean, "", 0, false, void, undefined, or null types respectively. Returns
// no flags for all other types (including non-falsy literal types).
function getFalsyFlags(type: Type): TypeFlags {
return type.flags & TypeFlags.Union ? getFalsyFlagsOfTypes((type as UnionType).types) :
return type.flags & TypeFlags.UnionOrIntersection ? getFalsyFlagsOfTypes((type as UnionType).types) :
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little surprised that adding intersections to getFalsyFlags didn't change any existing baselines.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, there are precious few intersections that would be affected, pretty much only those like T & undefined that will now appear to be falsy.

type.flags & TypeFlags.StringLiteral ? (type as StringLiteralType).value === "" ? TypeFlags.StringLiteral : 0 :
type.flags & TypeFlags.NumberLiteral ? (type as NumberLiteralType).value === 0 ? TypeFlags.NumberLiteral : 0 :
type.flags & TypeFlags.BigIntLiteral ? isZeroBigInt(type as BigIntLiteralType) ? TypeFlags.BigIntLiteral : 0 :
Expand Down Expand Up @@ -27265,15 +27265,9 @@ namespace ts {
}

function isValidSpreadType(type: Type): boolean {
if (type.flags & TypeFlags.Instantiable) {
const constraint = getBaseConstraintOfType(type);
if (constraint !== undefined) {
return isValidSpreadType(constraint);
}
}
return !!(type.flags & (TypeFlags.Any | TypeFlags.NonPrimitive | TypeFlags.Object | TypeFlags.InstantiableNonPrimitive) ||
getFalsyFlags(type) & TypeFlags.DefinitelyFalsy && isValidSpreadType(removeDefinitelyFalsyTypes(type)) ||
type.flags & TypeFlags.UnionOrIntersection && every((type as UnionOrIntersectionType).types, isValidSpreadType));
const t = removeDefinitelyFalsyTypes(mapType(type, getBaseConstraintOrType));
Copy link
Member

Choose a reason for hiding this comment

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

love that we have functions named getBaseConstraintOrType and getBaseConstraintOfType

return !!(t.flags & (TypeFlags.Any | TypeFlags.NonPrimitive | TypeFlags.Object | TypeFlags.InstantiableNonPrimitive) ||
t.flags & TypeFlags.UnionOrIntersection && every((t as UnionOrIntersectionType).types, isValidSpreadType));
}

function checkJsxSelfClosingElementDeferred(node: JsxSelfClosingElement) {
Expand Down
42 changes: 42 additions & 0 deletions tests/baselines/reference/spreadObjectOrFalsy.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts(2,14): error TS2698: Spread types may only be created from object types.
tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts(10,14): error TS2698: Spread types may only be created from object types.


==== tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts (2 errors) ====
function f1<T>(a: T & undefined) {
return { ...a }; // Error
~~~~
!!! error TS2698: Spread types may only be created from object types.
}

function f2<T>(a: T | T & undefined) {
return { ...a };
}

function f3<T extends undefined>(a: T) {
return { ...a }; // Error
~~~~
!!! error TS2698: Spread types may only be created from object types.
}

function f4<T extends undefined>(a: object | T) {
return { ...a };
}

function f5<S, T extends undefined>(a: S | T) {
return { ...a };
}

function f6<T extends object | undefined>(a: T) {
return { ...a };
}

// Repro from #46976

function g1<T extends {}, A extends { z: (T | undefined) & T }>(a: A) {
const { z } = a;
return {
...z
};
}

83 changes: 83 additions & 0 deletions tests/baselines/reference/spreadObjectOrFalsy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//// [spreadObjectOrFalsy.ts]
function f1<T>(a: T & undefined) {
return { ...a }; // Error
}

function f2<T>(a: T | T & undefined) {
return { ...a };
}

function f3<T extends undefined>(a: T) {
return { ...a }; // Error
}

function f4<T extends undefined>(a: object | T) {
return { ...a };
}

function f5<S, T extends undefined>(a: S | T) {
return { ...a };
}

function f6<T extends object | undefined>(a: T) {
return { ...a };
}

// Repro from #46976

function g1<T extends {}, A extends { z: (T | undefined) & T }>(a: A) {
const { z } = a;
return {
...z
};
}


//// [spreadObjectOrFalsy.js]
"use strict";
var __assign = (this && this.__assign) || function () {
__assign = Object.assign || function(t) {
for (var s, i = 1, n = arguments.length; i < n; i++) {
s = arguments[i];
for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p))
t[p] = s[p];
}
return t;
};
return __assign.apply(this, arguments);
};
function f1(a) {
return __assign({}, a); // Error
}
function f2(a) {
return __assign({}, a);
}
function f3(a) {
return __assign({}, a); // Error
}
function f4(a) {
return __assign({}, a);
}
function f5(a) {
return __assign({}, a);
}
function f6(a) {
return __assign({}, a);
}
// Repro from #46976
function g1(a) {
var z = a.z;
return __assign({}, z);
}


//// [spreadObjectOrFalsy.d.ts]
declare function f1<T>(a: T & undefined): any;
declare function f2<T>(a: T | T & undefined): T | (T & undefined);
declare function f3<T extends undefined>(a: T): any;
declare function f4<T extends undefined>(a: object | T): {};
declare function f5<S, T extends undefined>(a: S | T): S | T;
declare function f6<T extends object | undefined>(a: T): T;
declare function g1<T extends {}, A extends {
z: (T | undefined) & T;
}>(a: A): (T | undefined) & T;
87 changes: 87 additions & 0 deletions tests/baselines/reference/spreadObjectOrFalsy.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
=== tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts ===
function f1<T>(a: T & undefined) {
>f1 : Symbol(f1, Decl(spreadObjectOrFalsy.ts, 0, 0))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 0, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 0, 15))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 0, 12))

return { ...a }; // Error
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 0, 15))
}

function f2<T>(a: T | T & undefined) {
>f2 : Symbol(f2, Decl(spreadObjectOrFalsy.ts, 2, 1))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 4, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 4, 15))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 4, 12))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 4, 12))

return { ...a };
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 4, 15))
}

function f3<T extends undefined>(a: T) {
>f3 : Symbol(f3, Decl(spreadObjectOrFalsy.ts, 6, 1))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 8, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 8, 33))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 8, 12))

return { ...a }; // Error
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 8, 33))
}

function f4<T extends undefined>(a: object | T) {
>f4 : Symbol(f4, Decl(spreadObjectOrFalsy.ts, 10, 1))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 12, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 12, 33))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 12, 12))

return { ...a };
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 12, 33))
}

function f5<S, T extends undefined>(a: S | T) {
>f5 : Symbol(f5, Decl(spreadObjectOrFalsy.ts, 14, 1))
>S : Symbol(S, Decl(spreadObjectOrFalsy.ts, 16, 12))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 16, 14))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 16, 36))
>S : Symbol(S, Decl(spreadObjectOrFalsy.ts, 16, 12))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 16, 14))

return { ...a };
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 16, 36))
}

function f6<T extends object | undefined>(a: T) {
>f6 : Symbol(f6, Decl(spreadObjectOrFalsy.ts, 18, 1))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 20, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 20, 42))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 20, 12))

return { ...a };
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 20, 42))
}

// Repro from #46976

function g1<T extends {}, A extends { z: (T | undefined) & T }>(a: A) {
>g1 : Symbol(g1, Decl(spreadObjectOrFalsy.ts, 22, 1))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 26, 12))
>A : Symbol(A, Decl(spreadObjectOrFalsy.ts, 26, 25))
>z : Symbol(z, Decl(spreadObjectOrFalsy.ts, 26, 37))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 26, 12))
>T : Symbol(T, Decl(spreadObjectOrFalsy.ts, 26, 12))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 26, 64))
>A : Symbol(A, Decl(spreadObjectOrFalsy.ts, 26, 25))

const { z } = a;
>z : Symbol(z, Decl(spreadObjectOrFalsy.ts, 27, 11))
>a : Symbol(a, Decl(spreadObjectOrFalsy.ts, 26, 64))

return {
...z
>z : Symbol(z, Decl(spreadObjectOrFalsy.ts, 27, 11))

};
}

75 changes: 75 additions & 0 deletions tests/baselines/reference/spreadObjectOrFalsy.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
=== tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts ===
function f1<T>(a: T & undefined) {
>f1 : <T>(a: T & undefined) => any
>a : T & undefined

return { ...a }; // Error
>{ ...a } : any
>a : T & undefined
}

function f2<T>(a: T | T & undefined) {
>f2 : <T>(a: T | (T & undefined)) => T | (T & undefined)
>a : T | (T & undefined)

return { ...a };
>{ ...a } : T | (T & undefined)
>a : T | (T & undefined)
}

function f3<T extends undefined>(a: T) {
>f3 : <T extends undefined>(a: T) => any
>a : T

return { ...a }; // Error
>{ ...a } : any
>a : T
}

function f4<T extends undefined>(a: object | T) {
>f4 : <T extends undefined>(a: object | T) => {}
>a : object | T

return { ...a };
>{ ...a } : {}
>a : object | T
}

function f5<S, T extends undefined>(a: S | T) {
>f5 : <S, T extends undefined>(a: S | T) => S | T
>a : S | T

return { ...a };
>{ ...a } : S | T
>a : S | T
}

function f6<T extends object | undefined>(a: T) {
>f6 : <T extends object | undefined>(a: T) => T
>a : T

return { ...a };
>{ ...a } : T
>a : T
}

// Repro from #46976

function g1<T extends {}, A extends { z: (T | undefined) & T }>(a: A) {
>g1 : <T extends {}, A extends { z: (T | undefined) & T; }>(a: A) => (T | undefined) & T
>z : (T | undefined) & T
>a : A

const { z } = a;
>z : (T | undefined) & T
>a : A

return {
>{ ...z } : (T | undefined) & T

...z
>z : (T | undefined) & T

};
}

35 changes: 35 additions & 0 deletions tests/cases/conformance/types/spread/spreadObjectOrFalsy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// @strict: true
// @declaration: true

function f1<T>(a: T & undefined) {
return { ...a }; // Error
}

function f2<T>(a: T | T & undefined) {
return { ...a };
}

function f3<T extends undefined>(a: T) {
return { ...a }; // Error
}

function f4<T extends undefined>(a: object | T) {
return { ...a };
}

function f5<S, T extends undefined>(a: S | T) {
return { ...a };
}

function f6<T extends object | undefined>(a: T) {
return { ...a };
}

// Repro from #46976

function g1<T extends {}, A extends { z: (T | undefined) & T }>(a: A) {
const { z } = a;
return {
...z
};
}