multiple questions about golangci-lint

[ldez]

#1861 (comment)

[ldez]

@thebeline could you provide the information to fill the issue?

• Config file
• Verbose output of running
• Go environment
• example or a public repository

[ldez]

@thebeline I think that your problem is related to something else:

• are you using go1.16?
• are you using go modules?

[ldez]

I think it's about https://github.com/thebeline/OctoScreen/tree/IMP/GolangLinter or https://github.com/thebeline/OctoScreen/tree/MESS/GolangLinter

@thebeline could you confirm?

[thebeline]

@ldez - Certainly. I need to draft up an MVP for the issue. But below I will try to describe the steps as I can remember.

• Hardware: Raspberry Pi 3 B
• OS: OctoPi (Debian BUSTER)
• Go Package Checkout
  • Clone https://github.com/thebeline/OctoScreen.git into /home/pi/OctoPrint
  • Checkout IMP/GolangLinter
• Golang Setup
  • Raspbian (or, at least OctoPi) ships with an old Go (1.11, I believe)
  • We want Golang 1.15.10, as 1.16.0 behaves oddly with the version of gotk3 we are using.
  • Remove Golang if installed.
  • Install Golang 1.15.10
    • $ wget https://golang.org/dl/go1.15.10.linux-armv6l.tar.gz
    • $ sudo tar -C /usr/local -xzf go1.15.10.linux-armv6l.tar.gz
  • Install GolangCI-Lint
    • curl -sSfL https://github.com/raw/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.39.0
  • Install the build dependencies (not all are needed, but this was in my history, I think only GTK and Cairo are needed)
    • sudo apt-get update && sudo apt-get install -y --no-install-recommends build-essential git-buildpackage fakeroot libcairo2-dev libgtk-3-dev git devscripts
  • Preconfigure some build flags (GTK 2.24 is bugged as of January, flag for 2.22)
    • $ go env -w GOFLAGS=-tags=gtk_3_22,pango_1_42,gdk_pixbuf_2_38,glib_2_58
• Run golangci-lint
  • $ /home/pi/go/bin/golangci-lint run --new=false --issues-exit-code 0 -v --modules-download-mode vendor

There are probably some red flags in there, but note that it does run in GitHubActions.

Verbose Output HEAD (errors as described follow):

/home/pi/go/bin/golangci-lint run --new=false --new-from-rev= --issues-exit-code 0 -vvv --modules-download-mode vendor
INFO [config_reader] Config search paths: [./ /home/pi/OctoScreen /home/pi /home /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 9 linters: [dogsled errorlint gofumpt makezero nolintlint unparam unused whitespace wsl]
INFO [loader] Go packages loading at mode 575 (imports|types_sizes|compiled_files|deps|files|exports_file|name) took 1.942055235s
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 56.589407ms
INFO [linters context/goanalysis] analyzers took 4.629646ms with top 10 stages: errorlint: 2.578153ms, buildir: 364.737µs, whitespace: 327.289µs, makezero: 219.216µs, gofumpt: 205.726µs, the_only_name: 185.988µs, dogsled: 156.249µs, nolintlint: 143.54µs, buildssa: 136.404µs, isgenerated: 95.729µs
INFO [runner] Processors filtering stat (out/in): max_per_file_from_linter: 2220/2220, path_prefixer: 2220/2220, nolint: 2220/2220, path_prettifier: 2220/2220, exclude: 2220/2220, exclude-rules: 2220/2220, diff: 2220/2220, max_same_issues: 2220/2220, source_code: 2220/2220, path_shortener: 2220/2220, filename_unadjuster: 2220/2220, severity-rules: 2220/2220, identifier_marker: 2220/2220, uniq_by_line: 2220/2220, max_from_linter: 2220/2220, skip_files: 2220/2220, skip_dirs: 2220/2220, autogenerated_exclude: 2220/2220, sort_results: 2220/2220, cgo: 2220/2220
INFO [runner] processing took 580.551429ms with stages: identifier_marker: 275.213929ms, exclude-rules: 224.839465ms, nolint: 45.997165ms, source_code: 11.651398ms, path_prettifier: 9.721106ms, sort_results: 5.710415ms, path_shortener: 2.261071ms, filename_unadjuster: 1.245666ms, cgo: 1.150563ms, severity-rules: 1.018845ms, max_per_file_from_linter: 932.7µs, autogenerated_exclude: 788.691µs, max_same_issues: 5.625µs, uniq_by_line: 2.448µs, exclude: 2.448µs, skip_files: 2.135µs, path_prefixer: 2.083µs, skip_dirs: 2.083µs, max_from_linter: 1.876µs, diff: 1.717µs
INFO [runner] linters took 11.045904951s with stages: goanalysis_metalinter: 10.464684572s

After alll of this, I think I should clarify: Locally, I know this error is caused because gccgo is not installed.

That is my mistake.

What I am actually raising as an issue:

• Like I said in my previous comment, I think converting this condition to a "linter error" hides underlying issues (that I would have seen much more clearly in 1.38)
• This triggers LINTER_COUNT+1 times.
  • If it is known that this error state will prevent a linter from continuing, maybe don't continue (although I know that is probably difficult to predict)
  • "Unique by line and character" seems like it would be helpful, just in general.
• By and large, it would seem that if golangci-lint were able to detect catastrophic environment issues (such as cgo preprocessing failed, or failing to load basic go standard libraries), there have to be more helpful debug messages.

MESS/GolangLinter can be ignored for now, but yes, IMP/GolangLinter was where I was working.

[ldez]

Like I said in my previous comment, I think converting this condition to a "linter error" hides underlying issues (that I would have seen much more clearly in 1.38)

It doesn't hide underlying issue: if a real critical error occurs, the output will be the same as in golangci-lint v1.38

If it is known that this error state will prevent a linter from continuing, maybe don't continue (although I know that is probably difficult to predict)

#1875 (comment)

"Unique by line and character" seems like it would be helpful, just in general.

In general, it's useless because errors on a line are mainly around the same topic.

By and large, it would seem that if golangci-lint were able to detect catastrophic environment issues (such as cgo preprocessing failed, or failing to load basic go standard libraries), there have to be more helpful debug messages.

golangci-lint is a linter, not a compiler, and if a real critical error occurs, the output will be the same as in golangci-lint v1.38

The errors that you currently obtain are not "catastrophic environment issues", there is a lot of minor reasons to get them.

[thebeline]

With regards to the IMP/GolangLinter branch, please note that the majority of those commits were just seeing how things behaved.

One thing that I was looking at was related to: https://github.com/golangci/golangci-lint-action/issues/192

Which, as I go down the rabbit hole, I now realize has nothing to do with the action, but moreso with linter it's self.

Actually, I have never run the linter locally, which is why I hadn't seen this issue until now. I only just started trying to run it locally because I was going to try poking things to figure out how the linter worked, and why exactly it was behaving as I had mentioned in the Action Issue I opened.

[ldez]

Your problem in https://github.com/golangci/golangci-lint-action#192 is related to #1490, it's not related to the github action

[thebeline]

Ah, it would seem, more specifically: #1490 (comment)

A function like mergeLineIssues seems like it might have something to do with things I mentioned. :-)

Still, it seems odd that if golangci-lint, under the hood, knows that the env is broken, why does it keep going?

[ldez]

golangci-lint doesn't know if the env is broken, golangci-lint just run Go analysis and run the linters.
It just displays the report from Go analysis: each linter has differents requirements and the fact to compile is not required by all the linters.

[ldez]

I converted the issue to a discussion because you problem is a bit blurry

[ldez]

There are several topics:

• upgrade to go1.11 to go1.15
• upgrade to Go modules
• a bug with multiple fixes
• and maybe other stuff

---

The Go team only maintains the 2 last minor versions: go1.15 and go1.16
Then I will not try to handle a go1.11 environment.

---

To migrate to Go modules:

$ git checkout master 
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
$ go mod init github.com/Z-Bolt/OctoScreen
go: creating new go.mod: module github.com/Z-Bolt/OctoScreen
go: copying requirements from Gopkg.lock
go: to add module requirements and sums:
	go mod tidy
$ go mod tidy
go: downloading github.com/mcuadros/go-octoprint v0.0.0-20180104080025-01362f94e21f
go: downloading github.com/gotk3/gotk3 v0.0.0-20171227192530-8364b1c62f2b
go: downloading github.com/sirupsen/logrus v1.0.4
go: downloading github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4
go: downloading github.com/shirou/gopsutil v2.17.12+incompatible
go: downloading golang.org/x/crypto v0.0.0-20171231215028-0fcca4842a8d
go: downloading golang.org/x/sys v0.0.0-20180104162908-a3f2cbd54cf5
go: downloading github.com/StackExchange/wmi v0.0.0-20170410192909-ea383cf3ba6e
go: downloading github.com/go-ole/go-ole v1.2.0
go: finding module for package gopkg.in/check.v1
go: finding module for package pifke.org/wpasupplicant
go: finding module for package github.com/coreos/go-systemd/daemon
go: finding module for package gopkg.in/gemnasium/logrus-airbrake-hook.v2
go: finding module for package github.com/golang-collections/collections/stack
go: finding module for package github.com/stretchr/testify/assert
go: downloading gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2
go: found github.com/coreos/go-systemd/daemon in github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf
go: found github.com/golang-collections/collections/stack in github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3
go: found pifke.org/wpasupplicant in pifke.org/wpasupplicant v0.0.0-20200816231324-12bdf536389f
go: found github.com/stretchr/testify/assert in github.com/stretchr/testify v1.7.0
go: found gopkg.in/gemnasium/logrus-airbrake-hook.v2 in gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2
go: found gopkg.in/check.v1 in gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
go: finding module for package gopkg.in/airbrake/gobrake.v2
go: downloading gopkg.in/airbrake/gobrake.v2 v2.0.9
go: found gopkg.in/airbrake/gobrake.v2 in gopkg.in/airbrake/gobrake.v2 v2.0.9
$ rm -rf vendor/
$ rm Gopkg.*

[ldez]

I saw that you have edited the vendor folder, this is a real issue, you have to create forks instead 😉
Because it's clearly a bad idea to apply a linter on dependencies and it will impossible to manage with go modules.

[ldez]

Also, the configuration file on the page https://golangci-lint.run/usage/configuration/#config-file is not an example: it's just a sort of sample of all available options.
Then I recommend not using it.

[ldez]

@thebeline I feel there are too many problems not related to golangci-lint, I don't know how to help you efficiently.

[ldez]

FYI I'm able to run golangci-lint on OctoScreen (a local edited version) without any issue, it's slow (~3m) but it works.

[thebeline]

I am out of the office, likely until tomorrow afternoon. But a few things of note:

• I understand go support. I am saying that Raspbian ships with 11, we build inside docker against 15.
• My notes showed that I installed 15.
• I know that the cause for my error is that I borked my environment.
• My issue is not that I can't get it to run, it is more the helpfulness and the duplication of reporting.

As for helpfulness: I get that the CI linter it's self doesnt check the environment, but it does get a sort of stack trace, and i would think "Can't import C" or "gccg" errors would be a fairly good indicator that everything is about to go belly up. That said, I can understand that detecting that is not the CI Linters responsibility.

With regards to duplication: I started running some tests, but things got weirder and weirder. I really need to make you a chart to show you what I mean.

I know that the original topic started because i was getting typecheck errors locally, but i have a suspicion the duplication issue i am talking about extends beyond that on my local environment. Once I isolate linters that cause this issue locally, i will test with the GitHub Action to see if my theory is correct.

[thebeline]

Short version of my findings:

• Linters that don't error dont error

• there are some linters that error their own errors

• there are some linters that (in the original topic) trigger 'typecheck'

• Linters that trigger non-'typecheck' errors work fine together (trigger their own errors separately, correct places, no dupes.

• Of linters that trigger 'typecheck':

  • If only linter, no dupes
  • It not only linter:
    • If there are other included linters that would otherwise throw their own error, those errors do not show up.
    • Some trigger dupes in the form of 1+{number of included linters that would also error}
    • Some trigger dupes in the form of 1+{total number of active linters}

I did have a short list of linters I was testing by hand (you may have it in your inbox), but when i realized I needed to do more research, i deleted the comment.

If I recall: unparam caused 1+{erroring linters} and unused caused 1+{number of linters}, but I have to run more tests, as well as fix my environment and see if my other thought is correct.

[thebeline]

OOFF, while looking, I found something that may be related to the typecheck error appearing more with unused:

golangci-lint/pkg/lint/runner.go

Line 120 in fce3949

// Currently DoesChangeTypes is true only for `unused`.

Honestly I have no idea, but that's a thing I found.

[thebeline]

Yeah let's just forget about the typecheck thing entirely. I am completely lost.

I started by using unparam, which triggered typecheck, and proceeded to try to issolate what else might trigger duplicates of # fo linters + 1 (what unused does).

• I though it had to do with considerSlow, but revive doesn't behave the same way (making more duplicates, it only produces duplicates of #+1).
• Then I thought it had to do with alternativeNames, so I tried govet, but that made SEVEN duplicates.
• So I thought, "well, that has alternative names, AND multiple pre-sets, try something with just presets real quick), so I tried bodyclose, which produces NO duplicates, just the single typecheck errors that unparam throws, not even duplicates of # (THAT WAS A FIRST).
• Well, maybe something with JUST an alternative name, so I tried gas which just produced 2 duplicates in the form of just # of linters...

Ok, so, I'll just stick with my position of forgetting about the typecheck thing entirely (even though it does seem like a bug that it also breaks linters that otherwise would work).

So, back to focusing on the behaviour of uniq-by-line, that feels like a solid bug to me...

[ldez]

uniq-by-line reports only one issue by line (not depend on linters): if several linters have an issue on the same line, you will see only one report for only one linter, it's expected.

[thebeline]

Ok, if it is expected then I suppose that is not a bug. I have dealt with other github actions in the past that behave that same way, so I get it. In my experience it does make it a bit of a pain wrt pull requests, however. As, if the contributor doesn't have the linter locally, and the linter (or linters) are blocking on style issues, it means they may have to push multiple times before they resolve all of the issues on a line (if there are multiple).

That is why I removed that flag.

Fair fair. I suppose the unique part of unique by line caught me off guard. Semantically that should probably read single by line or something, as unique indicates it would just make sure that only unique errors are reported per line. Oh well...

So, now that I understand ALL THE THINGS I suppose, do you have any advisement on:

Before I go and make a fool out of myself again, could you tell me if there is a way to make the linter not exit with 1 if only warning issues are triggered?
I am thinking I want to set some up as warnings that are more suggestions, and some as errors that prevent the pull request.
I was going to do it by running the linter twice with only certain linters enabled, but it seems that if you have linters enabled in the config, you can't limit it to just a few from the command line, unless I am being a dunce again... 😅😭

[thebeline]

Only adding this because I forgot to in the last comment:

FWIW, even your comment:

If you change the default value of uniq-by-line to false, you will have some duplicates.

Seems to indicate that the uniq-by-line directive is for trimming "duplicates", which would imply that non-duplicates would be preserved.

As does the Example Configuration File comment:

# make issues output unique by line, default is true

But again, I can't say I haven't seen this behaviour before, so I guess... shrugs

[thebeline]

On a different topic, any idea what would cause:

level=error msg="Running error: goanalysis_metalinter: gci: package \"uiWidgets\" (isInitialPkg: true, needAnalyzeSource: true): runtime error: slice bounds out of range [28:27]"

[thebeline]

(did I mention this is the first time I have ever worked with a compiled language?? 😢 )

I did some looking, and I am guessing it is an actual slice issue in the code proper, but it compiles just fine? Oh well, must be a piece of code that doesn't get touched I would guess.

That said: golangci-lint doesn't seem to report the file and line number, which is going to make it difficult to find... 😞 Not your problem, but would you have a suggestion on how to find it? ❔

[thebeline]

Scratch that, seems like an issue with golang1.15, I figured out a workaround: We still need to modify the golang ENV prior to lint. SO I just call golangci-lint twice, once just with --version to install golang1.16, configure the go env, and then call it again to actually run the linters.

Seems to work. 👍

Boy oh boy the gci linter takes a bit longer... 😆

I do still have to stay at golang1.15.x for the time being, however, as there is an issue in the shipped version of our gotk3 module (yes, I know we are shipping Go Modules incorrectly, we will resolve that soon enough), so it is 1.15.x until we get that sorted.