Add more linters to improve code readability

[42wim]

Add nakedret, unconvert, wastedassign, stylecheck and nolintlint linters to improve code readability

• nakedret - https://github.com/alexkohler/nakedret - nakedret is a Go static analysis tool to find naked returns in functions greater than a specified function length.
• unconvert - https://github.com/mdempsky/unconvert - Remove unnecessary type conversions
• wastedassign - https://github.com/sanposhiho/wastedassign - wastedassign finds wasted assignment statements.
• notlintlint - Reports ill-formed or insufficient nolint directives
• stylecheck - https://staticcheck.io/docs/checks/#ST - keep style consistent
  • excluded: ST1003 - Poorly chosen identifier and ST1005 - Incorrectly formatted error string

[42wim]

I don't think anyone will mind the unconvert, wastedassign and nolintlint linter ?
Anyone any remarks about the nakedret linter ? so that I won't waste time fixing them if it's not wanted :)

[zeripath]

Would it be possible to link to these linters so we can see what they are?

Tbh we need to make linting faster and less CPU/memory intensive so adding more right now just seems kinda counter to that.

[delvh]

@zeripath I disagree. I don't think the linters cause the performance problem, the tests are its cause. Especially because every database is tested individually.
I think adding these linters can have close to the effect achieved by the tests (in terms of quality, not latency).

[42wim]

Would it be possible to link to these linters so we can see what they are?

Tbh we need to make linting faster and less CPU/memory intensive so adding more right now just seems kinda counter to that.

I compared this build to the other drone builds.. doesn't seem to make any noticeable difference.

[42wim]

Nakedrets fixed and stylecheck linter added.

[zeripath]

Would it be possible to link to these linters so we can see what they are?
Tbh we need to make linting faster and less CPU/memory intensive so adding more right now just seems kinda counter to that.

I compared this build to the other drone builds.. doesn't seem to make any noticeable difference.

Time on the CI is not the only important thing here.

I want to be able to run lint on my laptop without the fans going insane.

I have an i7-9750H CPU with 6 physical cores @ 2.60GHz with 16Gb of memory - linting shouldn't be killing my PC.

Running (without this PR)

$ /usr/bin/time make lint-backend                                                            [22:57:34]
go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.0 run
WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649. 
Running go vet...
go run github.com/editorconfig-checker/editorconfig-checker/cmd/editorconfig-checker@2.4.0 templates
186.00user 13.61system 0:28.28elapsed 705%CPU (0avgtext+0avgdata 1175436maxresident)k
574304inputs+881064outputs (982major+4824595minor)pagefaults 0swaps

This is ridiculous.

705% CPU for linting and we're adding more?!

[42wim]

Would it be possible to link to these linters so we can see what they are?
Tbh we need to make linting faster and less CPU/memory intensive so adding more right now just seems kinda counter to that.

I compared this build to the other drone builds.. doesn't seem to make any noticeable difference.

Time on the CI is not the only important thing here.

I want to be able to run lint on my laptop without the fans going insane.

I have an i7-9750H CPU with 6 physical cores @ 2.60GHz with 16Gb of memory linting shouldn't be killing my PC.

Running (without this PR)

$ /usr/bin/time make lint-backend                                                            [22:57:34]
go run github.com/golangci/golangci-lint/cmd/golangci-lint@v1.46.0 run
WARN [linters context] structcheck is disabled because of go1.18. You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649. 
Running go vet...
go run github.com/editorconfig-checker/editorconfig-checker/cmd/editorconfig-checker@2.4.0 templates
186.00user 13.61system 0:28.28elapsed 705%CPU (0avgtext+0avgdata 1175436maxresident)k
574304inputs+881064outputs (982major+4824595minor)pagefaults 0swaps

This is ridiculous.

705% CPU for linting and we're adding more?!

You're also losing a few seconds by using go run instead of the compiled golangci-lint binary.
Also the linter actually needs to actually compile gitea too, so much (the most part) of this CPU usage is the go compiler itself.

I've done some more debugging and found out why I'm getting so much faster linting then you, my go environment has CGO_ENABLED=0 by default and that's making it go faster by reusing the go build cache.

Now normally it should always use the go build cache and (also it's own cache), but it seems that for CGO stuff it's not doing that and it rebuilds sqlite stuff everytime, which is causing the slowdown.

To proof you that it's not the linter causing the 28 seconds on your CPU, try running:
go clean -cache && CGO_ENABLED=0 /usr/bin/time go build && CGO_ENABLED=0 /usr/bin/time golangci-lint run and you'll notice that golangci-lint is not causing the slow-down, it'll probably take something like 7 seconds. (running it again will take something like 3 to 4 seconds as it now uses it's own cache)

(which also means that even when we remove almost all linters from the config and for example only keep gosimple, it will still take you almost as long)

[delvh]

Can confirm, with these instructions, the linters run in 0.15user 0.03system 0:00.08elapsed 234%CPU (0avgtext+0avgdata 27092maxresident)k.
The weird thing is, I get the same results even when omitting the CGO_ENABLED=0...

[delvh]

I'd still like prealloc, but apart from that LGTM

[6543]

🚀