Fix #1809, introduce non primitive object type

[Herrington Darkholme (HerringtonDarkholme)]

Fixes #1809. I found object primitive type is quite confusing. Because what it means is that other primitive types cannot be assigned to. So I change the name to non primitive.

non primitive type is quite useful for ECMA API like Object.create, Object.setPrototype and Proxy.
It is also useful for userland libraries such as immutable and backbone.

[Daniel Rosenwasser (DanielRosenwasser)]

Anders Hejlsberg (@ahejlsberg) can judge better, but I don't think you want to set any structured type members - I think you want to just need to add a branch in getApparentType for object.

[Daniel Rosenwasser (DanielRosenwasser)]

This is only used in one place right now, so consider just inlining.

[Daniel Rosenwasser (DanielRosenwasser)]

Herrington Darkholme (@HerringtonDarkholme) thanks for the PR! As a heads up, we'll probably have to discuss this more at a design meeting, but I think we can iterate on it for a bit.

I think that a change like this would need the following tests:

• Narrowing from object
  • to an object type (e.g. instanceof, type predicates)
  • to a primitive (e.g. typeof foo === "number", type predicates)
• Narrowing from object | null | undefined (under strictNullChecks)
  • With == null
  • With === null
  • With typeof foo === "object"
  • With typeof foo === "undefined"
• Assigning (under strictNullChecks)
  • A null to something of type object.
  • A undefined to something of type object.
  • Something of type Number, String, or Boolean to something of type object.
• Trying to access members (under noImplicitAny and strictNullChecks)
  • that exist on Object.prototype (e.g. toString, hasOwnProperty)
  • that don't exist on Object.prototype (e.g. abcdef)

Most of these should all run under noImplicitAny and strictNullChecks personally.

[Herrington Darkholme (HerringtonDarkholme)]

Thanks for your quick response and kind suggestion!

I'm quite unsure about the behavior of narrowing behavior.

For example, an empty class like class A {}, should instanceof narrow object to A? Then other primitives will be assignable to object in that branch. Also, should typeof foo === "number" yield never type? I think that will be reasonable.(But I didn't add that test since #1809 does not specify it and TS has changed a lot since then.)

Thanks for the review! I will research other issues!

[Daniel Rosenwasser (DanielRosenwasser)]

For example, an empty class like class A {}, should instanceof narrow object to A? Then other primitives will be assignable to object in that branch.

If I'm not mistaken TypeScript uses the declared type (i.e. the original type) when getting the left side of an assignment. In those cases, the type should still be object, which should in turn should save you.from that concern, but it'd definitely be very helpful to have a test for this.

[Ali Taheri Moghaddar (alitaheri)]

This is a great feature, specially when dealing with generic constraints. Can it fully cover the bellow case?

interface Proxy<T extends object> {
  // ...
}

// fails:
const a: Proxy<number> = { ... };
const c: Proxy<null> = { ... };
const c: Proxy<undefined> = { ... };

// won't fail:

interface Blah {
  foo: number;
}

const b: Proxy<Blah> = { ... };

[Herrington Darkholme (HerringtonDarkholme)]

Thanks for all your valuable suggestion!
Ali Taheri Moghaddar (@alitaheri) Proxy<null> will only fail under strictNullChecks. I think a test is needed here.

[Claudia Meadows (dead-claudia)]

Question: what about functions? They're also reference types that can be Object.created and Object.assigned to.

---

Edit: I don't know what I'm talking about... they implicitly subtype Function.

[Nathan Shively-Sanders (sandersn)]

I like the semantics but I agree with Daniel Rosenwasser (@DanielRosenwasser) that (1) we need to have the whole team discuss at a design meeting (2) nonPrimitiveType should be a simple marker type, more like stringType, numberType etc and then add a branch in getApparentType. IntrinsicType with TypeFlags.Object might even work.

[Nathan Shively-Sanders (sandersn)]

typo in filename: Priimitive

[Herrington Darkholme (HerringtonDarkholme)]

Thanks!

[Nathan Shively-Sanders (sandersn)]

A few comments on the tests

[Nathan Shively-Sanders (sandersn)]

what about a = x and a = y? I think neither should have errors.

[Herrington Darkholme (HerringtonDarkholme)]

Added

[Nathan Shively-Sanders (sandersn)]

line 51 and 52 are only ok because strictNullChecks is not turned on, right?

[Herrington Darkholme (HerringtonDarkholme)]

Yes, and there are corresponding tests in nonPrimitiveStrictNull.ts which produces errors.

[Nathan Shively-Sanders (sandersn)]

except for null and undefined, I don't think these tests are that valuable, since they are just testing assignability again

[Herrington Darkholme (HerringtonDarkholme)]

Nathan Shively-Sanders (@sandersn) Pardon my question.
I have tried to replace nonPrimitive type by createIntrinsicType(TypeFlags.Object, "object"). But I get more errors in objectTypeRelatedTo, isFunctionObjectType and etc, where nonPrimitiveType is used as target and is directly compared rather than its apparent type. Currently I create nonPrimitiveType by reusing TypeFlags.Object so that all object type checking logic is reused.

I wonder if nonPrimitiveType can be labelled as intrinsic. One intrinsic type needs to use one bit flag like Number or String which I think is too expensive for non primitive checking.

Maybe I can change the TypeFlags? Or patching more functions?

[Nathan Shively-Sanders (sandersn)]

Yeah, I talked to Anders Hejlsberg (@ahejlsberg) and he said that object needs a new TypeFlags entry of its own. This probably means that you'll have to handle it specially in more places. I think, for most of these places, it's a good idea to have object-specific code in place, though.

[Herrington Darkholme (HerringtonDarkholme)]

Thanks for the clarification! I'm quite hesitant about adding TypeFlags since TypeScript will have more features like variadic type parameter and exact type, which all need TypeFlag entries.

I think I will wait for your whole team discussion.

[Herrington Darkholme (HerringtonDarkholme)]

I have updated the implementation to use intrinsic type. One thing I'm not sure is whether intrinsic object should be counted as StructuredType. It seems StructuredType flag is used in narrowing type, so adding nonPrimitive to StructuredType enables narrowing object by instanceof.

But if intrinsic type is used in resolveStrucutredTypeMembers, intrinsic type will be returned as resolvedType while it isn't. (I didn't managed to create a test case to cover that.)

I'm very grateful to you for all your review! 😃

Update:
TSLint breaks CI. Awaiting #13006

[Nathan Shively-Sanders (sandersn)]

I don't think you should re-use StructuredType either -- it's a generalisation of Object that includes any type that will eventually have members but may not yet. I think the object intrinsic should stay separate.

[Herrington Darkholme (HerringtonDarkholme)]

Thanks for your advice! I have changed nonPrimitive to Narrowable from StructuredType.
Also rebased.

[Nathan Shively-Sanders (sandersn)]

Thanks so much Herrington Darkholme (@HerringtonDarkholme)!

(And sorry for the delay from the holidays.)

[Gabriel-Calin Lazar (glazar)]

One common task is to take an existing type and make each of its properties entirely optional.
Thus we have support for Partial since 2.1.

The issue with using Partial is that any usages of Partial also accepts primitives, even though the intent is that primitives be excluded.

Is this new object type, added with 2.2, used by default when using Partial construct ?

It would make sense to make use of it, since it would solve the issue of usages of Partial also accepting primitives.

[Nathan Shively-Sanders (sandersn)]

Gabriel-Calin Lazar (@glazar) That seems reasonable. Can you create an issue for that so that we can track it separately? The PR should be simple.

[Marin Marinov (gcnew)]

If that were the case, wouldn't it break the currently used "deep partial" implementations? The same applies to Readonly, I think.

[Nathan Shively-Sanders (sandersn)]

Marin Marinov (@gcnew) can you give an example of a deep partial type? The Partial<T> in lib.d.ts looks shallow to me.

As I recall, the basic problem with a recursive partial type is that we have no way to stop the recursion right now, so it does in fact recur right though primitives, chopping them up into length, charAt, charCodeAt, etc. Eventually I suspect that mapped types will add conditionals so you can stop the recursion upon reaching primitives.

[Gabriel-Calin Lazar (glazar)]

Nathan Shively-Sanders (@sandersn) Marin Marinov (@gcnew)
I have created a dedicated issue for the situation I mentioned in my previous comment - #14224

[Marin Marinov (gcnew)]

Nathan Shively-Sanders (@sandersn) I don't have anything specific in mind. I think I've seen people asking question and submitting hacky code that might be affected. On a more serious note, will it work correctly if the target type has type parameters?