Skip to content
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

Reduce golint min_confidence to default 0.8 #191

Closed
jayvdb opened this issue May 6, 2021 · 7 comments · Fixed by #329
Closed

Reduce golint min_confidence to default 0.8 #191

jayvdb opened this issue May 6, 2021 · 7 comments · Fixed by #329

Comments

@jayvdb
Copy link
Member

jayvdb commented May 6, 2021

The current golint level of 1.0 causes several test cases to become errors, mostly due to capitalisation and docstrings.

However the default of 0.8 causes many more errors.

@adsharma
Copy link
Collaborator

Did you look at gometalint? It looks unlikely that we'll have a comment on every global variable in tests.

golang/lint#186

After the upcoming diff, the only lint errors at confidence=1.0 are missing comments.

@adsharma
Copy link
Collaborator

Or staticcheck. golint is deprecated and frozen.

golang/go#38968

@jayvdb
Copy link
Member Author

jayvdb commented May 11, 2021

Used gometalint a bit when I was doing go; it used to manage their own fork of go tools which were no longer supported by the go team. I dont know staticcheck - from a quick scan of https://staticcheck.io/ it looks like a decent general purpose tool, which is suffice to replace golint.

adsharma added a commit that referenced this issue May 11, 2021
This affects symbol visibility.

Fixes: #174
Related to: #191
@jayvdb
Copy link
Member Author

jayvdb commented May 12, 2021

gometalinter is deprecated, and they recommend https://github.com/golangci/golangci-lint , which is in brew https://formulae.brew.sh/formula/golangci-lint, so is staticcheck. Neither are in chocolatey.

@jayvdb
Copy link
Member Author

jayvdb commented May 12, 2021

staticcheck complains about assert.go

assert not (0 == 1)

becomes

if !(!(0 == 1)) {
    panic("assert")
}

and staticcheck error:

cases/assert.go:11:5: negating a boolean twice has no effect; is this a typo? (SA4013)

@jayvdb
Copy link
Member Author

jayvdb commented May 12, 2021

On macos, the default linters for golangci-lint are

Enabled by default 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]
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]
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]

In addition to the error from staticcheck about assert.go, the other errors from golangci-lint are all from deadcode which is quite rightly pointing out that our test cases have dead code, c.f. #123

coverage.go:

cases/coverage.go:17:6: `InlineEllipsis` is unused (deadcode)
func InlineEllipsis() {
     ^
cases/coverage.go:31:6: `InferBool` is unused (deadcode)
func InferBool(code int) bool {
     ^

global2.go:

cases/global2.go:9:5: `Code1` is unused (deadcode)
var Code1 int = 1
    ^
cases/global2.go:13:5: `LC` is unused (deadcode)
var LC = map[string]int{CodeB: Code0}
    ^

infer_ops.go:

cases/infer_ops.go:35:6: `Add1` is unused (deadcode)
func Add1(x int8, y int8) int16 {
     ^
cases/infer_ops.go:39:6: `Add2` is unused (deadcode)
func Add2(x int16, y int16) int32 {
     ^
cases/infer_ops.go:43:6: `Add3` is unused (deadcode)
func Add3(x int32, y int32) int64 {
     ^
cases/infer_ops.go:47:6: `Add4` is unused (deadcode)
func Add4(x int64, y int64) int64 {
     ^
cases/infer_ops.go:51:6: `Add5` is unused (deadcode)
func Add5(x uint8, y uint8) uint16 {
     ^
cases/infer_ops.go:55:6: `Add6` is unused (deadcode)
func Add6(x uint16, y uint16) uint32 {
     ^
cases/infer_ops.go:59:6: `Add7` is unused (deadcode)
func Add7(x uint32, y uint32) uint64 {
     ^
cases/infer_ops.go:63:6: `Add8` is unused (deadcode)
func Add8(x uint64, y uint64) uint64 {
     ^
cases/infer_ops.go:67:6: `Add9` is unused (deadcode)
func Add9(x int8, y uint16) uint32 {
     ^
cases/infer_ops.go:71:6: `Sub` is unused (deadcode)
func Sub(x int8, y int8) int8 {
     ^
cases/infer_ops.go:75:6: `Mul` is unused (deadcode)
func Mul(x int8, y int8) int16 {
     ^

adsharma added a commit that referenced this issue May 13, 2021
Also remove unused variables and consolidate similar funcs

Related to: #191
@adsharma adsharma mentioned this issue Jun 3, 2021
@jayvdb
Copy link
Member Author

jayvdb commented Jun 23, 2021

Another tool we could use is https://github.com/SonarSource/slang , part of SonarQube, and usable as a github action with https://github.com/kitabisa/sonarqube-action .

sonarqube is available on brew, and chocolatey has some packages but they might not be suitable, as the official packages are MSBuild/.Net specific, and the other packages look a bit dodgy

jayvdb added a commit to jayvdb/py2many that referenced this issue Jun 23, 2021
jayvdb added a commit to jayvdb/py2many that referenced this issue Jun 24, 2021
jayvdb added a commit to jayvdb/py2many that referenced this issue Jun 24, 2021
MiguelMarcelino pushed a commit to MiguelMarcelino/py2many that referenced this issue May 23, 2022
This affects symbol visibility.

Fixes: py2many#174
Related to: py2many#191
MiguelMarcelino pushed a commit to MiguelMarcelino/py2many that referenced this issue May 23, 2022
Also remove unused variables and consolidate similar funcs

Related to: py2many#191
MiguelMarcelino pushed a commit to MiguelMarcelino/py2many that referenced this issue May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants