golangci-lint
golangci-lint copied to clipboard
make --fix work (naively) with --prefix-path
Issue
Currently, using --path-prefix
together with --fix
does not work with any other path prefix than "".
This is easily reproduced merely by adding --path-prefix=something
to the command line when also using --fix
(and, of course, actually having something fixable) or adding it to the arguments used in TestFix
, which will cause the test to fail.
This is problematic for my use case, a repository with multiple go modules in it (in separate directories) where I'd like to run golangci-lint
on all of them with pre-commit
before commiting. To do this, I did not figure out other approaches than to run golangci-lint
in each of those directories and using --path-prefix
to add the module directory to the output which does not work.
Reproduction
matias@kiri ~/c/t/fix_sample> ls
main.go
matias@kiri ~/c/t/fix_sample> cat main.go
package main
func main() {
println(
"hello")
}
matias@kiri ~/c/t/fix_sample> golangci-lint version
golangci-lint has version 1.42.0 built from c6142e38 on 2021-08-17T11:47:22Z
matias@kiri ~/c/t/fix_sample> golangci-lint run --fix --path-prefix=foo ./main.go -v --disable-all --enable gofmt
INFO [config_reader] Config search paths: [./ /home/matias/code/temp/fix_sample /home/matias/code/temp /home/matias/code /home/matias /home /]
INFO [lintersdb] Active 1 linters: [gofmt]
INFO [loader] Go packages loading at mode 7 (compiled_files|files|name) took 41.533512ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 122.782µs
INFO [linters context/goanalysis] analyzers took 731.031µs with top 10 stages: gofmt: 731.031µs
INFO [runner] Processors filtering stat (out/in): max_same_issues: 1/1, max_from_linter: 1/1, severity-rules: 1/1, path_prefixer: 1/1, sort_results: 1/1, identifier_marker: 1/1, path_prettifier: 1/1, autogenerated_exclude: 1/1, exclude: 1/1, nolint: 1/1, uniq_by_line: 1/1, max_per_file_from_linter: 1/1, path_shortener: 1/1, filename_unadjuster: 1/1, diff: 1/1, skip_files: 1/1, skip_dirs: 1/1, exclude-rules: 1/1, source_code: 1/1, cgo: 1/1
INFO [runner] processing took 123.9µs with stages: identifier_marker: 22.489µs, exclude-rules: 14.388µs, nolint: 13.41µs, path_prettifier: 12.431µs, autogenerated_exclude: 12.153µs, source_code: 9.429µs, skip_dirs: 5.308µs, cgo: 3.841µs, exclude: 3.352µs, skip_files: 2.934µs, sort_results: 2.864µs, uniq_by_line: 2.793µs, diff: 2.445µs, severity-rules: 2.444µs, path_shortener: 2.375µs, path_prefixer: 2.374µs, max_same_issues: 2.305µs, filename_unadjuster: 2.305µs, max_per_file_from_linter: 2.304µs, max_from_linter: 1.956µs
INFO [runner] linters took 1.433009ms with stages: gofmt: 1.159161ms
ERRO Failed to fix issues in file foo/main.go: failed to get file bytes for foo/main.go: can't read file foo/main.go: open foo/main.go: no such file or directory
INFO fixer took 16.623µs with stages: all: 16.623µs
foo/main.go:3: File is not `gofmt`-ed with `-s` (gofmt)
func main() {
println(
"hello")
INFO File cache stats: 1 entries of total size 53B
INFO Memory: 2 samples, avg is 72.7MB, max is 72.8MB
INFO Execution took 50.472601ms
(even though this test was ran with not quite the latest version, the issue reproduces with tip of master as well)
The error in the output clearly points out the problem;
ERRO Failed to fix issues in file foo/main.go: failed to get file bytes for foo/main.go: can't read file foo/main.go: open foo/main.go: no such file or directory
That is, the path prefix is prepended to the path of the file to be fixed which does not seem to be what is intended, as the help / documentation states that --prefix-path
configures the Path prefix to add to **output**
.
Fix
This PR adds a mock --path-prefix
argument to TestFix
and implements a (very naive) fix for the issue, stripping the configured path prefix from the path passed to Fixer.fixIssuesInFile
if the configured path prefix is not "".
I'm happy to create an alternative fix if some other approach is seen to be better, options off the top of my head would include;
- Adding the actual path to
token.Position
, allowingFixer
to use it instead. - Adding the actual path to
Issue
, allowingIssue.FilePath()
to return it, as this is howFixer
seems to determine the path to use. - Adding the path prefix only to (all of?) the output(s) rather than prefixing
token.Position.Filename
as there could be other needs for the actual path of the file in the future? This seems like the best option to me but the details of how to do this and implications it could have are unclear to me as I'm quite unfamiliar with the codebase, so would need some pointers to implement this.
Hey, thank you for opening your first Pull Request !
Hi, thanks for the review and sorry for the delay!
@SVilgelm This is tested, but it's in the same TestFix
test as all other fix-related testing (as far as I can tell?).
I can break it into a separate test but I'm not sure what the value of that would be, as my uneducated gut feeling is that it would be pretty much the exact same test with less validation (and inputs) but more implied maintenance down the road.
Happy to do either so let me know what you'd prefer.
@SVilgelm @alexandear sorry about the ping, just making sure you've noticed @moitias' comment above -- if you could let us know what you think so we can get this moving forward. Thanks!
Hi @moitias, others, I took the liberty to submit #3626 which builds on this, resolves conflicts, and addresses remaining open comments here.