Enable LuaCheck to report common whitespace issues #81
Conversation
n1tehawk
force-pushed
the
check_whitespace
branch
from
388f8b9
to
27324b6
Compare
|
Thank you for your work! I'll post some comments now, let me know if you want me to merge this as is and tweak things myself. |
| @@ -33,7 +33,7 @@ script: | |||
| - lua -e 'package.preload.lanes=error;package.path="./src/?.lua;./src/?/init.lua;"..package.path' -lluacov bin/luacheck.lua --version | grep 'Not found' | |||
| - lua install.lua path/to/luacheck | |||
| - mv src src2 | |||
| - path/to/luacheck/bin/luacheck spec/*.lua | |||
| - path/to/luacheck/bin/luacheck spec/*.lua --ignore 612 | |||
There was a problem hiding this comment.
This is needed for that one long string literal in a test, right? It would be better to ignore this warning specifically using inline options ("-- luacheck: ignore 612").
|
|
||
| function inconsistent_indentation() | ||
| return "Don't do this" | ||
| end |
There was a problem hiding this comment.
This file shouldn't produce unrelated warnings. Functions can be made fields of a local table which is returned at the end.
There was a problem hiding this comment.
Thanks! I had tried local functions, but overlooked the obvious solution of returning a "module" table.
|
|
||
| it("detects whitespace issues", function() | ||
| local warnings = check( | ||
| io.open("spec/samples/bad_whitespace.lua", "rb"):read("*a") |
There was a problem hiding this comment.
Tests in check_spec mostly operate on strings. Checks on files go into cli_spec, where error messages are also tested.
|
I'm fine with correcting these myself, if you don't mind me force-pushing a bit to this branch. I'll let you know when I consider it 'final' for merging. |
n1tehawk
force-pushed
the
check_whitespace
branch
from
27324b6
to
3615056
Compare
Otherwise LuaCheck would now FAIL its self-test. :D Note: spec/lexer_spec.lua contains a trailing space that is to be kept. Add an "inline" option so that LuaCheck won't complain there.
While at it, fix samples file count for cli_spec.lua accordingly.
This explicitly checks for the expected warnings, and improves code coverage during the "busted -c" run.
n1tehawk
force-pushed
the
check_whitespace
branch
from
3615056
to
6172b5f
Compare
|
Done. You may review and/or merge now... Regards, NiteHawk |
|
Merging, thanks! I plan to make a new release within a week. |
|
Great. 😃 The documentation needs to be updated accordingly. If you want me to, I'm happy to help with that (assuming that it would mainly affect Regards, NiteHawk |
|
Yes, that would only affect |
|
Shouldn't we mention the "long string literals" issue (as a caveat), and that it may require an (inline) |
|
I don't think we should, I'd rather open an issue for it. |
|
You may cherry-pick from n1tehawk@9c3280b. Or want me to create another PR for it? |
|
I'll cherry-pick it, thanks! |
|
Since this was merged we're seeing an error like this: Is this because we need to wait for luarocks to get an updated luacheck package? Our fault for not pinning to a specific version, of course--just wondering if this is expected. Thanks! |
|
I had modified the .rockspec in the repo to include the new sourcefile - a local test with Are you building from a scm .rockspec "out there" in the rocks repositories? That might actually lack behind, and if it causes the build to retrieve 'fresh' files from GitHub for the outdated set of files (i.e. not including whitespace.lua), then check.lua will reference a 'non-existent' file... Regards, NiteHawk |
|
I have restarted a Travis build for a project that uses Luacheck via the standard |
|
We're doing this specifically: i.e. pointing to this version of the rockspec: |
|
@justinmk if you really want to use bleeding-edge version you should get the rockspec from master branch, not a fixed commit, like neovim does now. (And the correct way to install from a specific commit is cloning that commit and running "luarocks make" from project directory.) The reason is that running "install" or "build" on an scm rockspec fetches sources from master but uses build description from the rockspec itself, which may be outdated. I would recommend pinning the version or at least installing latest release though. |
|
@mpeterv That makes sense, thanks a lot. |
mpeterv/luacheck#81 (comment) > If you really want to use bleeding-edge version you should get the > rockspec from master branch, not a fixed commit ... > The correct way to install from a specific commit is cloning that > commit and running "luarocks make" from project directory. The reason > is that running "install" or "build" on an scm rockspec fetches > sources from master but uses build description from the rockspec > itself, which may be outdated.
See #79