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] Implicit casts for common conversions #3027

Open
emmericp opened this issue Jan 4, 2025 · 4 comments
Open

[Feature request] Implicit casts for common conversions #3027

emmericp opened this issue Jan 4, 2025 · 4 comments

Comments

@emmericp
Copy link
Contributor

emmericp commented Jan 4, 2025

I got some code for parsing command line flags/user inputs that commonly uses patterns like this

---@return string ...
local function strsplit(str, sep) end

local a, b, c = strsplit(userinput, ",")
b = tonumber(b) or 0 -- error:  This variable is defined as type `string`. Cannot convert its type to `number`

The fix is to define b as string|number in the declaration, but it's a bit cumbersome, especially if I just want b as a number:

local a,
---@type number|string
b, c = strsplit(userinput, ",")

So it would be nice if common type conversion patterns like this could be recognized automatically and either allow the cast or infer the type as string|number from the beginning.

Edit: See comment below, the implicit cast is already happening, this is really about type inference based on later assignments. Which may or may not be a good idea.

@tomlau10
Copy link
Contributor

tomlau10 commented Jan 5, 2025

IMO an explicit cast should be needed for this kind of use case.
But now the problem is that even I use ---@cast b +number (add number type to it, rather than just replacing the type`), still there will be a warning 😕

local b = ""

---@cast b +number
print(b) --> string|number

b = tonumber(b) or 0 --[[>
This variable is defined as type `string`. Cannot convert its type to `number`.
- Type `integer` cannot match `string`
- Type `number` cannot match `string`Lua Diagnostics.(cast-local-type)
]]
  • seems that currently cast-local-type diagnostic doesn't use the latest casted type of that variable 🤔
  • instead it just uses the type when setting the local variable:
    for _, ref in ipairs(loc.ref) do
    if ref.type == 'setlocal' and ref.value then
    await.delay()
    local refNode = vm.compileNode(ref)
    local value = ref.value
    if value.type == 'getfield'
    or value.type == 'getindex' then
    -- 由于无法对字段进行类型收窄,
    -- 因此将假值移除再进行检查
    refNode = refNode:copy():setTruthy()
    end
    local errs = {}
    if not vm.canCastType(uri, locNode, refNode, errs) then

If that is fixed, then we can at least have something like

local b = ""

---@cast b -string,+number
b = tonumber(b) or 0

@emmericp
Copy link
Contributor Author

emmericp commented Jan 6, 2025

IMO an explicit cast should be needed for this kind of use case.

Agreed, implicitly casting is probably not exactly sound. Not sure how other dynamic languages with types as an "afterthought" handle this situation.

However, note that the implicit cast wouldn't be new behavior, it already does that automatically! So my original request is poorly phrased, it's really about the type inference (and I guess the bug you are pointing out). For example:

---@param num number
local function onlyTakesNumbers(num) end

---@type number|string
local var = ""
onlyTakesNumbers(var) -- Error: Cannot assign `string|number` to parameter `number`.
var = tonumber(var) or 0 -- no explicit cast needed
onlyTakesNumbers(var) -- no errors here, var is magically just number

If the type was inferred as number|string here based on the later assignment everything would just work magically :)
Of course just inferring a different type based on assignments is probably not a good idea and defeats the point of type checks to some degree, but if this was done only for assignments that are conversions, then that should be sound.

@CppCXY
Copy link
Collaborator

CppCXY commented Jan 6, 2025

I suggest redefining a variable with the same name to resolve the conflict, which is very common in Rust.

local var = "123131"
local var = tonumber(var)

@tomlau10
Copy link
Contributor

tomlau10 commented Jan 6, 2025

I suggest redefining a variable with the same name to resolve the conflict, which is very common in Rust.

But then this will violate another rule: redefined-local
Our team also setup luacheck in projects and it will check for redefined local as well. 🫠

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

No branches or pull requests

3 participants