Weak type detection

[RyanCavanaugh]

This implements the other suggestion from @ahejlsberg in #3755, and fixes a compiler bug found by the rule (in editorServices.ts).

A weak type is one which has only optional properties and is not empty. Because these types are assignable from anything except types with matching non-assignable properties, they are very weakly typechecked. The simple fix here is to require that a type is only assignable to a weak type if they are not completely disjoint.

Example:

interface TextOptions {
    alignment?: string;
    color?: string;
    padding?: number;
}
function drawText(opts: TextOptions) { ... }

// All of these are errors under this PR

// Also error under #3823
drawText({ align: 'center'} ); 

// Also error under #3823
function getDefaultOptions(): TextOptions {
    return { colour: 'blue'; };
}

// Not covered by #3823, but is covered in this PR
drawText(getDefaultOptions);
// Not covered by #3823, but is covered in this PR
drawText(getDefaultOptions());
// Not covered by #3823, but is covered in this PR
drawText(42);

[danquirk]

Note this would be a breaking change.

[JsonFreeman]

👍

[mhegazy]

@RyanCavanaugh can we close this.

[RyanCavanaugh]

Need to discuss at a design meeting to get a 👍 or 👎 on this

[xLama]

What about this?

interface TextOptions {
    alignment?: string;
    color?: string;
    padding?: number;
    length?: number;
}
function drawText(opts: TextOptions) { ... }
drawText( [] );

Would it show an error?

[RyanCavanaugh]

@xLama that would not be an error due to the corresponding length properties in both types.

[xLama]

👍

[xLama]

Will this be in 1.6?

[rotemdan]

Just encountered this:

let intendedToBeANonPrimitive: { val1?: number, val2?: string };
intendedToBeANonPrimitive = 45.234234; // Works?!

If the intention of the programmer was to allow the value to be assignable from a primitive, they would never have defined properties on it, either optional or required. If that was actually desirable, it is possible to use a union:

let canAlsoBeANumber: { val1?: number, val2?: string } | number;
canAlsoBeANumber= 45.234234; // OK

I encounter this problem very frequently with function "options" arguments, which are commonly defined to have all-optional properties.

I frequently convert an old API that looks something like this:

function getValues(startIndex?: number, endIndex?: number)

To a new one accepting an options object instead:

function getValues(options: { startIndex?: number, endIndex?: number })

Then unexpectedly I don't get any error on incorrect calls like these:

getValues(456);

And have to track them manually.

It would be great to have this fixed already in the 1.6 release. I personally consider this to be a bug. Anyway, keep up the great work :).

[DanielRosenwasser]

That issue surfaces pretty badly when .d.ts authors order overloads incorrectly and order an overload taking a string after an overload taking an options bag.

[JsonFreeman]

@DanielRosenwasser While I agree it's a bad problem, I don't think the overload ordering makes it that much worse because the initial pass of overload resolution uses subtype, not assignability. Subtype is tighter than assignability, and actually requires optional properties in the target to be present in the source, except when the source is an object literal. So a primitive, when passed to a function that expects an options bag, would fail subtype.

[DanielRosenwasser]

One temporary workaround I gave a user was the following

interface NoPrimitives {
    valueOf?: { __DoNotUse: {} }; // Incompatible with 'valueOf' in strings, numbers, and booleans
    test?: { __DoNotUse: {} }; // Incompatible with 'test' in regexes
}

 interface Schema extends NoPrimitives {
  type?: string;
  items?: Schema | Schema[];
  properties?: {[property: string]: Schema};
}

[kitsonk]

This appears to have been closed without merging (or at least it doesn't appear to be working re: #7485). Was that intended?

[mhegazy]

the issue still remains. the proposal is still under consideration, but there were no immediate plans to pursue it, that is why it is closed.

[kitsonk]

Is there an issue with a proposal that is open?

[mhegazy]

Not that i am aware of. @RyanCavanaugh ?

[RyanCavanaugh]

Nope.

[sandersn]

I merged master with this PR and immediately found an incorrect argument being passed from checker. I'll create a new PR based on a branch in Microsoft/TypeScript when I get all the bugs fixed and tests passing.

[sandersn]

I found an additional case that we may not want to report errors for:

class A { a? }
declare function foo(a: A): void;
function bar(faux: Object) {
  foo(faux) // Error: Object doesn't have any properties in common with weak type A
}

The author should have used faux: {} to avoid this error, but lots of our users don't know that.

The real example is more believable because foo is a type guard:

class A { a? }
class B extends A { b }
declare function isB(a: A): a is B;
function bar(faux: Object) {
  if (isB(faux)) { // Error: Object doesn't have any properties in common with weak type A
    // sad code ... should have said `faux: A` or `isB(a: any)`
  }
}