More strict linter

[tzdybal]

We need stronger linting.

@adlerjohn suggested:

• catching warnings as errors
• more checks

Ref: #223

[adlerjohn]

Should also enable the staticcheck linter if it's not enabled currently.

[liamsi]

Where is the current config?

[adlerjohn]

https://github.com/celestiaorg/optimint/blob/37de59b12db983c0bf2f2d2e6103889c9c85e4a1/.golangci.yml#L1-L9

[liamsi]

Ah, OK. https://github.com/celestiaorg/optimint/blob/37de59b12db983c0bf2f2d2e6103889c9c85e4a1/.golangci.yml#L7-L9

More important than warnings as errors is to use the default set of linters here. And maybe disable only some

[liamsi]

Could use this as a solid basis IMO: https://github.com/tendermint/tendermint/blob/05d8a0bd05bcdbaefc387244d19a74b31c206240/.golangci.yml#L1-L59

[liamsi]

Don't have time to go through each linter individually but a good heuristic is: take the above linked config as a basis. Fix all issues that this catches or explain why some of them should be disabled.

[tzdybal]

Ah, OK.

https://github.com/celestiaorg/optimint/blob/37de59b12db983c0bf2f2d2e6103889c9c85e4a1/.golangci.yml#L7-L9

More important than warnings as errors is to use the default set of linters here. And maybe disable only some

Just for clarification: this config adds some linters, not enables only 3 of them.

[tzdybal]

Enabled by your configuration linters:
deadcode: Finds unused code [fast: false, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: false, auto-fix: false]
gofmt: Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification [fast: true, auto-fix: true]
goimports: In addition to fixing imports, goimports also formats your code in the same style as gofmt. [fast: true, auto-fix: true]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: false, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: false, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false]
structcheck: Finds unused struct fields [fast: false, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: false, auto-fix: false]

[adlerjohn]

staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: false, auto-fix: false]

Running staticcheck locally shows warnings however, fixed only in #262

[tzdybal]

Running staticcheck locally shows warnings however,

Yes. This is strange for me.

[tzdybal]

golangci-lint run --exclude-use-default=false solves this. Apparently SA4011 is excluded by default (among others).