diff --git a/.golangci.next.reference.yml b/.golangci.next.reference.yml index e08464bdd6b8..90e37aab7333 100644 --- a/.golangci.next.reference.yml +++ b/.golangci.next.reference.yml @@ -2607,6 +2607,7 @@ linters: - bidichk - bodyclose - canonicalheader + - constructorcheck - containedctx - contextcheck - copyloopvar @@ -2722,6 +2723,7 @@ linters: - bidichk - bodyclose - canonicalheader + - constructorcheck - containedctx - contextcheck - copyloopvar diff --git a/go.mod b/go.mod index adc5da1cfd9c..902287f6bfb4 100644 --- a/go.mod +++ b/go.mod @@ -86,6 +86,7 @@ require ( github.com/pelletier/go-toml/v2 v2.2.2 github.com/polyfloyd/go-errorlint v1.6.0 github.com/quasilyte/go-ruleguard/dsl v0.3.22 + github.com/reflechant/constructor-check v1.0.1 github.com/ryancurrah/gomodguard v1.3.3 github.com/ryanrolds/sqlclosecheck v0.5.1 github.com/sanposhiho/wastedassign/v2 v2.0.7 diff --git a/go.sum b/go.sum index 0d4fc6b658b4..34f95b2d048b 100644 --- a/go.sum +++ b/go.sum @@ -458,6 +458,8 @@ github.com/quasilyte/regex/syntax v0.0.0-20210819130434-b3f0c404a727 h1:TCg2WBOl github.com/quasilyte/regex/syntax v0.0.0-20210819130434-b3f0c404a727/go.mod h1:rlzQ04UMyJXu/aOvhd8qT+hvDrFpiwqp8MRXDY9szc0= github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 h1:M8mH9eK4OUR4lu7Gd+PU1fV2/qnDNfzT635KRSObncs= github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567/go.mod h1:DWNGW8A4Y+GyBgPuaQJuWiy0XYftx4Xm/y5Jqk9I6VQ= +github.com/reflechant/constructor-check v1.0.1 h1:25tl40Iw0CBVc23Sqdv3XfrY1arixstjgeVLKLCxu7w= +github.com/reflechant/constructor-check v1.0.1/go.mod h1:X+8Duv7dTdhWAhZ4MlVESYewP3ebgf/CcG9NjVZoQPA= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= diff --git a/jsonschema/golangci.next.jsonschema.json b/jsonschema/golangci.next.jsonschema.json index f8233a77759d..dd4eb80cf16b 100644 --- a/jsonschema/golangci.next.jsonschema.json +++ b/jsonschema/golangci.next.jsonschema.json @@ -304,6 +304,7 @@ "bidichk", "bodyclose", "canonicalheader", + "constructorcheck", "containedctx", "contextcheck", "copyloopvar", diff --git a/pkg/golinters/constructorcheck/constructorcheck.go b/pkg/golinters/constructorcheck/constructorcheck.go new file mode 100644 index 000000000000..5b8ffe7009f1 --- /dev/null +++ b/pkg/golinters/constructorcheck/constructorcheck.go @@ -0,0 +1,18 @@ +package constructorcheck + +import ( + "github.com/golangci/golangci-lint/pkg/goanalysis" + constructorcheck "github.com/reflechant/constructor-check/analyzer" + "golang.org/x/tools/go/analysis" +) + +func New() *goanalysis.Linter { + a := constructorcheck.Analyzer + + return goanalysis.NewLinter( + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + nil, + ).WithLoadMode(goanalysis.LoadModeTypesInfo) +} diff --git a/pkg/golinters/constructorcheck/constructorcheck_test.go b/pkg/golinters/constructorcheck/constructorcheck_test.go new file mode 100644 index 000000000000..5e7a6a5e644d --- /dev/null +++ b/pkg/golinters/constructorcheck/constructorcheck_test.go @@ -0,0 +1,11 @@ +package constructorcheck + +import ( + "testing" + + "github.com/golangci/golangci-lint/test/testshared/integration" +) + +func TestFromTestdata(t *testing.T) { + integration.RunTestdata(t) +} diff --git a/pkg/golinters/constructorcheck/testdata/constructorcheck.go b/pkg/golinters/constructorcheck/testdata/constructorcheck.go new file mode 100644 index 000000000000..c13faf07e05b --- /dev/null +++ b/pkg/golinters/constructorcheck/testdata/constructorcheck.go @@ -0,0 +1,96 @@ +//golangcitest:args -Econstructorcheck +package testdata + +import ( + "bytes" + "fmt" +) + +var buf = bytes.Buffer{} // standard library is excluded from analysis + +// T is a type whose zero values are supposedly invalid +// so a constructor NewT was created. +type T struct { + x int + s string + m map[int]int +} + +var ( + tNil *T // want `nil value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + tZero = T{} // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + tZeroPtr = &T{} // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + tNew = new(T) // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + tComposite = T{ // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + x: 1, + s: "abc", + } + tCompositePtr = &T{ // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + x: 1, + s: "abc", + } + tColl = []T{T{x: 1}} // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + tPtrColl = []*T{&T{x: 1}} // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + +) + +// NewT is a valid constructor for type T. Here we check if it's called +// instead of constructing values of type T manually +func NewT() *T { + return &T{ + m: make(map[int]int), + } +} + +type structWithTField struct { + i int + t T +} + +var structWithT = structWithTField{ + i: 1, + t: T{x: 1}, // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` +} + +type structWithTPtrField struct { + i int + t *T +} + +var structWithTPtr = structWithTPtrField{ + i: 1, + t: &T{x: 1}, // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` +} + +func fnWithT() { + x := T{} // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + x2 := &T{} // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + x3 := new(T) // want `zero value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + fmt.Println(x, x2, x3) +} + +func retT() T { + return T{ // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + x: 1, + } +} + +func retTPtr() *T { + return &T{ // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` + x: 1, + } +} + +func retTNilPtr() *T { + var t *T // want `nil value of type command-line-arguments.T may be unsafe, use constructor NewT instead` + return t +} + +type T2 struct { + x int +} + +func NewT2() *T2 { + // new(T) inside T's constructor is permitted + return new(T2) +} diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index 2e6c148e329c..ea3811559a5f 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -8,6 +8,7 @@ import ( "github.com/golangci/golangci-lint/pkg/golinters/bidichk" "github.com/golangci/golangci-lint/pkg/golinters/bodyclose" "github.com/golangci/golangci-lint/pkg/golinters/canonicalheader" + "github.com/golangci/golangci-lint/pkg/golinters/constructorcheck" "github.com/golangci/golangci-lint/pkg/golinters/containedctx" "github.com/golangci/golangci-lint/pkg/golinters/contextcheck" "github.com/golangci/golangci-lint/pkg/golinters/copyloopvar" @@ -161,6 +162,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithLoadForGoAnalysis(). WithURL("https://github.com/lasiar/canonicalHeader"), + linter.NewConfig(constructorcheck.New()). + WithSince("v1.61.0"). + WithPresets(linter.PresetStyle). + WithLoadForGoAnalysis(). + WithURL("https://github.com/reflechant/constructor-check"), + linter.NewConfig(containedctx.New()). WithSince("1.44.0"). WithLoadForGoAnalysis().