lua-language-server icon indicating copy to clipboard operation
lua-language-server copied to clipboard

string|nil passed to string parameter not flagged as error with strict settings

Open andy265 opened this issue 6 months ago â€ĸ 2 comments

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

  1. Save this code in a .lua file:
---@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)

  1. Use the following lua-language-server config (.luarc.json or 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
  }
}
  1. 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

andy265 avatar Jul 07 '25 10:07 andy265

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.

tomlau10 avatar Jul 08 '25 01:07 tomlau10

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.x is marked with ? => |nil
  • but after an unknow assignment = XX, it might have value
  • and extension author does not want to trigger param-type-mismatch when calling f(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. â˜šī¸

tomlau10 avatar Jul 08 '25 01:07 tomlau10