Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Olivia5k opened this issue Nov 19, 2018 · 7 comments
Closed
Labels
area: cgo Related to CGO or line directives

Comments

@Olivia5k
Copy link

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:

  • It tries to parse the .rl files but fails (it's not Go code)
  • It thinks that the file is NONE for some of the errors
  • Visible in the full log below - errors are found in the autogenerated code.

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. 🙂

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)
    cb5d1da (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:
    - mytag

  # which dirs to skip: they won't be analyzed;
  # can use regexp here: generated.*, regexp is applied on full path;
  # default value is empty list, but next dirs are always skipped independently
  # from this option's value:
  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  skip-dirs:
    - ".*migrations.*"
    - ".*ingestion/syslog/.*"

  # which files to skip: they will be analyzed, but issues from them
  # won't be reported. Default value is empty list, but there is
  # no need to include all autogenerated files, we confidently recognize
  # autogenerated files. If it's not please let us know.
  skip-files:
    # - ".*\\.pb\\.go$"
    # - ".*_gen\\.go$"
    # - ".*\\.rl$"


# 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:
      - github.com/davecgh/go-spew/spew
  misspell:
    # Correct spellings using locale preferences for US or UK.
    # Default is to use a neutral variety of English.
    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
    locale: US
  lll:
    # max line length, lines longer will be reported. Default is 120.
    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
    line-length: 120
    # tab width in spaces. Default to 1.
    tab-width: 1
  unused:
    # treat code as a program (not a library) and report unused exported identifiers; default is false.
    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  unparam:
    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
    # and rta for programs with main packages. Default is cha.
    algo: cha

    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  prealloc:
    # XXX: we don't recommend using this linter before doing performance profiling.
    # For most programs usage of prealloc will be a premature optimization.

    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
    # True by default.
    simple: true
    range-loops: true # Report preallocation suggestions on range loops, true by default
    for-loops: false # Report preallocation suggestions on for loops, false by default
  gocritic:
    # which checks should be enabled; can't be combined with 'disabled-checks';
    # default are: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref
    # ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef
    # unlambda unslice rangeValCopy defaultCaseOrder];
    # all checks list: https://github.com/go-critic/checkers
    enabled-checks:
      - rangeValCopy
    # which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
    # disabled-checks:
      # - regexpMust
    settings: # settings passed to gocritic
      # captLocal: # must be valid enabled check name
      #   checkLocals: true
      rangeValCopy:
        sizeThreshold: 32

linters:
  enable:
    - megacheck
    - govet
  enable-all: false
  disable:
    - maligned
    - prealloc
  fast: false

  1. 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"
  1. 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
Copy link
Author

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
Copy link
Author

Olivia5k commented Nov 19, 2018

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
Copy link
Author

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
Copy link
Contributor

jirfag commented Nov 23, 2018

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
Copy link
Contributor

jirfag commented Jun 9, 2019

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

@Olivia5k
Copy link
Author

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
Copy link
Contributor

jirfag commented Jun 10, 2019

ok, sorry for delay =(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: cgo Related to CGO or line directives
Projects
None yet
Development

No branches or pull requests

3 participants