mpeterv / luacheck

A tool for linting and static analysis of Lua code.
MIT License
1.92k stars 322 forks source link

Skip directories can't be scanned instead of aborting the whole check. #159

Closed spacewander closed 6 years ago

spacewander commented 6 years ago

Assumed we have directory structure like this: . ├── A │   └── B │   └── C └── D

If directory B could not be opened, previously luacheck will abort. Thus we have to run luacheck separately for directory C and D, and more.

With this patch, luacheck will continue to scan directory C and D.

mpeterv commented 6 years ago

It's a good idea to have this directory skipping behaviour, but simply writing the error to stderr doesn't work very well, other errors are more nicely formatted, and formatting can be different based on options (e.g. it could be XML when using JUnit formatter).

So extract_files should return all the errors somehow, and the filenames as well, and the calling function should handle them. I would keep returning an array of paths as the first return value, whether they had an error or not, and then also return a sparse array mapping indexes from the first table to error messages if for that path directory expansion failed. But I'd merge another implementation with the same behaviour, too.

spacewander commented 6 years ago

@mpeterv I will change this pull request once I have enough free time.

codecov-io commented 6 years ago

Codecov Report

Merging #159 into master will decrease coverage by 0.02%. The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
- Coverage   97.77%   97.75%   -0.03%     
==========================================
  Files          35       35              
  Lines        4819     4820       +1     
==========================================
  Hits         4712     4712              
- Misses        107      108       +1
Impacted Files Coverage Δ
src/luacheck/fs.lua 90.42% <50%> (-1.07%) :arrow_down:
src/luacheck/runner.lua 87.73% <80%> (+0.05%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43d4645...54226be. Read the comment docs.

spacewander commented 6 years ago

@mpeterv I have just squashed my commits with the latest change and rebased it to include the Windows tests fix.

mpeterv commented 6 years ago

@spacewander I think there is a minor issue, I've commented on the diff. I'd merge and fix it myself but a bit busy now unfortunately.

spacewander commented 6 years ago

@mpeterv Good catch! I just submitted a new commit to fix the handling of initial scan call.

mpeterv commented 6 years ago

LGTM, thanks!