Skip to content

Unexpected detection of file as autogenerated skipped all linters #2254

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
4 tasks done
electrofelix opened this issue Sep 24, 2021 · 7 comments · Fixed by #4507
Closed
4 tasks done

Unexpected detection of file as autogenerated skipped all linters #2254

electrofelix opened this issue Sep 24, 2021 · 7 comments · Fixed by #4507
Labels
area: processors bug Something isn't working

Comments

@electrofelix
Copy link

electrofelix commented Sep 24, 2021

Welcome

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).
  • Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

Detection of auto generated files based on the string code generated can be a little wide and ended up picking up a file that had the text before the imports:

// Integrates with code generated from the swagger spec

It appears it is a deliberate choice to be less strict with matching compared to https://pkg.go.dev/cmd/go#hdr-Generate_Go_files_by_processing_source.

The problem is that, the only reason this was spotted was that I noticed some lines that looked like they should have failed linting. Otherwise the changes would have been committed as is with issues that should have been flagged by the linter.

There does not appear to be any way to easily catch this besides linting with debug enabled and checking the files detected as autogenerated against a separate list to ensure no accidental detection's are occurring.

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.42.1 built from 54f4301d on 2021-09-06T17:05:10Z

Configuration file

$ cat .golangci.yml
# paste output here

Go environment

$ go version && go env
go version go1.15.14 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/<username>/.cache/go-build"
GOENV="/home/<username>/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/<username>/go/pkg/mod"
GONOPROXY="<private-org>"
GONOSUMDB="<private-org>"
GOOS="linux"
GOPATH="/home/<username>/go"
GOPRIVATE="<private-org>"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/<username>/local/gosdks/go1.15.14"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/<username>/local/gosdks/go1.15.14/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="<can't share this as it has a private org name in it>"
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/user/<id>/go-build179142829=/tmp/go-build -gno-record-gcc-switches"

I've had to replace some of the details due it having references to employer private org and UID.

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
level=info msg="[config_reader] Config search paths: [./ /home/<username>/tmp/golangci-lint-bug-demo /home/<username>/tmp /home/<username> /home /]"
level=info msg="[lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]"
level=info msg="[loader] Go packages loading at mode 575 (compiled_files|deps|files|name|exports_file|imports|types_sizes) took 115.328814ms"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 338.681µs"
level=info msg="[linters context/goanalysis] analyzers took 1.501985242s with top 10 stages: buildir: 1.061415812s, fact_purity: 97.66441ms, nilness: 79.009514ms, inspect: 61.552866ms, ctrlflow: 51.791239ms, fact_deprecated: 39.544268ms, printf: 36.360172ms, SA5012: 32.919872ms, typedness: 16.887059ms, S1038: 634.132µs"
level=info msg="[runner] Issues before processing: 4, after processing: 0"
level=info msg="[runner] Processors filtering stat (out/in): cgo: 4/4, skip_files: 4/4, filename_unadjuster: 4/4, path_prettifier: 4/4, skip_dirs: 4/4, autogenerated_exclude: 0/4"
level=info msg="[runner] processing took 178.037µs with stages: path_prettifier: 64.418µs, autogenerated_exclude: 60.472µs, skip_dirs: 38.94µs, cgo: 2.485µs, filename_unadjuster: 1.553µs, max_same_issues: 1.458µs, nolint: 1.444µs, uniq_by_line: 802ns, identifier_marker: 745ns, max_from_linter: 725ns, diff: 691ns, exclude-rules: 686ns, source_code: 629ns, skip_files: 498ns, exclude: 429ns, sort_results: 427ns, path_shortener: 424ns, severity-rules: 420ns, max_per_file_from_linter: 401ns, path_prefixer: 390ns"
level=info msg="[runner] linters took 1.93828604s with stages: goanalysis_metalinter: 1.937924977s"
level=info msg="File cache stats: 0 entries of total size 0B"
level=info msg="Memory: 22 samples, avg is 130.1MB, max is 215.4MB"
level=info msg="Execution took 2.066626895s"

Code example or link to a public repository

https://github.com/electrofelix/golangci-lint-bug-demo

@electrofelix electrofelix added the bug Something isn't working label Sep 24, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 24, 2021

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@electrofelix
Copy link
Author

As it stands with a project with a number of autogenerated mock files, turning on debug to catch something that is accidental such as this would be quite cumbersome. Right now it was caught because I seem to spot lint mistakes in code and was left wondering why they weren't caught automatically, but my colleagues didn't see any issue until I mentioned it.

Would it be better to have both a strict set of strings and a lax set of strings and let end users select which to use? Or to have an option that warns on files that only match a lax set unless they or their pattern is listed in the config to not warn?

@butuzov
Copy link
Member

butuzov commented Sep 26, 2021

I checked for another match of the autogenerated file and it doesn't seem to appear in a lot of files (just ~1200 matches atm in whole github).

@golangci/team I would propose to abandon partial matches and switch to only checking "idiomatic" pattern used almost everywhere else ( see Generate_Go_files_by_processing_source?
)

^// Code generated .* DO NOT EDIT\.$

@johejo
Copy link
Contributor

johejo commented Sep 26, 2021

It might be nice to be able to customize the generated code comment pattern with config yaml.

@bombsimon
Copy link
Member

@golangci/team I would propose to abandon partial matches and switch to only checking "idiomatic" pattern used almost everywhere else ( see Generate_Go_files_by_processing_source?
)

^// Code generated .* DO NOT EDIT\.$

I think this sound like the way to go and I don't think there's a big risk introducing this change since the amount of code that uses a non recommended pattern but has been ignored and still contains linting issues must be pretty limited. I don't think having a configuration parameter for this is the way to go since it's very clear to me that the documentation only recommends one exact pattern and that's what I'm used to see and use.

@stale
Copy link

stale bot commented Nov 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Nov 13, 2022
@ldez ldez removed the stale No recent correspondence or work activity label Nov 13, 2022
@ldez
Copy link
Member

ldez commented Mar 21, 2024

Fixed by #4507

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: processors bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants