string|nil passed to string parameter not flagged as error with strict settings
How are you using the lua-language-server?
Command Line
Which OS are you using?
Linux
What is the issue affecting?
Type Checking
Expected Behaviour
With the following config (see below), I expect Lua Language Server to report a type mismatch when passing a string|nil value to a function that expects a non-optional string parameter.
Specifically, this line should raise a diagnostic:
test_func(params.ip) -- where params.ip is string|nil, but test_func expects string
Actual Behaviour
No error is reported by LLS, neither in VS Code nor when using the CLI with --check.
Reproduction steps
- Save this code in a
.luafile:
---@class TestParams
---@field ip string | nil
---@param ip string
---@return string
local function test_func(ip)
return ip
end
---@type TestParams
local params = { ip = nil }
test_func(params.ip)
- Use the following
lua-language-server config(.luarc.jsonor through your client):
{
"runtime": {
"version": "Lua 5.1"
},
"diagnostics": {
"enable": true,
"disable": [],
"groupFileStatus": {
"ambiguity": "Any",
"await": "Any",
"codestyle": "Any",
"duplicate": "Any",
"global": "Any",
"luadoc": "Any",
"redefined": "Any",
"strict": "Any",
"strong": "Any",
"type-check": "Any",
"unbalanced": "Any",
"unused": "Any"
},
"groupSeverity": {
"ambiguity": "Error",
"await": "Error",
"codestyle": "Error",
"duplicate": "Error",
"global": "Error",
"luadoc": "Error",
"redefined": "Error",
"strict": "Error",
"strong": "Error",
"type-check": "Error",
"unbalanced": "Error",
"unused": "Error"
}
},
"type": {
"weakNilCheck": false
}
}
- Run the CLI:
lua-language-server-3.15.0-linux-x64/bin/lua-language-server --check your_file.lua
Or use VS Code extension.
Additional Notes
No response
Log File
Diagnosis completed, no problems found
That's really strange đ¤ . If we just use a plain variable, it can flag param-type-mismatch correctly:
---@param ip string
---@return string
local function test_func(ip)
return ip
end
---@type string|nil
local s
test_func(s) --> param-type-mismatch: Cannot assign `string|nil` to parameter `string`. â
expected
---@type { ip: string|nil }
local t
test_func(t.ip) --> no warning â bad
Then I looked into the source code of this param-type-mismatch diagnostic and found this:
https://github.com/LuaLS/lua-language-server/blob/32fec3cc99af8b9a1e45c2455a8c3bd0d3e38f66/script/core/diagnostics/param-type-mismatch.lua#L99-L105
Translating the comments within gives:
-- Since it is not possible to narrow the field type, -- remove the false value and then check
The :setTruthy() just strip the |nil part of the value of a getfield operation, so t.ip becomes just string and passes the test...
I am not quite sure what this line is trying to fix, but after removing it, param-type-mismatch for t.ip does show up correctly.
However I believe this logic exists for some reasons đ and it would be a bad idea to remove it directly without knowing the reason behind.
From git blame, this logic tries to "fix" the following case https://github.com/LuaLS/lua-language-server/commit/8c56e6ba9d85f3e01fb3658901386ee89de847ab#diff-a4ea140f144f56418be957919190bea33b3d1bd18973064d994b18d08f19b2ebR383-R397
---@class A
---@param n A
local function f(n)
end
---@class B
local a = {}
---@type A?
a.x = XX
f(a.x)
my educated guess
- although
a.xis marked with?=>|nil - but after an unknow assignment
= XX, it might have value - and extension author does not want to trigger
param-type-mismatchwhen callingf(a.x)in this case - thus the
:setTruthy()logic is added
I am not sure if this rationale is justified đ
In my project, I would instead setup "type.weakUnionCheck": true, and "type.weakNilCheck": true, to allow passing variable with |nil to a function which doesn't accept |nil.
So removing this line at least would affect my projects đ but it may affect many others.
Maybe there should be another option (or just use type.weakNilCheck) to toggle this behavior? đ¤
edit
đŽ I think I know the rationale behind
---@type string|nil
local s
test_func(s) --> param-type-mismatch: Cannot assign `string|nil` to parameter `string`. â
expected
s = "" --> now it becomes `s: string`
test_func(s) --> no warning â
expected
---@type { ip: string|nil }
local t = {}
t.ip = "" --> but this line cannot do type narrowing on `t.ip`
test_func(t.ip) --> so it will trigger **false positive** warnings
- by adding the
:setTruthy()logic => the server avoids false positive warnings - but of course it will now have false negative
- that might be a design choice by maintainers đ¤
edit2
After testing around a bit, I guess the existing design choice is right... Since currently the language server cannot type narrow a table field => you cannot easily suppress the false positive warnings above đ¤Ļââī¸
---@type { ip: string|nil }
local t = {}
if t.ip ~= nil then
test_func(t.ip) --> this will still throw warning, which is undesired
end
Apart from using ---@diagnostic disable-next-line: param-type-mismatch
(which is a bad idea I think)
The only way that I can think of, is to use a local variable to do the nil check
---@type { ip: string|nil }
local t = {}
local s = t.ip --> s: string|nil
if s ~= nil then
--> s: string
test_func(s) --> now it passes the test
end
So personally I go for the existing false negative behavior, unless LLS is capable to type narrow a table field. âšī¸