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] Required nullable arguments #2070

Open
omarkmu opened this issue Apr 15, 2023 · 5 comments
Open

[Feature Request] Required nullable arguments #2070

omarkmu opened this issue Apr 15, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@omarkmu
Copy link

omarkmu commented Apr 15, 2023

When calling Java methods from Kahlua, the number of arguments is used to determine the appropriate implementation to call. Because of this, arguments that can be nil are required in the function call for appropriate resolution. This is usually not a concern in Lua, but it could be.

In this example, both f() and f(nil) are appropriate, since the value of x defaults to nil.

---@param x integer?
local function f(x) end

In this example, however, the behaviors of t.f() and t.f(nil) are not equivalent.
(Admittedly, this is a contrived example. Nonetheless, it shows that this would not only be useful for strange cases like Kahlua's implementation.)

---@class Example
---@field f fun(x: integer?)
t = {
    f = function(...)
        assert(select('#', ...) == 1)
    end
}

I don't feel strongly about the annotation that would be used to specify this, but ! comes to mind:

---@class Example
---@field f fun(x: integer?!)
t = {
    f = function(...)
        assert(select('#', ...) == 1)
    end
}

The ?!/!? pair may be redundant, since non-nil argument types are always required. It would also be feasible to represent required but nil-able arguments with ! alone.

@firas-assaad
Copy link
Contributor

I agree with the general idea. If it is implemented, introducing a special operator like ! for a special use case like this one could harm readability, especially since it's not clear what it does without consulting the documentation, and it has different meanings in other languages (e.g. in C# and Typescript it's a way to tell the compiler a value isn't null).

Perhaps another kind of nil can be introduced (e.g. fun(x: integer|<explicit_nil>)), or (based on a proposal I saw for ReadOnly<> specifiers): fun(x: Required<integer?>), or maybe something else, such as introducing a @required param annotation coupled with the idea of documenting inline functions over multiple lines.

@C3pa
Copy link
Contributor

C3pa commented Apr 16, 2023

What about changing the behavior of existing features? What if:

---@param x integer|nil
local function f(x) end

would require for a function to be called as f(12) or f(nil) while

---@param x integer?
local function f(x) end

would allow calling the function as f(12) or f() without warnings from the LLS?

To sum up, integer? would specify a really optional integer argument, while the integer|nil union would require the user to pass either an integer or nil.

@omarkmu
Copy link
Author

omarkmu commented Apr 16, 2023

I'm partial to Required<integer?> as mentioned by @firas-assaad, but I like the idea of changing the behavior of integer|nil, too. However, I'm concerned that this would cause existing code to produce warnings where it didn't before.

@firas-assaad
Copy link
Contributor

firas-assaad commented Apr 16, 2023

There are cases where I find using |nil more readable than ?, e.g. @param val string|string[]|integer|integer[]|nil, since the ? gets lost in long expressions with complex types.

On the other hand, I always felt that having three different ways to annotate optional parameters (x? : integer, x : integer?, and x: integer|nil) a little weird. Of those, only the first one is saying something about the parameter being optional, while the others are about specifying the type. If putting the question mark after the parameter name became the only way to say that a parameter is optional then I'll be fine with it (and I actually think it makes more sense)

So right now all of these functions behave similarly when being called without any arguments:

---@param x integer?
local function f1(x) end

---@param x? integer?
local function f2(x) end

---@param x? integer
local function f3(x) end

---@param x integer|nil
local function f4(x) end

---@param x? integer|nil
local function f5(x) end

My idea is that f1 and f4 will now give a warning if called without explicitly passing a parameter. Existing code that relies on putting the question mark after the type will have to be updated, but it seems like a good way to differentiate between the concept of optional arguments and that of nil-able types.

Interestingly, the current type information when hovering is identical for all except for f4, which doesn't deduce that x is optional and doesn't complain about not passing a value explicitly either.

Anyway, this is mostly bikeshedding at this point, assuming the feature can be implemented. :)

@C3pa
Copy link
Contributor

C3pa commented Apr 17, 2023

I also agree that it's not nice to break previously working annotations, but that's when a major version bump should be made in semantic versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants