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

[andy265]

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)

2. 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
  }
}

3. 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

[tomlau10]

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:

lua-language-server/script/core/diagnostics/param-type-mismatch.lua

Lines 99 to 105 in 32fec3c

	if arg.type == 'getfield'
	or arg.type == 'getindex'
	or arg.type == 'self' then
	-- 由于无法对字段进行类型收窄，
	-- 因此将假值移除再进行检查
	refNode = refNode:copy():setTruthy()
	end

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]

From git blame, this logic tries to "fix" the following case
8c56e6b#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. ☹️