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

'Discarding Parameters' plus 'Duck Typing' makes every parameter optional? #9352

Closed
SlurpTheo opened this issue Jun 24, 2016 · 4 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@SlurpTheo
Copy link

SlurpTheo commented Jun 24, 2016

After reading the Comparing two functions section of the handbook I got myself pretty confused and concerned. I have an example below that I think (taken to the prepare-for-duck-typing-extreme) means I should code every class's method as if all of its parameters are optional (even though they're not defined as such).

TypeScript Version:

1.8.10 / next (1.9.0-dev.20160624-1.0)

Code

(function () { "use strict";


class C1 { one(...hi: string[]) { } }
class C2 { one(hi: string) { hi.length; } }

function Test() {
    const c2 = new C2();
    const c2as1: C1 = c2;   // BAD assignment

    // TypeError: Cannot read property 'length' of undefined
    c2as1.one();            // BAD execution

    // error TS2346: Supplied parameters do not match any signature of call target.
    //c2.one();

    // error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
    //                 Happens once you enable TypeScript 2.0's --strictNullChecks.
    //c2as1.one(void 0);
    //c2.one(void 0);
}


})();

Actual behavior:

I have to remember to write run-time checking of typeof hi === "string" ? hi.length : 0 instead of the simple/easy hi.length inside my C2#one(hi: string) method to prevent against a TypeError.

Expected behavior:

I'd love for there to be some way for TypeScript to stop the BAD assignment line above; maybe one that turns off 'discarding' parameters altogether?

The defense given for discarding parameters is a signature like Array#forEach -- where callers commonly omit the 2nd/3rd parameters to the callbackfn. That doesn't make sense to me though, why isn't the *.d.ts for Array#forEach just defined as:

forEach(callbackfn: (value?: T, index?: number, array?: T[]) => void, thisArg?: any): void;

Instead of how it is?

forEach(callbackfn: (value: T, index: number, array: T[]) => void, thisArg?: any): void;

Maybe my targeting of parameter discarding is too strong, but it really seems like there should be some way to configure TypeScript to block the assertion of a C2 instance to a C1 interface without requiring me to just write better code like:

class betterC1 { one(hi: string, ...restHi: string[]) { } }
@RyanCavanaugh
Copy link
Member

Your example here is specific to rest parameters, which get special treatment. Note that if you had written this with actually zero arguments, you'd get an error:

class C1 { one() { } }
class C2 { one(hi: string) { hi.length; } }

var c1 = new C1();
var c2 = new C2();
c1 = c2; // Error

The reason for this is that many callbacks provide a variable but predictable number of arguments. For example, String#replace will invoke with one argument per matching group in the RegExp, so code like this is very common:

var r = /(.*)_(.*)_(.*)/;
var x = 'a_b_c'.replace(r, (str, m1, m2, m3) => { ... });

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 24, 2016
@SlurpTheo
Copy link
Author

@RyanCavanaugh

Imagine C1#one actually has code that interacts with the ...hi: string[] rest parameter. As shown, sure the parameter could have just been omitted, but that's a problem with the specific example code I provided -- sorry about that.

String#replace gives ...args: any[] in the callback function parameter signature; i.e., not too much from a type safety perspective. It feels different to me than the class-based (--noImplicitAny and --strictNullChecks) example I gave.

If a class method is defined with a single rest parameter, 0 arguments from a CallExpression is perfectly legal. Assigning something to that class method's interface which can accept no less than 1 argument from a CallExpression feels very different than most the static type safety we are usually provided from TypeScript compilation.

Should methods defined directly on an object/class (maybe even something extra-restrictive/specific to the class keyword here?) have special/extra analysis compared to a function expression, a module function declaration, or class static method declaration? The two examples in this issue-thread which show the usefulness of parameter discarding are for callback function parameters; is it unique to them? Maybe it's just in those cases that function assignment should be more accepting of parameter omissions?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 27, 2016

It's a design-trade off in terms of complexity/understandability vs soundness.

The problem once you start special-casing these rules is that it's immediately unclear where to draw the line. For example, we believe this code to be OK, based on how it's used in the wild (concrete data point: this was illegal in one release of TypeScript and everyone complained about it or was confused)

declare function invoke<T>(arr: T[], cb: (...args: T[]) => void);

invoke([1, 2, 3], (a, b, c) => a + b + c);

Now let's say "Oh callbacks are special!". Then we write this code:

declare function invoke<T>(arr: T[], cb: { success: (...args: T[]) => void; fail?: () => void });

invoke([1, 2, 3], { success: (a, b, c) => a + b + c });

Maybe that's OK and maybe it isn't? It's not obvious that this is a callback anymore, but from the developer's perspective it's a trivial refactoring. It also breaks the basic symmetry that if T is assignable to U, then { x: T } is assignable to { x: U }, which is terrible in terms of explaining to someone why something happens ("Yes, X is assignable to Y, except if it's wrapped in an object, or is used in a derived class" ==> ❓ 😕 ❓).

Now we write this code:

declare class InvokeeBase<T> {
  invoke(...args: T[]): void;
}
declare function invoke<T>(arr: T[], cb: InvokeeBase<T>);

class MyInvokee extends InvokeeBase<number> {
  invoke(a: number, b: number, c: number) { ... }
}
invoke([1, 2, 3], new MyInvokee());

Again, it's a basic refactoring of the original code that we thought was totally fine.

@mhegazy mhegazy closed this as completed Jun 28, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants