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

Syntax suggestion: Ignore last void parameters #4260

Closed
teintinu opened this issue Aug 10, 2015 · 15 comments
Closed

Syntax suggestion: Ignore last void parameters #4260

teintinu opened this issue Aug 10, 2015 · 15 comments
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@teintinu
Copy link

class X<T>
{
    f(t: T) { return { a: t }; }
}


var x: X<number>;
var t1 = x.f(5);
t1.a = 5; // Should not error: t1 should have type {a: number}, instead has type {a: T}
var x2: X<void>; 
var t2a = x.f(void 0); // It works but void 0 it's unnecessary
var t2b = x.f(); // SUGGESTION: ignore syntax check of last argument if it's type is void
t2b.a = void 0; // Should not error: t2 should have type {a: void}, instead has type {a: T}

test case: tests/cases/compiler/genericObjectLitReturnType.ts

maybe the solution is the same for #3356

@weswigham
Copy link
Member

Would I be correct in saying that the type signature T|void should be equivalent to T?? (Any type which contains the void type is optional as a parameter) This equality is true in the closure compiler type system (but it uses undefined rather than void for the type name).

@teintinu
Copy link
Author

Not always @weswigham, just in last arguments of functions.

class X<T, U>
{
    f(t: T|void, u: U) { ... }
}

t: T it's mandatory because is not the last function argument and we can't lose u: U. My suggestion it's only for last argument. However, the rule can be "last void parameters" because, if X<void,void>, f function can be invoked with f() lossless

I don't understand when you say T|undefined (rather than T|void) equality is true in the closure compiler, can you send me a sample?

Thanks

@weswigham
Copy link
Member

Oh, sorry, I was (completely) mistaken - The equality's not true for optional function arguments, only optional record type members. :S (Optional function arguments in closure are denoted with an =, while optional members use ?, hence my mixup.)

@RyanCavanaugh
Copy link
Member

Chatting with @danquirk, our proposal (roughly) is that at a call expression, we treat the unprovided required arguments as if they were of type void, then check that those are assignable to the parameter types (modulo any). Leave assignability/subtyping of function types unchanged.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Aug 10, 2015
@teintinu
Copy link
Author

Yes @RyanCavanaugh, just for call expressions. I don't know how works typescript checker then I can't say if it's better don't throw error if last parameter is void and missing or if it's treat the unprovided required arguments as if they were of type void. But it's what you say, just in call expression leaving assignability/subtyping of function types unchanged.

If I can help you just tell what to do.
Thanks.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". and removed In Discussion Not yet reached consensus labels Oct 5, 2015
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 5, 2015
@RyanCavanaugh
Copy link
Member

Approved. Should be useful for Promise<void>.

@jack-williams
Copy link
Collaborator

@RyanCavanaugh What is the status on this issue? I saw that @Kingwl was working on a PR that was abandoned (#20642) with some spec issues raised by mhegazy.

Anyway, I have a fix that works for the given examples as well as generic rest / tuple types here 9f6e28b if you want to breathe some life into this issue.

@Kingwl : I didn't mean to conflict with your work (I didn't actually see your old PR until I finished my fix). If you want to take my work and finish the PR I would be fine with that.

@Kingwl
Copy link
Contributor

Kingwl commented Sep 14, 2018

@jack-williams take it easy, I have already close the pr

@FrogTheFrog
Copy link

@jack-williams It does not seem to work with constructors. Is this intentional or were constructors just forgotten?

@jack-williams
Copy link
Collaborator

@FrogTheFrog Can you provide an example?

@FrogTheFrog
Copy link

FrogTheFrog commented Nov 19, 2018

@jack-williams Here you go:

class Test<T1, T2 = void> {
    constructor(input1: T1, input2: T2) {
        // Do something
    }
}

const test = new Test({});

I'm getting this error with 3.2.0-dev.20181117:

test.ts:7:14 - error TS2554: Expected 2 arguments, but got 1.

7 const test = new Test({});
               ~~~~~~~~~~~~

  test.ts:2:29
    2     constructor(input1: T1, input2: T2) {
                                  ~~~~~~~~~~
    An argument for 'input2' was not provided.


Found 1 error.

@jack-williams
Copy link
Collaborator

jack-williams commented Nov 19, 2018

@FrogTheFrog Thanks for the example. The issue is not to do with constructors; the issue is the generics. See this comment here in an older PR I made.

For instance, this is fine:

class VoidClass {
    constructor(_x: void) {}
}

new VoidClass(); // ok

Basically the special casing for void does not apply to arguments that are instantiated as type void at the point the argument is called. Getting it to work with generics caused too many problems to ultimately be worth it.

Here is a workaround if you really want it:

class Test<T1, T2 = void> {
    constructor(input1: T1, input2: T2) {
        // Do something
    }

    static makeFactory<T1, T2 = void>(): (input1: T1, input2: T2) => Test<T1,T2> {
        return (input1: T1, input2: T2) => new Test(input1, input2);
    }
}

const factory = Test.makeFactory<number>();
const factoryTwo = Test.makeFactory<number, number>();
const testOne = factory(3); // ok
const testTwo = factoryTwo(3); // not ok

@FrogTheFrog
Copy link

@jack-williams I'm just nitpicking at this point. Since I mainly use "options" object for public classes, it does not really bother me. I'm using this simple trick fallback to void type when nothing is provided:

interface TestOptions<T1, T2> {
    input1: T1;
    input2?: T2;
}

class Test<T1, T2 = void> {
    constructor(options: TestOptions<T1, T2>) {
        if (options.input2 !== void 0) {
            // Do something with input2
        }
    }
}

const test = new Test({ input1: {} }); //=> test has Test<{}, void> type

I'm just glad that the void 0 nonsense is finally over thanks to you :D

@garkin
Copy link

garkin commented Jun 3, 2019

Why does this fail? Playground. npx tsc --version: Version 3.5.1

function qwe<T = void>(a: T) {}
qwe(); // ERROR: Expected 1 arguments, but got 0. function qwe<void>(a: void): void

function abc(a: void) {}
abc(); // Passes

@jack-williams
Copy link
Collaborator

@garkin

Basically the special casing for void does not apply to arguments that are instantiated as type void at the point the argument is called. Getting it to work with generics caused too many problems to ultimately be worth it.

The checks are done before T is replaced with void. This is tracked here: #29131

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants