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.

Sign up for GitHub

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

Jump to bottom

{} & null and {} & undefined should always be never #50553

Merged
merged 4 commits into from Sep 1, 2022
Merged

{} & null and {} & undefined should always be never #50553

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3652682
{} & null and {} & undefined should be never in non-strictNullChecks …
ahejlsberg Aug 30, 2022
dd32cf8
Add tests
ahejlsberg Aug 30, 2022
03cb73f
Address code review feedback
ahejlsberg Sep 1, 2022
4502553
Accept new baselines
ahejlsberg Sep 1, 2022
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Expand Up @@ -15166,7 +15166,7 @@ namespace ts {
return includes & TypeFlags.IncludesWildcard ? wildcardType : anyType;
}
if (!strictNullChecks && includes & TypeFlags.Nullable) {
Copy link
Member

Choose a reason for hiding this comment

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

Can't we try to delete this whole !strictNullChecks branch entirely? Or are you still fine with undefined & null being weirdly undefined in non-strict but never in strict?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need that check for cases where the intersection is a singleton null or undefined. undefined & null is already resolved to never by an earlier check.

Copy link
Member

@weswigham weswigham Aug 31, 2022 •
edited

Choose a reason for hiding this comment

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

Oh, yeah, duh, I know why we can just delete this branch - 15 lines up there's a strictNullChecks only branch that does this already for strict mode - we can just delete both that strictNullChecks gate and this entire conditional.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nah, that branch also resolves null & object and null & { foo: string } to never, which we don't want to do in non-strictNullChecks mode.

Copy link
Member

Choose a reason for hiding this comment

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

But why not? If we're already adjusting null & {}, isn't that just a logical followup? null & {x} seems even more suspect than null & {}.

Copy link
Member Author

Choose a reason for hiding this comment

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

The whole premise of null and undefined being in the domain of every type is suspect. But that's non-strictNullChecks. My only objective here is to better align with pre-4.8 behavior in a corner case.

Copy link
Member

Choose a reason for hiding this comment

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

I would be open to seeing what happens and considering such a change for 4.9, but I think this workaround for 4.8 is okay for now.

return includes & TypeFlags.Undefined ? undefinedType : nullType;
return includes & TypeFlags.IncludesEmptyObject ? neverType : includes & TypeFlags.Undefined ? undefinedType : nullType;
}
if (includes & TypeFlags.String && includes & (TypeFlags.StringLiteral | TypeFlags.TemplateLiteral | TypeFlags.StringMapping) ||
includes & TypeFlags.Number && includes & TypeFlags.NumberLiteral ||
Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/NonNullableInNonStrictMode.js
View file
Open in desktop
@@ -0,0 +1,21 @@
//// [NonNullableInNonStrictMode.ts]
// These should all resolve to never

type T0 = NonNullable<null>;
type T1 = NonNullable<undefined>;
type T2 = null & {};
type T3 = undefined & {};
type T4 = null & undefined;
type T6 = null & { a: string } & {};

// Repro from #50519

type NonNullableNew<T> = T & {};
type NonNullableOld<T> = T extends null | undefined ? never : T;

type IsNullWithoutStrictNullChecks = NonNullableNew<null>;
type IsAlwaysNever = NonNullableOld<null>;


//// [NonNullableInNonStrictMode.js]
// These should all resolve to never
45 changes: 45 additions & 0 deletions tests/baselines/reference/NonNullableInNonStrictMode.symbols
View file
Open in desktop
@@ -0,0 +1,45 @@
=== tests/cases/compiler/NonNullableInNonStrictMode.ts ===
// These should all resolve to never

type T0 = NonNullable<null>;
>T0 : Symbol(T0, Decl(NonNullableInNonStrictMode.ts, 0, 0))
>NonNullable : Symbol(NonNullable, Decl(lib.es5.d.ts, --, --))

type T1 = NonNullable<undefined>;
>T1 : Symbol(T1, Decl(NonNullableInNonStrictMode.ts, 2, 28))
>NonNullable : Symbol(NonNullable, Decl(lib.es5.d.ts, --, --))

type T2 = null & {};
>T2 : Symbol(T2, Decl(NonNullableInNonStrictMode.ts, 3, 33))

type T3 = undefined & {};
>T3 : Symbol(T3, Decl(NonNullableInNonStrictMode.ts, 4, 20))

type T4 = null & undefined;
>T4 : Symbol(T4, Decl(NonNullableInNonStrictMode.ts, 5, 25))

type T6 = null & { a: string } & {};
>T6 : Symbol(T6, Decl(NonNullableInNonStrictMode.ts, 6, 27))
>a : Symbol(a, Decl(NonNullableInNonStrictMode.ts, 7, 18))

// Repro from #50519

type NonNullableNew<T> = T & {};
>NonNullableNew : Symbol(NonNullableNew, Decl(NonNullableInNonStrictMode.ts, 7, 36))
>T : Symbol(T, Decl(NonNullableInNonStrictMode.ts, 11, 20))
>T : Symbol(T, Decl(NonNullableInNonStrictMode.ts, 11, 20))

type NonNullableOld<T> = T extends null | undefined ? never : T;
>NonNullableOld : Symbol(NonNullableOld, Decl(NonNullableInNonStrictMode.ts, 11, 32))
>T : Symbol(T, Decl(NonNullableInNonStrictMode.ts, 12, 20))
>T : Symbol(T, Decl(NonNullableInNonStrictMode.ts, 12, 20))
>T : Symbol(T, Decl(NonNullableInNonStrictMode.ts, 12, 20))

type IsNullWithoutStrictNullChecks = NonNullableNew<null>;
>IsNullWithoutStrictNullChecks : Symbol(IsNullWithoutStrictNullChecks, Decl(NonNullableInNonStrictMode.ts, 12, 64))
>NonNullableNew : Symbol(NonNullableNew, Decl(NonNullableInNonStrictMode.ts, 7, 36))

type IsAlwaysNever = NonNullableOld<null>;
>IsAlwaysNever : Symbol(IsAlwaysNever, Decl(NonNullableInNonStrictMode.ts, 14, 58))
>NonNullableOld : Symbol(NonNullableOld, Decl(NonNullableInNonStrictMode.ts, 11, 32))

43 changes: 43 additions & 0 deletions tests/baselines/reference/NonNullableInNonStrictMode.types
View file
Open in desktop
@@ -0,0 +1,43 @@
=== tests/cases/compiler/NonNullableInNonStrictMode.ts ===
// These should all resolve to never

type T0 = NonNullable<null>;
>T0 : never
>null : null

type T1 = NonNullable<undefined>;
>T1 : never

type T2 = null & {};
>T2 : never
>null : null

type T3 = undefined & {};
>T3 : never

type T4 = null & undefined;
>T4 : never
>null : null

type T6 = null & { a: string } & {};
>T6 : never
>null : null
>a : string

// Repro from #50519

type NonNullableNew<T> = T & {};
>NonNullableNew : NonNullableNew<T>

type NonNullableOld<T> = T extends null | undefined ? never : T;
>NonNullableOld : NonNullableOld<T>
>null : null

type IsNullWithoutStrictNullChecks = NonNullableNew<null>;
>IsNullWithoutStrictNullChecks : never
>null : null

type IsAlwaysNever = NonNullableOld<null>;
>IsAlwaysNever : never
>null : null

18 changes: 18 additions & 0 deletions tests/cases/compiler/NonNullableInNonStrictMode.ts
View file
Open in desktop
@@ -0,0 +1,18 @@
// @strict: false
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be good to actually test these behaviors in both strict on and off.

Suggested change
// @strict: false
// @strict: true,false

Then rename the test to nonNullablesAndObjectIntersections.ts


// These should all resolve to never

type T0 = NonNullable<null>;
type T1 = NonNullable<undefined>;
type T2 = null & {};
type T3 = undefined & {};
type T4 = null & undefined;
type T6 = null & { a: string } & {};

// Repro from #50519

type NonNullableNew<T> = T & {};
type NonNullableOld<T> = T extends null | undefined ? never : T;

type IsNullWithoutStrictNullChecks = NonNullableNew<null>;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this name seems to be confusing to me since it doesn't actually describe the expected result (if I understand correctly) but rather it refers to what was a bug

type IsAlwaysNever = NonNullableOld<null>;