Closed ColinKennedy closed 2 weeks ago
First, I’d like to clarify that LLSCheck is a wrapper around the Lua Language Server, so this issue is related to LuaLS, not directly LLSCheck itself.
That said, LuaLS supports definition files, which help it recognize functions and methods added by external libraries like Busted and Luassert. These definition files inform the server about additional functions like assert.same, describe, it, etc.
Lua Language Server provides pre-built definition files for Busted and Luassert. You can enable these via the workspace.library setting in your .luarc.json
.
Here is an example working in vscode. I'm not familiar with neovim, not sure if the ${3rd} variable will work.
{
"workspace.library": [
"${3rd}/luassert/library",
"${3rd}/busted/library"
]
}
My understanding is that the ${3rd}
shortcut is deprecated across the board, though I don't know what the timeline is for its removal. (That said, I believe that the shortcut works in neovim, and my guess is that it will work there as long until the language server itself removes the shortcut.)
As an alternative, what I do is manually clone repos from the LuaCATS project and then add those repos to the workspace.library
list. (In fact, I put all the documentation repos in a single LLS-Addons
directory and add just that to the workspace.library
list. You may not want to put everything in one place if you worry about scanning too many repos every time you start the language server. So far, it has not been a problem for me.)
Thank you both, I'll try to get my project to a clean point where I can try these suggestions out soon.
In fact, I put all the documentation repos in a single LLS-Addons directory and add just that to the workspace.library
If you have a non-VSCode project setup that you can refer to would you mind sharing it here? I'll be looking into this next
I tried this out but am not getting the effect I'm hoping for.
The desired outcome:
.dependencies/
subdirectoryllscheck
once on the whole repository, including test files, to get a report
This is my WIP repository: https://github.com/ColinKennedy/nvim-best-practices-plugin-template and the llscheck
call is here
llscheck:
llscheck --configpath .luarc.json .
If I clone luassert
and busted
into a .depedencies/
subdirectory, they get included in the llscheck
report because the current command runs on everything.
.dependencies/luassert/library/luassert/mock.lua:16:10-13: cast-local-type: This variable is defined as type `luassert.mock`. Cannot convert its type to `function`.
- `function` cannot match `luassert.mock`
- Type `function` cannot match `table`
- Type `function` cannot match `luassert.mock`
.dependencies/luassert/library/luassert/stub.lua:13:10-13: cast-local-type: This variable is defined as type `luassert.stub`. Cannot convert its type to `function`.
- `function` cannot match `luassert.stub`
- Type `function` cannot match `table`
- Type `function` cannot match `luassert.stub`
I then tried to split the llscheck
command into several commands
llscheck:
llscheck --configpath .luarc.json lua/
llscheck --configpath .luarc.json plugin/
llscheck --configpath .luarc.json spec/
but of course that won't work because .
doesn't represent the files / subdirectories to check but the actual workspace root. As far as I can see there is no "llscheck please only check X Y Z subdirectories" CLI option.
So with that not working, I tried again to look at the the documentation for .luarc.json. workspace.ignoreDir
looked promising but I think that wouldn't have the effect I'm looking for. At least I couldn't figure it out.
Anyway I ended up doing a not-so-good solution which is to still call git clone on luassert + busted but instead put it to the directory above the current directory.
{
"Lua.diagnostics.libraryFiles": "Disable",
"Lua.workspace.checkThirdParty": "Disable",
"Lua.workspace.library": [
"../.dependencies/busted/library",
"../.dependencies/luassert/library",
],
"diagnostics.globals": [
"vim"
],
"runtime.version": "LuaJIT",
"workspace.checkThirdParty": "Disable"
}
And that worked. Pretty unclean though. Is there a way to get it all working while still cloning to the current directory / subdirectory? A working non-VSCode example would be much appreciated.
The only solution I can find is the one you don't like, namely to store the third-party libraries outside of your project directory.
For example, the following works for me.
{
"$schema": "https://raw.githubusercontent.com/sumneko/vscode-lua/master/setting/schema.json",
"diagnostics.libraryFiles": "Disable",
"diagnostics.globals": ["vim"],
"runtime.version": "LuaJIT",
"workspace.ignoreDir": [".dependencies"],
"workspace.library": ["/path/to/LLS-Addons"]
}
(For me /path/to/LLS-Addons
includes the busted and luassert directories.)
"workspace.ignoreDir"
works to ignore a directory. (I downloaded busted and luassert into .dependencies
, and I get no warnings if I include that "workspace.ignoreDir"
item as above.)
But "workspace.ignoreDir"
stops working if you also use that same directory as your "workspace.library"
. (I tried that, and the warnings about busted and luassert come back.)
You say the solution you found is "[p]retty unclean," but I'm not sure why. The third-party documentation is (1) not your project and (2) applies potentially to a lot of projects, so it makes sense to me that it belongs somewhere else.
All of that said, something does seem wrong to me. "diagnostics.libraryFiles": "Disable"
should exclude files added in "workspace.library"
from receiving diagnostics. That doesn't seem to work in this case, and I wonder whether that's because of llscheck
or because of the language server itself.
I find it unclean because for automation purposes and a consistent UX locally I shouldn't assume that a directory outside of a git repository is writable even if it's the parent directory. It's doable in most cases but that concern is still there.
Your example confuses me a bit. You'd said you were only able to get it to work when storing the 3rd party library outside of the project directory but the example appears to be inside the project. I tried out what I thought you meant with that and unfortunately it still errors so maybe it was my misunderstanding.
All of that said, something does seem wrong to me. "diagnostics.libraryFiles": "Disable" should exclude files added in "workspace.library" from receiving diagnostics. That doesn't seem to work in this case, and I wonder whether that's because of llscheck or because of the language server itself.
I wouldn't know the answer but maybe some more information that can help
I get these errors when I call llscheck --configpath .luarc.json .
llscheck --configpath .luarc.json .
.dependencies/luassert/library/luassert/mock.lua:16:10-13: cast-local-type: This variable is defined as type `luassert.mock`. Cannot convert its type to `function`.
- `function` cannot match `luassert.mock`
- Type `function` cannot match `table`
- Type `function` cannot match `luassert.mock`
.dependencies/luassert/library/luassert/stub.lua:13:10-13: cast-local-type: This variable is defined as type `luassert.stub`. Cannot convert its type to `function`.
- `function` cannot match `luassert.stub`
- Type `function` cannot match `table`
- Type `function` cannot match `luassert.stub`
And these errors come up whether "workspace.library"
is defined or not
{
"Lua.diagnostics.libraryFiles": "Disable",
"Lua.workspace.checkThirdParty": "Disable",
"Lua.workspace.library": [
".dependencies/busted/library",
".dependencies/luassert/library"
],
"diagnostics.globals": [
"vim"
],
"runtime.version": "LuaJIT",
"workspace.checkThirdParty": "Disable"
}
or
{
"Lua.diagnostics.libraryFiles": "Disable",
"Lua.workspace.checkThirdParty": "Disable",
"diagnostics.globals": [
"vim"
],
"runtime.version": "LuaJIT",
"workspace.checkThirdParty": "Disable"
}
To help, I've created a repository that reproduces your issue: https://github.com/jeffzi/luals-example. The README includes detailed steps to reproduce the problem. Could you test these steps and confirm if the conclusion matches your experience?
In short, the documentation for --check doesn't clarify whether the path should be full or relative. However, it seems the only combination that avoids checking library files stored in the workspace is to pass a relative path to --check and a full path to "workspace.library".
The issue you're encountering stems from llscheck always converting paths to full paths. I had initially implemented this behavior because it seemed more stable at the time. Since then, there have been multiple LuaLS releases that modified --check, but I haven't pinpointed the exact version where this behavior changed.
While I currently don't have the bandwidth to track down the versioning or support multiple LuaLS versions, I'll patch llscheck to address your specific use case. This fix should work with the latest version of the Lua Language Server.
Three quick notes:
lua-language-server
are you using? I get very different initial results with the test repo, but I'm using 3.7.4 from MacPorts. Maybe I should manually update to the 3.10.6 release.Please give your results with both 3.7.4 and 3.10.6, that will be helpful to pinpoint the issue.
If you try the pre-compiled versions on a mac, you'll need to disable macOS security with xattr -d com.apple.quarantine ~/Downloads/lua-language-server-3.7.4-darwin-arm64/bin/lua-language-server
Initially, I got lots more errors, but that's because I had a problem with the submodules. (The specifics don't matter, but it was my error not yours. Sorry.)
In any case, once I manually clone the submodules into annotations/
, I get the same exact results as you. I ran through all the variations you describe, and I tested on both 3.7.4 and with 3.10.6 (using the pre-compiled versions). At every step, I got the same results as you.
After some time away I got back on this (apologies for going silent)
I had same results as @telemachus. After some tinkering I got a working configuration.
Here are the requirements:
$PWD
. Specifically, in .luarc.json
, the line must be "workspace.library": ["$PWD/.dependencies"],
busted
/ luassert
[submodule ".dependencies/busted"]
path = .dependencies/busted
url = git@github.com:LuaCATS/busted.git
[submodule ".dependencies/luassert"]
path = .dependencies/luassert
url = git@github.com:LuaCATS/luassert.git
(Note: You do not actually need to create real git submodules. As long as the .gitmodules
file exists, lua-language-server
is happy)
lua-language-server --check .
- It must be a relative path like .
. An absolute path like lua-language-server --check $PWD
will fail to omit busted
/ luassert
from diagnostics results.If any of the 3 conditions above are off, you will get some sort of diagnostic false negative.
To wrap things up, this is definitely a lua-language-server
bug / issue. Requiring submodules is IMO is strange (what if people use Perforce or aren't using git or something) but I will take that up with the maintainers lua-language-server
. For the short term, we've got something working!
Let's close this GitHub issue. The bug is specific to lua-language-server
after all. Thank you both for your help! I will send the reproduction details https://github.com/LuaLS/lua-language-server to pay it forward :)
I spoke too soon - I was using a modified llscheck fork. When I went back to vanilla it errored. I'll make a PR
Thanks, @ColinKennedy! Just released v0.6.0 with your patch.
busted extends
assert
with extra functions such asassert.same
,assert.equal
, etc. These failllscheck
calls.assert.same({ "foo" }, { "bar" })
Gets this error
Busted also defines a number of functions such as
describe
,it
,after_each
,before_each
, etc. To disable those, I use the following .luarc.jsonThis works for
describe
,it
,after_each
,before_each
but not forassert.same
,assert.equal
, etc. Probably I'm guessing because Busted is extending Lua's built-inassert
and it isn't a "new" function.How should I go about ignoring issues for just these
assert.*
functions? I tried addingassert
,assert.same
, etc to thediagnostics.globals
with no luck.Ideally I'd like to avoid adding
--- @diagnostic disable: undefined-field
to all of my test files but that is what I've been doing to get around this issue so far.