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

[Feature request] Annotating function params with ... before last arg #33

Closed
adriweb opened this issue Jan 6, 2021 · 6 comments
Closed
Labels
wontfix This will not be worked on

Comments

@adriweb
Copy link

adriweb commented Jan 6, 2021

Hi,

I was wondering if it would be possible to support annotating functions taking n variadic args then a known fixed type.

Basically I have this utility function, which unfortunately can't be perfectly annotated AFAIK:
(ignore the lack of @generic, that's another topic for now, but see the bottom)

--- Convert the function taking a callback into a function returning a promise.
--- The function to wrap must take a callback as last parameter.
--- The callback must take an error as first parameter, or nil if no error, and the result as second parameter.
function promisify(wrappedFn) ... end

What I tried and what would seem to be the optimal annotation but isn't supported (with ... or ...:any):

--- @param wrappedFn fun(..., cb:fun(nilOrErr:nil|string, result:any)) The function to wrap.
--- @return fun(...):Promise The function converted to return a promise.

What I ended up doing for now, which is the best I could find:

--- @param wrappedFn fun(arg:any, cb:fun(nilOrErr:nil|string, result:any)) The function to wrap.
--- @return fun(...):Promise The function converted to return a promise.

Thanks for the awesome work as usual!


Note that I also tried the following, which seemed like an appropriate usage of @generic, although that triggers errors in usages of that function, where T isn't matched to the actual usage I have for some reason. Maybe I should make another specific issue for that though.

--- @generic T
--- @param wrappedFn fun(arg:T, cb:fun(nilOrErr:nil|string, result:any)) The function to wrap.
--- @return fun(arg:T):Promise The function converted to return a promise.

Or even more optimally with that feature request implemented, although it's even more complex because all the variadic args may have different types... Ideally it would just "forward" the type from the wrapped function to the new one, but that sounds quite complex to annotate.

--- @generic ...anyT
--- @param wrappedFn fun(...:anyT, cb:fun(nilOrErr:nil|string, result:any)) The function to wrap.
--- @return fun(...:anyT):Promise The function converted to return a promise.
@Benjamin-Dobell
Copy link
Owner

Benjamin-Dobell commented Jan 6, 2021

I was wondering if it would be possible to support annotating functions taking n variadic args then a known fixed type

As far as type systems go, any such type would be inherently unsound because it's impossible to know how many arguments are required to call such a function.

It sounds more like you're after some meta-type functionality which would allow you to derive new types from other types. In this case, specifically altering a parameter list.

You can see some rather creative solutions people have come up with for TypeScript:
https://stackoverflow.com/questions/60323726/typescript-add-one-argument-to-a-functions-params

Whilst this certainly is not impossible to implement, unfortunately, it's honestly far more sophisticated than anything I'm planning.

If you've any control over the API in question I'd be inclined to strongly suggesting avoiding writing an API that appends parameters to a parameter list. It seems unnecessarily complicated / error prone, particularly when optional parameters / overloads get involved. Prepending parameters would be much more manageable.

@Benjamin-Dobell Benjamin-Dobell added the wontfix This will not be worked on label Jan 6, 2021
@adriweb
Copy link
Author

adriweb commented Jan 6, 2021

Alright, thanks for the detailed reply - I don't actually disagree with you, but had a bit of hope just in case :D

And yeah unfortunately it's an external lib, as you can see for yourself: https://github.com/luvit/luvit/blob/master/deps/fs.lua (the async functions)

@Benjamin-Dobell
Copy link
Owner

Was just thinking about this some more. No intent to implement it, however it's probably not quite as unsound as I claimed. At least, it's not so much an issue for callers as it is for implementers. Basically the inferred parameter types aren't very useful:

---@alias WeirdCallback fun(a: string, ...: T, c: string): void

---@type fun(weirdCallback: WeirdCallback): void
local callWeirdCallback

callWeirdCallback(function(a, b, c) -- INFERRED fun(a: string, b: string | T, c: nil | string | T): void
end)

Of course you probably wouldn't provide a callback with explicit parameters, but instead you'd use .... However, use of ... isn't type-safe unless you've got an absurd amount of functionality.

Even if the IDE was to infer ... as the type (string, ...T, string) you can't actually access that last string by any simple means that an IDE is going to understand because T may include nil and thus you need to work with table.pack rather than the table length operator.

Anyway, I just thought this was interesting. It's not impossible to implement, just impractical 😛

@adriweb
Copy link
Author

adriweb commented Jan 6, 2021

Right, what they do in TS is quite complex indeed :P
Basically, we'd need to "forward" type definition/safety from a signature to another... at an arbitrary location (before other args, in this case). It's definitely something a bit exotic and too complex for now I guess.

@adriweb
Copy link
Author

adriweb commented Jan 13, 2021

Interestingly, TypeScript 4.2 just got the feature to have variadic/rest args in other positions than the end: https://devblogs.microsoft.com/typescript/announcing-typescript-4-2-beta/#leading-middle-rest-elements-in-tuple-types 😁

@Benjamin-Dobell
Copy link
Owner

Benjamin-Dobell commented Jan 13, 2021

On the plus side, I totally beat them to the smarter type alias preservation feature 😆

---@alias BasicPrimitive number | string | boolean

---@param value BasicPrimitive
function doStuff(value)
    if (math.random() < 0.5) then
        return nil
    end

    return value
end

Screen Shot 2021-01-14 at 12 49 13 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants