From f38a024459aaf77673f462ad176dbe766c7217aa Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 9 Mar 2024 03:22:03 +0100 Subject: [PATCH 1/2] build(deps): bump github.com/golangci/unconvert to HEAD --- .golangci.reference.yml | 8 ++ go.mod | 3 +- go.sum | 5 +- pkg/config/linters_settings.go | 6 + pkg/golinters/unconvert.go | 23 ++-- pkg/lint/lintersdb/builder_linter.go | 2 +- test/linters_test.go | 1 + test/testdata/unconvert.go | 183 ++++++++++++++++++++++++++- 8 files changed, 207 insertions(+), 24 deletions(-) diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 7ac8e56e98c8..ed828fe1caae 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -2293,6 +2293,14 @@ linters-settings: # Default: false syslog-priority: true + unconvert: + # Remove conversions that force intermediate rounding. + # Default: false + fast-math: true + # Be more conservative (experimental). + # Default: false + safe: true + unparam: # Inspect exported functions. # diff --git a/go.mod b/go.mod index 132f68d0afd7..9435701c8098 100644 --- a/go.mod +++ b/go.mod @@ -45,7 +45,7 @@ require ( github.com/golangci/gofmt v0.0.0-20231018234816-f50ced29576e github.com/golangci/misspell v0.4.1 github.com/golangci/revgrep v0.5.2 - github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 + github.com/golangci/unconvert v0.0.0-20240309020433-c5143eacb3ed github.com/gordonklaus/ineffassign v0.1.0 github.com/gostaticanalysis/forcetypeassert v0.1.0 github.com/gostaticanalysis/nilerr v0.1.1 @@ -151,7 +151,6 @@ require ( github.com/gostaticanalysis/comment v1.4.2 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/kisielk/gotool v1.0.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/magiconair/properties v1.8.6 // indirect github.com/mattn/go-isatty v0.0.20 // indirect diff --git a/go.sum b/go.sum index 852b05046c45..6ef617f6e8a7 100644 --- a/go.sum +++ b/go.sum @@ -233,8 +233,8 @@ github.com/golangci/misspell v0.4.1 h1:+y73iSicVy2PqyX7kmUefHusENlrP9YwuHZHPLGQj github.com/golangci/misspell v0.4.1/go.mod h1:9mAN1quEo3DlpbaIKKyEvRxK1pwqR9s/Sea1bJCtlNI= github.com/golangci/revgrep v0.5.2 h1:EndcWoRhcnfj2NHQ+28hyuXpLMF+dQmCN+YaeeIl4FU= github.com/golangci/revgrep v0.5.2/go.mod h1:bjAMA+Sh/QUfTDcHzxfyHxr4xKvllVr/0sCv2e7jJHA= -github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 h1:zwtduBRr5SSWhqsYNgcuWO2kFlpdOZbP0+yRjmvPGys= -github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4/go.mod h1:Izgrg8RkN3rCIMLGE9CyYmU9pY2Jer6DgANEnZ/L/cQ= +github.com/golangci/unconvert v0.0.0-20240309020433-c5143eacb3ed h1:IURFTjxeTfNFP0hTEi1YKjB/ub8zkpaOqFFMApi2EAs= +github.com/golangci/unconvert v0.0.0-20240309020433-c5143eacb3ed/go.mod h1:XLXN8bNw4CGRPaqgl3bv/lhz7bsGPh4/xSaMTbo2vkQ= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= @@ -316,7 +316,6 @@ github.com/karamaru-alpha/copyloopvar v1.0.8 h1:gieLARwuByhEMxRwM3GRS/juJqFbLraf github.com/karamaru-alpha/copyloopvar v1.0.8/go.mod h1:u7CIfztblY0jZLOQZgH3oYsJzpC2A7S6u/lfgSXHy0k= github.com/kisielk/errcheck v1.7.0 h1:+SbscKmWJ5mOK/bO1zS60F5I9WwZDWOfRsC4RwfwRV0= github.com/kisielk/errcheck v1.7.0/go.mod h1:1kLL+jV4e+CFfueBmI1dSK2ADDyQnlrnrY/FqKluHJQ= -github.com/kisielk/gotool v1.0.0 h1:AV2c/EiW3KqPNT9ZKl07ehoAGi4C5/01Cfbblndcapg= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/kkHAIKE/contextcheck v1.1.4 h1:B6zAaLhOEEcjvUgIYEqystmnFk1Oemn8bvJhbt0GMb8= github.com/kkHAIKE/contextcheck v1.1.4/go.mod h1:1+i/gWqokIa+dm31mqGLZhZJ7Uh44DJGZVmr6QRBNJg= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 319daca4b6dc..588aa643ccfa 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -268,6 +268,7 @@ type LintersSettings struct { Testifylint TestifylintSettings Testpackage TestpackageSettings Thelper ThelperSettings + Unconvert UnconvertSettings Unparam UnparamSettings Unused UnusedSettings UseStdlibVars UseStdlibVarsSettings @@ -882,6 +883,11 @@ type UseStdlibVarsSettings struct { SyslogPriority bool `mapstructure:"syslog-priority"` } +type UnconvertSettings struct { + FastMath bool `mapstructure:"fast-math"` + Safe bool `mapstructure:"safe"` +} + type UnparamSettings struct { CheckExported bool `mapstructure:"check-exported"` Algo string diff --git a/pkg/golinters/unconvert.go b/pkg/golinters/unconvert.go index 1e44f9fa5834..5f8c73182c57 100644 --- a/pkg/golinters/unconvert.go +++ b/pkg/golinters/unconvert.go @@ -3,9 +3,10 @@ package golinters import ( "sync" - unconvertAPI "github.com/golangci/unconvert" + "github.com/golangci/unconvert" "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" "github.com/golangci/golangci-lint/pkg/lint/linter" "github.com/golangci/golangci-lint/pkg/result" @@ -13,7 +14,8 @@ import ( const unconvertName = "unconvert" -func NewUnconvert() *goanalysis.Linter { +//nolint:dupl // This is not a duplicate of dogsled. +func NewUnconvert(settings *config.UnconvertSettings) *goanalysis.Linter { var mu sync.Mutex var resIssues []goanalysis.Issue @@ -21,7 +23,7 @@ func NewUnconvert() *goanalysis.Linter { Name: unconvertName, Doc: goanalysis.TheOnlyanalyzerDoc, Run: func(pass *analysis.Pass) (any, error) { - issues := runUnconvert(pass) + issues := runUnconvert(pass, settings) if len(issues) == 0 { return nil, nil @@ -45,18 +47,13 @@ func NewUnconvert() *goanalysis.Linter { }).WithLoadMode(goanalysis.LoadModeTypesInfo) } -func runUnconvert(pass *analysis.Pass) []goanalysis.Issue { - prog := goanalysis.MakeFakeLoaderProgram(pass) +func runUnconvert(pass *analysis.Pass, settings *config.UnconvertSettings) []goanalysis.Issue { + positions := unconvert.Run(pass, settings.FastMath, settings.Safe) - positions := unconvertAPI.Run(prog) - if len(positions) == 0 { - return nil - } - - issues := make([]goanalysis.Issue, 0, len(positions)) - for _, pos := range positions { + var issues []goanalysis.Issue + for _, position := range positions { issues = append(issues, goanalysis.NewIssue(&result.Issue{ - Pos: pos, + Pos: position, Text: "unnecessary conversion", FromLinter: unconvertName, }, pass)) diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index b9466a219f0e..60ec24979635 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -633,7 +633,7 @@ func (b LinterBuilder) Build(cfg *config.Config) []*linter.Config { WithPresets(linter.PresetBugs). WithURL(""), - linter.NewConfig(golinters.NewUnconvert()). + linter.NewConfig(golinters.NewUnconvert(&cfg.LintersSettings.Unconvert)). WithSince("v1.0.0"). WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). diff --git a/test/linters_test.go b/test/linters_test.go index ada3dbeb9e45..4ae7a67b8f8e 100644 --- a/test/linters_test.go +++ b/test/linters_test.go @@ -88,6 +88,7 @@ func testOneSource(t *testing.T, log *logutils.StderrLog, binPath, sourcePath st "--disable-all", "--out-format=json", "--max-same-issues=100", + "--max-issues-per-linter=100", } for _, addArg := range []string{"", "-Etypecheck"} { diff --git a/test/testdata/unconvert.go b/test/testdata/unconvert.go index bbd7ad090901..b0c422c5eef8 100644 --- a/test/testdata/unconvert.go +++ b/test/testdata/unconvert.go @@ -1,10 +1,183 @@ //golangcitest:args -Eunconvert package testdata -import "log" +import "io" -func Unconvert() { - a := 1 - b := int(a) // want "unnecessary conversion" - log.Print(b) +// Various explicit conversions of untyped constants +// that cannot be removed. +func _() { + const ( + _ = byte(0) + _ = int((real)(0i)) + _ = complex64(complex(1, 2)) + _ = (bool)(true || false) + + PtrSize = 4 << (^uintptr(0) >> 63) + c0 = uintptr(PtrSize) + c1 = uintptr((8-PtrSize)/4*2860486313 + (PtrSize-4)/4*33054211828000289) + ) + + i := int64(0) + _ = i +} + +// Make sure we distinguish function calls from +// conversion to function type. +func _() { + type F func(F) int + var f F + + _ = F(F(nil)) // want "unnecessary conversion" + _ = f(F(nil)) +} + +// Make sure we don't remove explicit conversions that +// prevent fusing floating-point operation. +func _() { + var f1, f2, f3, ftmp float64 + _ = f1 + float64(f2*f3) + ftmp = float64(f2 * f3) + _ = f1 + ftmp + ftmp = f2 * f3 + _ = f1 + float64(ftmp) + + var c1, c2, c3, ctmp complex128 + _ = c1 + complex128(c2*c3) + ctmp = complex128(c2 * c3) + _ = c1 + ctmp + ctmp = c2 * c3 + _ = c1 + complex128(ctmp) +} + +// Basic contains conversion errors for builtin data types +func Basic() { + var vbool bool + var vbyte byte + var vcomplex128 complex128 + var vcomplex64 complex64 + var verror error + var vfloat32 float32 + var vfloat64 float64 + var vint int + var vint16 int16 + var vint32 int32 + var vint64 int64 + var vint8 int8 + var vrune rune + var vstring string + var vuint uint + var vuint16 uint16 + var vuint32 uint32 + var vuint64 uint64 + var vuint8 uint8 + var vuintptr uintptr + + _ = bool(vbool) // want "unnecessary conversion" + _ = byte(vbyte) // want "unnecessary conversion" + _ = error(verror) // want "unnecessary conversion" + _ = int(vint) // want "unnecessary conversion" + _ = int16(vint16) // want "unnecessary conversion" + _ = int32(vint32) // want "unnecessary conversion" + _ = int64(vint64) // want "unnecessary conversion" + _ = int8(vint8) // want "unnecessary conversion" + _ = rune(vrune) // want "unnecessary conversion" + _ = string(vstring) // want "unnecessary conversion" + _ = uint(vuint) // want "unnecessary conversion" + _ = uint16(vuint16) // want "unnecessary conversion" + _ = uint32(vuint32) // want "unnecessary conversion" + _ = uint64(vuint64) // want "unnecessary conversion" + _ = uint8(vuint8) // want "unnecessary conversion" + _ = uintptr(vuintptr) // want "unnecessary conversion" + + _ = float32(vfloat32) + _ = float64(vfloat64) + _ = complex128(vcomplex128) + _ = complex64(vcomplex64) + + // Pointers + _ = (*bool)(&vbool) // want "unnecessary conversion" + _ = (*byte)(&vbyte) // want "unnecessary conversion" + _ = (*complex128)(&vcomplex128) // want "unnecessary conversion" + _ = (*complex64)(&vcomplex64) // want "unnecessary conversion" + _ = (*error)(&verror) // want "unnecessary conversion" + _ = (*float32)(&vfloat32) // want "unnecessary conversion" + _ = (*float64)(&vfloat64) // want "unnecessary conversion" + _ = (*int)(&vint) // want "unnecessary conversion" + _ = (*int16)(&vint16) // want "unnecessary conversion" + _ = (*int32)(&vint32) // want "unnecessary conversion" + _ = (*int64)(&vint64) // want "unnecessary conversion" + _ = (*int8)(&vint8) // want "unnecessary conversion" + _ = (*rune)(&vrune) // want "unnecessary conversion" + _ = (*string)(&vstring) // want "unnecessary conversion" + _ = (*uint)(&vuint) // want "unnecessary conversion" + _ = (*uint16)(&vuint16) // want "unnecessary conversion" + _ = (*uint32)(&vuint32) // want "unnecessary conversion" + _ = (*uint64)(&vuint64) // want "unnecessary conversion" + _ = (*uint8)(&vuint8) // want "unnecessary conversion" + _ = (*uintptr)(&vuintptr) // want "unnecessary conversion" +} + +// Counter is an int64 +type Counter int64 + +// ID is a typed identifier +type ID string + +// Metric is a struct +type Metric struct { + ID ID + Counter Counter +} + +// Custom contains conversion errors for builtin data types +func Custom() { + type Local struct{ id ID } + + var counter Counter + var id ID + var m Metric + var local Local + var x struct{ id ID } + + _ = Counter(counter) // want "unnecessary conversion" + _ = ID(id) // want "unnecessary conversion" + _ = Metric(m) // want "unnecessary conversion" + _ = Local(local) // want "unnecessary conversion" + _ = (struct{ id ID })(x) // want "unnecessary conversion" + + // Pointers + _ = (*Counter)(&counter) // want "unnecessary conversion" + _ = (*ID)(&id) // want "unnecessary conversion" + _ = (*Metric)(&m) // want "unnecessary conversion" + _ = (*Local)(&local) // want "unnecessary conversion" + _ = (*struct{ id ID })(&x) // want "unnecessary conversion" +} + +// Interfaces contains conversion errors for interfaces +func Interfaces() { + var writer io.Writer + + _ = (io.Writer)(writer) // want "unnecessary conversion" + _ = (*io.Writer)(&writer) // want "unnecessary conversion" +} + +// Constructor is a func type +type Constructor func() ID + +// Funcs contains conversion errors for func types +func Funcs() { + type Local func(ID) + type Recursive func(Recursive) + + var ctor Constructor + var local Local + var recursive Recursive + + _ = Constructor(ctor) // want "unnecessary conversion" + _ = Local(local) // want "unnecessary conversion" + _ = Recursive(recursive) // want "unnecessary conversion" + + _ = (*Constructor)(&ctor) // want "unnecessary conversion" + _ = (*Local)(&local) // want "unnecessary conversion" + _ = (*Recursive)(&recursive) // want "unnecessary conversion" } From 2324f84b0a05e6ef5ac172bfe7a9f65047f5d616 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Sat, 9 Mar 2024 03:26:44 +0100 Subject: [PATCH 2/2] chore: remove FakeLoaderProgram adapter --- pkg/golinters/goanalysis/adapters.go | 41 ---------------------------- 1 file changed, 41 deletions(-) delete mode 100644 pkg/golinters/goanalysis/adapters.go diff --git a/pkg/golinters/goanalysis/adapters.go b/pkg/golinters/goanalysis/adapters.go deleted file mode 100644 index 284215dfc65d..000000000000 --- a/pkg/golinters/goanalysis/adapters.go +++ /dev/null @@ -1,41 +0,0 @@ -package goanalysis - -import ( - "go/types" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/loader" //nolint:staticcheck // it's an adapter for golang.org/x/tools/go/packages -) - -func MakeFakeLoaderProgram(pass *analysis.Pass) *loader.Program { - var info types.Info - if pass.TypesInfo != nil { - info = *pass.TypesInfo - } - - prog := &loader.Program{ - Fset: pass.Fset, - Created: []*loader.PackageInfo{ - { - Pkg: pass.Pkg, - Importable: true, // not used - TransitivelyErrorFree: true, // TODO ??? - - Files: pass.Files, - Errors: nil, - Info: info, - }, - }, - AllPackages: map[*types.Package]*loader.PackageInfo{ - pass.Pkg: { - Pkg: pass.Pkg, - Importable: true, - TransitivelyErrorFree: true, - Files: pass.Files, - Errors: nil, - Info: info, - }, - }, - } - return prog -}