golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.58k stars 1.39k forks source link

Linter is confused by Ragel files and the subsequent autogenerated code #298

Closed Olivia5k closed 5 years ago

Olivia5k commented 5 years ago

At my company we use Ragel to define a state machine and parse structured data sets.

The interesting parts of the logs are the warnings that are printed just before the results start popping up;

level=warning msg="[loader/astcache] Can't parse AST of $GO_REPO/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)"
level=warning msg="[runner] Can't process result by autogenerated_exclude processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to `p`\", Pos:token.Position{Filename:\"company/scan/machines/kv.rl\", Offset:117535, Line:80, Column:0}, LineRange:(*result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file company/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)"
level=warning msg="[runner] Can't process result by nolint processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to `p`\", Pos:token.Position{Filename:\"company/scan/machines/kv.rl\", Offset:117535, Line:80, Column:0}, LineRange:(*result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file company/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)"
level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory"
level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory"
[...]

Several errors happen here - possibly all related to the same issue:

I tried adding skip-files, but it doesn't seem to listen about ".*\\.rl$" for some reason. Instead, an even weirder error about symbolink links to the Ragel files pop up (there are no symbolic links in our repo, so that one is Twilight Zone levels of weird to me).

Also, since the config file suggests to let you know if the autogeneration detection doesn't work, consider this a notice that _gen.go does not seem to be recognized. :slightly_smiling_face:

Anyways, it seems to me that all of these issues are related to the linters thinking they should work on the Ragel files and their subsequent autogenerated code. I would suggest that this is wrong and that they should be skipped.


  1. Version of golangci-lint: golangci-lint --version (or git commit if you don't use binary distribution) cb5d1da98628504e795c2599f8994462c19683c3 (one commit after 1.12.2)

  2. Config file: cat .golangci.yml

    
    # This file contains all available configuration options
    # with their default values.

options for analysis running

run:

default concurrency is a available CPU number

concurrency: 4

timeout for analysis, e.g. 30s, 5m, default is 1m

NOTE(thiderman): The CI doesn't have time to complete the

compilation in just one minute, so setting this to 10 instead.

deadline: 10m

exit code when at least one issue was found, default is 1

issues-exit-code: 1

include test files or not, default is true

tests: true

list of build tags, all linters use it. Default is empty list.

build-tags:

output configuration options

output:

colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"

format: colored-line-number

print lines of code with issue, default is true

print-issued-lines: true

print linter name in the end of issue text, default is true

print-linter-name: true

all available settings of specific linters

linters-settings: errcheck:

report about not checking of errors in type assetions: a := b.(MyStruct);

# default is false: such cases aren't reported by default.
check-type-assertions: false

# report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
# default is false: such cases aren't reported by default.
check-blank: false

govet:

report about shadowed variables

check-shadowing: true

golint:

minimal confidence for issues, default is 0.8

min-confidence: 0.8

gofmt:

simplify code: gofmt with -s option, true by default

simplify: true

goimports:

put imports beginning with prefix after 3rd-party packages;

# it's a comma-separated list of prefixes
local-prefixes: github.com/org/project

gocyclo:

minimal code complexity to report, 30 by default (but we recommend 10-20)

min-complexity: 10

maligned:

print struct with more effective memory layout or not, false by default

suggest-new: true

dupl:

tokens count to trigger issue, 150 by default

threshold: 100

goconst:

minimal length of string constant, 3 by default

min-len: 3
# minimal occurrences count to trigger, 3 by default
min-occurrences: 3

depguard: list-type: blacklist include-go-root: false packages:

linters: enable:


3. Go environment: `go version && go env`

go version go1.11.2 linux/amd64 GOARCH="amd64" GOBIN="" GOCACHE="/home/thiderman/.cache/go-build" GOEXE="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="linux" GOOS="linux" GOPATH="/home/thiderman" GOPROXY="" GORACE="" GOROOT="/usr/lib/go" GOTMPDIR="" GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64" GCCGO="gccgo" CC="gcc" CXX="g++" CGO_ENABLED="1" GOMOD="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build757637135=/tmp/go-build -gno-record-gcc-switches"


4. Verbose output of running: `golangci-lint run -v`

golangci-lint -v run --config build/lint/golangci.yml $GO_REPO/... level=info msg="[config_reader] Used config file build/lint/golangci.yml" level=info msg="Gocritic enabled checks: [rangeValCopy]" level=info msg="[lintersdb] Active 8 linters: [deadcode errcheck govet ineffassign megacheck structcheck typecheck varcheck]" level=info msg="[loader] Go packages loading at mode load deps types and syntax took 5.258998345s" level=info msg="[loader] SSA repr building timing: packages building 34.781778ms, total 4.034596775s" level=info msg="[loader] SSA for megacheck repr building timing: packages building 35.111017ms, total 3.815851712s" level=info msg="[runner] worker.3 took 496.813832ms with stages: varcheck: 232.378871ms, errcheck: 107.600384ms, deadcode: 89.396959ms, structcheck: 67.351445ms, typecheck: 5.682µs" level=info msg="[runner] worker.4 took 526.453441ms with stages: govet: 526.42779ms" level=warning msg="[runner] Can't run linter govet: can't eval symlinks for path $GO_REPO/ingestion/syslog/syslog/syslog_parser.rl: lstat $GO_REPO/ingestion/syslog/syslog: no such file or directory" level=info msg="[runner] worker.2 took 719.760613ms with stages: ineffassign: 719.737115ms" [... AST cache removed ...] level=warning msg="[loader/astcache] Can't parse AST of $GO_REPO/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)" level=warning msg="[runner] Can't process result by autogenerated_exclude processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to p\", Pos:token.Position{Filename:\"company/scan/machines/kv.rl\", Offset:117535, Line:80, Column:0}, LineRange:(result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file company/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)" level=warning msg="[runner] Can't process result by nolint processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to p\", Pos:token.Position{Filename:\"company/scan/machines/kv.rl\", Offset:117535, Line:80, Column:0}, LineRange:(result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file company/scan/machines/kv.rl: $GO_REPO/scan/machines/kv.rl:1:1: expected 'package', found '%' (and 11 more errors)" level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory" level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory" company/scan/fieldscanner.rl:113: ineffectual assignment to cs (ineffassign) any; company/scan/fieldscanner_gen.go:2403: ineffectual assignment to cs (ineffassign) cs = 0 company/scan/fieldscanner.rl:37: ineffectual assignment to cs (ineffassign) action start_object { fcall object_inner; } company/scan/machines/kv.rl:80: ineffectual assignment to p (ineffassign) kv_sep any => { $GO_REPO/scan/NONE:3: ineffectual assignment to p (ineffassign) $GO_REPO/scan/NONE:43: ineffectual assignment to p (ineffassign) company/scan/tokenscanner.rl:96: ineffectual assignment to typ (ineffassign) typ := TokenTypeLiteral company/scan/tokenscanner_gen.go:664: ineffectual assignment to _widec (ineffassign) _widec = int16(data[p]) company/scan/tokenscanner_gen.go:709: ineffectual assignment to _widec (ineffassign) _widec = int16(data[p]) level=info msg="[runner] worker.1 took 9.990441773s with stages: megacheck: 9.990423498s" level=info msg="[runner] Workers idle times: #2: 9.260875343s, #3: 9.478491445s, #4: 9.458929448s" [... AST parse removed ...] level=info msg="[runner/skip dirs] Skipped dirs: [migrations/3_popo/anomaly migrations/3_popo/situation migrations/2_blixem/after/sequence/api migrations/2_blixem/after/relog migrations/2_blixem/after/sequence/pb migrations/3_popo/framework ingestion/syslog/syslog migrations/1_highlander/migration migrations/3_popo migrations/2_blixem/after/events/pb]" level=info msg="[runner/max_same_issues] 3/6 issues with text \"ineffectual assignment to cs\" were hidden, use --max-same-issues" level=info msg="[runner/max_same_issues] 3/6 issues with text \"ineffectual assignment to p\" were hidden, use --max-same-issues" level=info msg="[runner/max_same_issues] 1/4 issues with text \"this value of p is never used\" were hidden, use --max-same-issues" company/scan/structurescanner_gen.go:7773:1: this value of p is never used (megacheck) p = (te) - 1 ^ company/scan/structurescanner_gen.go:7787:3: this value of p is never used (megacheck) {p = (te) - 1 ^ company/scan/structurescanner_gen.go:7827:3: this value of p is never used (megacheck) {p = (te) - 1 ^ company/ingestion/syslog/parser_gen.go:21:7: const syslog_first_final is unused (megacheck) const syslog_first_final int = 42 ^ company/ingestion/syslog/parser_gen.go:22:7: const syslog_error is unused (megacheck) const syslog_error int = 0 ^ company/ingestion/syslog/parser_gen.go:24:7: const syslog_en_main is unused (megacheck) const syslog_en_main int = 1 ^ level=info msg="Memory: 145 samples, avg is 2149.8MB, max is 3780.7MB" level=info msg="Execution took 23.17017087s" make: *** [Makefile:78: lint] Error 1



I've cleaned up these a bit by removing the _huge_ AST file lists. If they are needed they can be provided.
Olivia5k commented 5 years ago

Doing some cursory investigation, and it seems that the autogeneration detection scans for certain comments that the Ragel generator does not provide. Might be that the problem is with us then. But, it feels weird that the linters try to parse the .rl files still.

Olivia5k commented 5 years ago
level=warning msg="[runner] Can't run linter govet: can't eval symlinks for path $GO_REPO/ingestion/syslog/syslog/syslog_parser.rl: lstat $GO_REPO/ingestion/syslog/syslog: no such file or directory"
level=warning msg="[loader/astcache] Can't parse AST of $GO_REPO/scan/NONE: open $GO_REPO/scan/NONE: no such file or directory"
level=warning msg="[runner] Can't process result by autogenerated_exclude processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to `p`\", Pos:token.Position{Filename:\"$GO_REPO/scan/NONE\", Offset:117822, Line:3, Column:0}, LineRange:(*result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file $GO_REPO/scan/NONE: open $GO_REPO/scan/NONE: no such file or directory"
level=warning msg="[runner] Can't process result by nolint processor: can't filter issue result.Issue{FromLinter:\"ineffassign\", Text:\"ineffectual assignment to `p`\", Pos:token.Position{Filename:\"$GO_REPO/scan/NONE\", Offset:117822, Line:3, Column:0}, LineRange:(*result.Range)(nil), HunkPos:0, SourceLines:[]string(nil)}: can't parse file $GO_REPO/scan/NONE: open $GO_REPO/scan/NONE: no such file or directory"
level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory"
level=warning msg="[runner/source_code] Failed to get lines for file $GO_REPO/scan/NONE: can't read file $GO_REPO/scan/NONE for printing issued line: open $GO_REPO/scan/NONE: no such file or directory"
$GO_REPO/scan/NONE:3: ineffectual assignment to `p` (ineffassign)
$GO_REPO/scan/NONE:43: ineffectual assignment to `p` (ineffassign)

I added // autogenerated file comments to all the .rl and _gen.go code, just to see if it worked. Get the above as output. Not sure how to proceed with debugging. Happy to test things if you have suggestions!

Notable; syslog/syslog does not exist (only the single syslog), and there are no symlinks at all in the repository.

Olivia5k commented 5 years ago

Upon further investigation, it seems that the error boils down to ineffassign only. Disabling it made the problem go away.

Perhaps this should be an upstream bug to them?

jirfag commented 5 years ago

hi, thank you for such an interesting issue and it's investigation!

the reason is that ragel-generated files contain directives like //line filename.rl:1: they are standard and tell the parser that go file's name isn't filename.go but filename.rl.

I will try to fix it.

jirfag commented 5 years ago

Hi! I've made some fixes for such issues. Please, check it in the latest release

Olivia5k commented 5 years ago

That's awesome! Unfortunately I have left the company in which these problems were present, so I will not be able to verify that it fixed the original case that I reported.

Since I have no ways of checking if the provided fix solves the issue, you can feel free to close it as fixed.

jirfag commented 5 years ago

ok, sorry for delay =(