Calling upper on string|number

[kajacx]

Hello, I just installed this as a VSCode extension and tested it out with this typegate scenario:

local nextValue = math.random() > 0.5 and "hello" or 42

print(nextValue:upper())     -- type: string|number, but NO ERROR
if type(nextValue) == "string" then
    print(nextValue:upper()) -- type: string, correctly no error
else
    print(nextValue + 1)     -- type: number, correctly no error
end

but as you can see, it doesn't report an error when calling the "upper" function on the value of type string | number.

Maybe I don't have it set up correctly, but when I change it to simply local nextValue = 42 then it does report an error, so the setup seems to be correct. Am I missing something?

Edit: The type: info in the comment is what I get when I mouse over the variable.

[tomlau10]

This issue has been reported in #2842

---

IIUC, this is a limitation in the current type infer system:

1. when the nextValue is of type string|number, and you write nextValue:upper()
2. at first the server finds the string.upper function for it, and treat this statement as string.upper(nextValue)
3. then it tries to check if the param type of nextValue matches that of string.upper
4. however, the built-in annotation for string.upper() is function string.upper(s: string|number) -> string
   

   lua-language-server/meta/template/string.lua

   Lines 156 to 159 in 24b9faa

   	---@param s string|number
   	---@return string
   	---@nodiscard
   	function string.upper(s) end

5. that means it "accept" the param to be number type as well
6. => and finally the server doesn't report it as an error 😇

[kajacx]

Ok, I wanted to add a failing test as the first thing, but I didn't find how tests that show warnings work. I did find these tests that verify what type gets shown when hovering over a variable:

TEST 'string' [[
local x
x = '1'
print(<?x?>)
x = 1
]]

and I was able to break them, so I know I am running the tests successfully, but I couldn't find where are the tests for generating a warning in case of doing something bad.

I tried looking up the "Need check nil." text which led me to the DIAG_NEED_CHECK_NIL translation key, but it doesn't seem (at least from what I can tell) that there is an automated test for this warning being shown, at least not this nil warning specifically.

[tomlau10]

I have always called that an intersection, noted by A & B

Yeah! This keyword definitely rings a bell for me!
I remember seeing this in a feature request issue, where maintainer said this will be implemented in the future LuaLS v4.0 👀
#3152 (comment)

---

but I couldn't find where are the tests for generating a warning in case of doing something bad.

I am not sure if I understand you correctly 😕 You want a test script to expect a warning in specific position/variable ?

• the script you posted uses <?x?>, I believe it means capture the type of the variable and then TEST it with 'string'
• for expecting a warning, I believe it is <!x!>
• you may try to look into test/diagnostics/* files to see more examples of a specific diagnostic

Hope this helps 👀

[kajacx]

Thank you, that seems to be what I was looking for, I didn't find those diagnostic tests earlier.

Now when I am looking at it, I did find a need-check-nil diagnostic, but it seems to be completely unrelated to the "Need check nil." warning when you try to use a nil-able variable. On the other hand, the need-check-nil code does show up when I hover over the problem:

I think I might have a wrong version of the code? I forked master approximately a week ago. This is the only piece of code where the DIAG_NEED_CHECK_NIL translation key (which is the only thing connected to the "Need check nil." message) is mentioned:

        if node:hasFalsy() and not vm.getInfer(src):hasType(uri, 'any') then
            callback {
                start   = src.start,
                finish  = src.finish,
                message = lang.script('DIAG_NEED_CHECK_NIL'),
            }
        end     

And that code has no link to the need-check-nil diagnostic. My best bet is that the translated message gets back-translated to the DIAG_NEED_CHECK_NIL key, and that key gets "massaged" to need-check-nil with some search-and-replace, but that seems so cursed.

This may seem like a tangent, but I can't add a diagnostic test in good faith if I don't understand how they are supposed to work.

[tomlau10]

• Full searching DIAG_NEED_CHECK_NIL gives the file script/core/diagnostics/need-check-nil.lua.
  AFAIK each file in script/core/diagnostics/* is responsible for checking one type of diagnostics.
• I don't fully understand the testing framework either 😕
  but seems the bootstrap part lies in test/diagnostics/init.lua
  where it defines the TEST() function and then require all those test/diagnostics/* files
  => then let the server to run on each test script individually and capture the diagnostic for each script.

I did find a need-check-nil diagnostic, but it seems to be completely unrelated to the "Need check nil."

I am not sure if I understand this 🙈

• do you mean a test case in test/diagnostics/need-check-nil.lua is unrelated to need-check-nil ?
• or you are saying that the snippet you posted print(maybeString:upper()) should not have need-check-nil ?

For the first one test cases contain both positive and negative cases, i.e. some cases are expected not to throw errors, while others do
As for the latter one, I think you snippet should throw error as maybeString is of type string?, which does need a nil check

[kajacx]

Oh, it's the filename that links them together, I didn't think of that.

do you mean a test case in test/diagnostics/need-check-nil.lua is unrelated to need-check-nil ?

Yes, I did not understand how the "Need check nil." translated message was related to the need-check-nil diagnostic code, now I see it's linked via the filename.

or you are saying that the snippet you posted print(maybeString:upper()) should not have need-check-nil ?

No, that should definitely have the nil warning. Sorry if I said it in a confusing way.