From c4525d12151785a98b5474c97db458fd44fb3f82 Mon Sep 17 00:00:00 2001 From: Roman Gerasimov Date: Wed, 21 Aug 2024 22:50:03 +0200 Subject: [PATCH 1/5] add constructorcheck --- .../constructorcheck/constructorcheck.go | 277 ++++++++++++++++++ .../constructorcheck/constructorcheck_test.go | 11 + .../testdata/constructorcheck.go | 96 ++++++ pkg/lint/lintersdb/builder_linter.go | 6 + 4 files changed, 390 insertions(+) create mode 100644 pkg/golinters/constructorcheck/constructorcheck.go create mode 100644 pkg/golinters/constructorcheck/constructorcheck_test.go create mode 100644 pkg/golinters/constructorcheck/testdata/constructorcheck.go diff --git a/pkg/golinters/constructorcheck/constructorcheck.go b/pkg/golinters/constructorcheck/constructorcheck.go new file mode 100644 index 000000000000..1ee79dbdcc84 --- /dev/null +++ b/pkg/golinters/constructorcheck/constructorcheck.go @@ -0,0 +1,277 @@ +// Package analyzer is a linter that reports ignored constructors. +// It shows you places where someone is doing T{} or &T{} +// instead of using NewT declared in the same package as T. +// A constructor for type T (only structs are supported at the moment) +// is a function with name "NewT" that returns a value of type T or *T. +// Types returned by constructors are not checked right now, +// only that type T inferred from the function name exists in the same package. +// Standard library packages are excluded from analysis. +package constructorcheck + +import ( + "go/ast" + "go/token" + "go/types" + "log" + "os/exec" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +type ConstructorFact struct { + ConstructorName string + Pos token.Pos + End token.Pos +} + +func (f *ConstructorFact) AFact() {} + +var Analyzer = &analysis.Analyzer{ + Name: "constructor_check", + Doc: "check for types constructed manually ignoring constructor", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + FactTypes: []analysis.Fact{(*ConstructorFact)(nil)}, +} + +var stdPackages = stdPackageNames() + +func run(pass *analysis.Pass) (interface{}, error) { + inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + (*ast.ValueSpec)(nil), + (*ast.CompositeLit)(nil), + (*ast.TypeSpec)(nil), + (*ast.FuncDecl)(nil), + } + + zeroValues := make(map[token.Pos]types.Object) + nilValues := make(map[token.Pos]types.Object) + compositeLiterals := make(map[token.Pos]types.Object) + typeAliases := make(map[types.Object]types.Object) + + inspector.Preorder(nodeFilter, func(node ast.Node) { + switch decl := node.(type) { + case *ast.CallExpr: + // check if it's a new call + fn, ok := decl.Fun.(*ast.Ident) + if !ok { + break + } + if fn.Name != "new" { + break + } + // check we have only one argument (the type) + if len(decl.Args) != 1 { + break + } + + ident := typeIdent(decl.Args[0]) + if ident == nil { + break + } + + typeObj := pass.TypesInfo.ObjectOf(ident) + if typeObj == nil { + break + } + zeroValues[node.Pos()] = typeObj + case *ast.ValueSpec: + // check it's a pointer value + starExpr, ok := decl.Type.(*ast.StarExpr) + if !ok { + break + } + // check it's using a named type + ident := typeIdent(starExpr.X) + if ident == nil { + break + } + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + break + } + nilValues[node.Pos()] = obj + case *ast.CompositeLit: + ident := typeIdent(decl.Type) + if ident == nil { + break + } + + obj := pass.TypesInfo.ObjectOf(ident) + if obj == nil { + break + } + // if it's a zero value literal + if decl.Elts == nil { + zeroValues[node.Pos()] = obj + break + } + compositeLiterals[node.Pos()] = obj + case *ast.TypeSpec: + // get base type if any + baseIdent := typeIdent(decl.Type) + if baseIdent == nil { + break + } + // get base type object + baseTypeObj := pass.TypesInfo.ObjectOf(baseIdent) + if baseTypeObj == nil { + break + } + + // get this type's object + typeObj := pass.TypesInfo.ObjectOf(decl.Name) + if typeObj == nil { + break + } + typeAliases[typeObj] = baseTypeObj + case *ast.FuncDecl: + // check if it's a function not a method + if decl.Recv != nil { + break + } + + // check if function name starts with "New" + if !strings.HasPrefix(decl.Name.Name, "New") { + break + } + + // check if function name follows the NewT template + // TODO: think about easing this requirement because often + // they rename types and forget to rename constructors + typeName, ok := strings.CutPrefix(decl.Name.Name, "New") + if !ok { + break + } + + // check if type T extracted from function name exists + obj := pass.Pkg.Scope().Lookup(typeName) + if obj == nil { + break + } + + // ignore standard library types + if _, ok := stdPackages[obj.Pkg().Name()]; ok { + break + } + // check if supposed constructor returns exactly one value + // TODO: implement other cases ? + // (T, err), (*T, err), (T, bool), (*T, bool) + returns := decl.Type.Results.List + if len(returns) != 1 { + break + } + // to be done later: + // // check if supposed constructor returns a value of type T or *T + // // declared in the same package and T equals extracted type name + + // assume we have a valid constructor + fact := ConstructorFact{ + ConstructorName: decl.Name.Name, + Pos: decl.Pos(), + End: decl.End(), + } + pass.ExportObjectFact(obj, &fact) + default: + // fmt.Printf("%#v\n", node) + } + }) + + for typeObj, baseTypeObj := range typeAliases { + // check the base type has a constructor + existingFact := new(ConstructorFact) + if !pass.ImportObjectFact(baseTypeObj, existingFact) { + continue + } + + // mark derived type as having constructor + newFact := ConstructorFact{ + ConstructorName: existingFact.ConstructorName, + Pos: existingFact.Pos, + End: existingFact.End, + } + pass.ExportObjectFact(typeObj, &newFact) + } + + for pos, obj := range nilValues { + if constr, ok := constructorName(pass, obj, pos); ok { + pass.Reportf( + pos, + "nil value of type %s may be unsafe, use constructor %s instead", + obj.Type(), + constr, + ) + } + } + for pos, obj := range zeroValues { + if constr, ok := constructorName(pass, obj, pos); ok { + pass.Reportf( + pos, + "zero value of type %s may be unsafe, use constructor %s instead", + obj.Type(), + constr, + ) + } + } + for pos, obj := range compositeLiterals { + if constr, ok := constructorName(pass, obj, pos); ok { + pass.Reportf( + pos, + "use constructor %s for type %s instead of a composite literal", + constr, + obj.Type(), + ) + } + } + + return nil, nil +} + +func constructorName(pass *analysis.Pass, obj types.Object, pos token.Pos) (string, bool) { + fact := new(ConstructorFact) + if !pass.ImportObjectFact(obj, fact) { + return "", false + } + + // if used inside T's constructor - ignore + if pos >= fact.Pos && + pos < fact.End { + return "", false + } + + return fact.ConstructorName, true +} + +// typeIdent returns either local or imported type ident or nil +func typeIdent(expr ast.Expr) *ast.Ident { + switch id := expr.(type) { + case *ast.Ident: + return id + case *ast.SelectorExpr: + return id.Sel + } + return nil +} + +func stdPackageNames() map[string]struct{} { + // inspired by https://pkg.go.dev/golang.org/x/tools/go/packages#Load + cmd := exec.Command("go", "list", "std") + + output, err := cmd.Output() + if err != nil { + log.Fatal("can't load standard library package names") + } + pkgs := strings.Fields(string(output)) + + stdPkgNames := make(map[string]struct{}, len(pkgs)) + for _, pkg := range pkgs { + stdPkgNames[pkg] = struct{}{} + } + return stdPkgNames +} 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..22e77ffcfa86 --- /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 { // want T:`{NewT \d* \d*}` + x int + s string + m map[int]int +} + +var ( + tNil *T // want `nil value of type p.T may be unsafe, use constructor NewT instead` + tZero = T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` + tZeroPtr = &T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` + tNew = new(T) // want `zero value of type p.T may be unsafe, use constructor NewT instead` + tComposite = T{ // want `use constructor NewT for type p.T instead of a composite literal` + x: 1, + s: "abc", + } + tCompositePtr = &T{ // want `use constructor NewT for type p.T instead of a composite literal` + x: 1, + s: "abc", + } + tColl = []T{T{x: 1}} // want `use constructor NewT for type p.T instead of a composite literal` + tPtrColl = []*T{&T{x: 1}} // want `use constructor NewT for type p.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 p.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 p.T instead of a composite literal` +} + +func fnWithT() { + x := T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` + x2 := &T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` + x3 := new(T) // want `zero value of type p.T may be unsafe, use constructor NewT instead` + fmt.Println(x, x2, x3) +} + +func retT() T { + return T{ // want `use constructor NewT for type p.T instead of a composite literal` + x: 1, + } +} + +func retTPtr() *T { + return &T{ // want `use constructor NewT for type p.T instead of a composite literal` + x: 1, + } +} + +func retTNilPtr() *T { + var t *T // want `nil value of type p.T may be unsafe, use constructor NewT instead` + return t +} + +type T2 struct { // want T2:`{NewT2 \d* \d*}` + 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..aa07170763af 100644 --- a/pkg/lint/lintersdb/builder_linter.go +++ b/pkg/lint/lintersdb/builder_linter.go @@ -161,6 +161,12 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithLoadForGoAnalysis(). WithURL("https://github.com/lasiar/canonicalHeader"), + linter.NewConfig(canonicalheader.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(). From 4c94d091b4cae71c0b1594b7099d62b9beae68bb Mon Sep 17 00:00:00 2001 From: Roman Gerasimov Date: Wed, 21 Aug 2024 23:10:23 +0200 Subject: [PATCH 2/5] add to other places --- .golangci.next.reference.yml | 2 + go.mod | 1 + go.sum | 2 + jsonschema/golangci.next.jsonschema.json | 1 + .../constructorcheck/constructorcheck.go | 281 +----------------- pkg/lint/lintersdb/builder_linter.go | 3 +- 6 files changed, 20 insertions(+), 270 deletions(-) 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..569d4b3bb84d 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 v0.6.0 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..8f22366f3a38 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 v0.6.0 h1:ACVHRcn+ih1+gReOlvOEM1nja6bFYOMLpqAJaoe29ck= +github.com/reflechant/constructor-check v0.6.0/go.mod h1:7VStHftrgw4FNrRYt2Cvl0D9Q3lqah7osWTsJ1a6irg= 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 index 1ee79dbdcc84..da800c275c3a 100644 --- a/pkg/golinters/constructorcheck/constructorcheck.go +++ b/pkg/golinters/constructorcheck/constructorcheck.go @@ -1,277 +1,20 @@ -// Package analyzer is a linter that reports ignored constructors. -// It shows you places where someone is doing T{} or &T{} -// instead of using NewT declared in the same package as T. -// A constructor for type T (only structs are supported at the moment) -// is a function with name "NewT" that returns a value of type T or *T. -// Types returned by constructors are not checked right now, -// only that type T inferred from the function name exists in the same package. -// Standard library packages are excluded from analysis. package constructorcheck import ( - "go/ast" - "go/token" - "go/types" - "log" - "os/exec" - "strings" - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" -) - -type ConstructorFact struct { - ConstructorName string - Pos token.Pos - End token.Pos -} - -func (f *ConstructorFact) AFact() {} - -var Analyzer = &analysis.Analyzer{ - Name: "constructor_check", - Doc: "check for types constructed manually ignoring constructor", - Run: run, - Requires: []*analysis.Analyzer{inspect.Analyzer}, - FactTypes: []analysis.Fact{(*ConstructorFact)(nil)}, -} - -var stdPackages = stdPackageNames() - -func run(pass *analysis.Pass) (interface{}, error) { - inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) - - nodeFilter := []ast.Node{ - (*ast.CallExpr)(nil), - (*ast.ValueSpec)(nil), - (*ast.CompositeLit)(nil), - (*ast.TypeSpec)(nil), - (*ast.FuncDecl)(nil), - } - - zeroValues := make(map[token.Pos]types.Object) - nilValues := make(map[token.Pos]types.Object) - compositeLiterals := make(map[token.Pos]types.Object) - typeAliases := make(map[types.Object]types.Object) - - inspector.Preorder(nodeFilter, func(node ast.Node) { - switch decl := node.(type) { - case *ast.CallExpr: - // check if it's a new call - fn, ok := decl.Fun.(*ast.Ident) - if !ok { - break - } - if fn.Name != "new" { - break - } - // check we have only one argument (the type) - if len(decl.Args) != 1 { - break - } - - ident := typeIdent(decl.Args[0]) - if ident == nil { - break - } - - typeObj := pass.TypesInfo.ObjectOf(ident) - if typeObj == nil { - break - } - zeroValues[node.Pos()] = typeObj - case *ast.ValueSpec: - // check it's a pointer value - starExpr, ok := decl.Type.(*ast.StarExpr) - if !ok { - break - } - // check it's using a named type - ident := typeIdent(starExpr.X) - if ident == nil { - break - } - obj := pass.TypesInfo.ObjectOf(ident) - if obj == nil { - break - } - nilValues[node.Pos()] = obj - case *ast.CompositeLit: - ident := typeIdent(decl.Type) - if ident == nil { - break - } - - obj := pass.TypesInfo.ObjectOf(ident) - if obj == nil { - break - } - // if it's a zero value literal - if decl.Elts == nil { - zeroValues[node.Pos()] = obj - break - } - compositeLiterals[node.Pos()] = obj - case *ast.TypeSpec: - // get base type if any - baseIdent := typeIdent(decl.Type) - if baseIdent == nil { - break - } - // get base type object - baseTypeObj := pass.TypesInfo.ObjectOf(baseIdent) - if baseTypeObj == nil { - break - } - - // get this type's object - typeObj := pass.TypesInfo.ObjectOf(decl.Name) - if typeObj == nil { - break - } - typeAliases[typeObj] = baseTypeObj - case *ast.FuncDecl: - // check if it's a function not a method - if decl.Recv != nil { - break - } - - // check if function name starts with "New" - if !strings.HasPrefix(decl.Name.Name, "New") { - break - } - // check if function name follows the NewT template - // TODO: think about easing this requirement because often - // they rename types and forget to rename constructors - typeName, ok := strings.CutPrefix(decl.Name.Name, "New") - if !ok { - break - } - - // check if type T extracted from function name exists - obj := pass.Pkg.Scope().Lookup(typeName) - if obj == nil { - break - } - - // ignore standard library types - if _, ok := stdPackages[obj.Pkg().Name()]; ok { - break - } - // check if supposed constructor returns exactly one value - // TODO: implement other cases ? - // (T, err), (*T, err), (T, bool), (*T, bool) - returns := decl.Type.Results.List - if len(returns) != 1 { - break - } - // to be done later: - // // check if supposed constructor returns a value of type T or *T - // // declared in the same package and T equals extracted type name - - // assume we have a valid constructor - fact := ConstructorFact{ - ConstructorName: decl.Name.Name, - Pos: decl.Pos(), - End: decl.End(), - } - pass.ExportObjectFact(obj, &fact) - default: - // fmt.Printf("%#v\n", node) - } - }) - - for typeObj, baseTypeObj := range typeAliases { - // check the base type has a constructor - existingFact := new(ConstructorFact) - if !pass.ImportObjectFact(baseTypeObj, existingFact) { - continue - } - - // mark derived type as having constructor - newFact := ConstructorFact{ - ConstructorName: existingFact.ConstructorName, - Pos: existingFact.Pos, - End: existingFact.End, - } - pass.ExportObjectFact(typeObj, &newFact) - } - - for pos, obj := range nilValues { - if constr, ok := constructorName(pass, obj, pos); ok { - pass.Reportf( - pos, - "nil value of type %s may be unsafe, use constructor %s instead", - obj.Type(), - constr, - ) - } - } - for pos, obj := range zeroValues { - if constr, ok := constructorName(pass, obj, pos); ok { - pass.Reportf( - pos, - "zero value of type %s may be unsafe, use constructor %s instead", - obj.Type(), - constr, - ) - } - } - for pos, obj := range compositeLiterals { - if constr, ok := constructorName(pass, obj, pos); ok { - pass.Reportf( - pos, - "use constructor %s for type %s instead of a composite literal", - constr, - obj.Type(), - ) - } - } - - return nil, nil -} - -func constructorName(pass *analysis.Pass, obj types.Object, pos token.Pos) (string, bool) { - fact := new(ConstructorFact) - if !pass.ImportObjectFact(obj, fact) { - return "", false - } - - // if used inside T's constructor - ignore - if pos >= fact.Pos && - pos < fact.End { - return "", false - } - - return fact.ConstructorName, true -} - -// typeIdent returns either local or imported type ident or nil -func typeIdent(expr ast.Expr) *ast.Ident { - switch id := expr.(type) { - case *ast.Ident: - return id - case *ast.SelectorExpr: - return id.Sel - } - return nil -} - -func stdPackageNames() map[string]struct{} { - // inspired by https://pkg.go.dev/golang.org/x/tools/go/packages#Load - cmd := exec.Command("go", "list", "std") + "github.com/golangci/golangci-lint/pkg/goanalysis" + constructorcheck "github.com/reflechant/constructor-check/analyzer" +) - output, err := cmd.Output() - if err != nil { - log.Fatal("can't load standard library package names") - } - pkgs := strings.Fields(string(output)) +func New() *goanalysis.Linter { + a := constructorcheck.Analyzer - stdPkgNames := make(map[string]struct{}, len(pkgs)) - for _, pkg := range pkgs { - stdPkgNames[pkg] = struct{}{} - } - return stdPkgNames + return goanalysis.NewLinter( + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + nil, + ).WithLoadMode(goanalysis.LoadModeSyntax) + // WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/lint/lintersdb/builder_linter.go b/pkg/lint/lintersdb/builder_linter.go index aa07170763af..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,7 +162,7 @@ func (LinterBuilder) Build(cfg *config.Config) ([]*linter.Config, error) { WithLoadForGoAnalysis(). WithURL("https://github.com/lasiar/canonicalHeader"), - linter.NewConfig(canonicalheader.New()). + linter.NewConfig(constructorcheck.New()). WithSince("v1.61.0"). WithPresets(linter.PresetStyle). WithLoadForGoAnalysis(). From 61c62065ce0a0175cf9b815bf37547ecf648d4e0 Mon Sep 17 00:00:00 2001 From: Roman Gerasimov Date: Wed, 21 Aug 2024 23:16:09 +0200 Subject: [PATCH 3/5] organize imports --- pkg/golinters/constructorcheck/constructorcheck.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/golinters/constructorcheck/constructorcheck.go b/pkg/golinters/constructorcheck/constructorcheck.go index da800c275c3a..f67df834ce50 100644 --- a/pkg/golinters/constructorcheck/constructorcheck.go +++ b/pkg/golinters/constructorcheck/constructorcheck.go @@ -1,10 +1,9 @@ package constructorcheck import ( - "golang.org/x/tools/go/analysis" - "github.com/golangci/golangci-lint/pkg/goanalysis" constructorcheck "github.com/reflechant/constructor-check/analyzer" + "golang.org/x/tools/go/analysis" ) func New() *goanalysis.Linter { From 0997d5f8c1ba136b94b15e7085c1d57ba876e05a Mon Sep 17 00:00:00 2001 From: Roman Gerasimov Date: Wed, 21 Aug 2024 23:53:17 +0200 Subject: [PATCH 4/5] update constructorcheck dependency to v1.0.0 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 569d4b3bb84d..f0d0b1a03393 100644 --- a/go.mod +++ b/go.mod @@ -86,7 +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 v0.6.0 + github.com/reflechant/constructor-check v1.0.0 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 8f22366f3a38..43c6f2c63f24 100644 --- a/go.sum +++ b/go.sum @@ -458,8 +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 v0.6.0 h1:ACVHRcn+ih1+gReOlvOEM1nja6bFYOMLpqAJaoe29ck= -github.com/reflechant/constructor-check v0.6.0/go.mod h1:7VStHftrgw4FNrRYt2Cvl0D9Q3lqah7osWTsJ1a6irg= +github.com/reflechant/constructor-check v1.0.0 h1:x+u0abm28ZETNXq5MTnUyarS653I1GyNnkgadi7i6SE= +github.com/reflechant/constructor-check v1.0.0/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= From f5014e91145f5ff5fdf58781cd56e1ed252c1868 Mon Sep 17 00:00:00 2001 From: Roman Gerasimov Date: Thu, 22 Aug 2024 00:11:28 +0200 Subject: [PATCH 5/5] fix tests --- go.mod | 2 +- go.sum | 4 +-- .../constructorcheck/constructorcheck.go | 3 +- .../testdata/constructorcheck.go | 36 +++++++++---------- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/go.mod b/go.mod index f0d0b1a03393..902287f6bfb4 100644 --- a/go.mod +++ b/go.mod @@ -86,7 +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.0 + 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 43c6f2c63f24..34f95b2d048b 100644 --- a/go.sum +++ b/go.sum @@ -458,8 +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.0 h1:x+u0abm28ZETNXq5MTnUyarS653I1GyNnkgadi7i6SE= -github.com/reflechant/constructor-check v1.0.0/go.mod h1:X+8Duv7dTdhWAhZ4MlVESYewP3ebgf/CcG9NjVZoQPA= +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/pkg/golinters/constructorcheck/constructorcheck.go b/pkg/golinters/constructorcheck/constructorcheck.go index f67df834ce50..5b8ffe7009f1 100644 --- a/pkg/golinters/constructorcheck/constructorcheck.go +++ b/pkg/golinters/constructorcheck/constructorcheck.go @@ -14,6 +14,5 @@ func New() *goanalysis.Linter { a.Doc, []*analysis.Analyzer{a}, nil, - ).WithLoadMode(goanalysis.LoadModeSyntax) - // WithLoadMode(goanalysis.LoadModeTypesInfo) + ).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/golinters/constructorcheck/testdata/constructorcheck.go b/pkg/golinters/constructorcheck/testdata/constructorcheck.go index 22e77ffcfa86..c13faf07e05b 100644 --- a/pkg/golinters/constructorcheck/testdata/constructorcheck.go +++ b/pkg/golinters/constructorcheck/testdata/constructorcheck.go @@ -10,27 +10,27 @@ 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 { // want T:`{NewT \d* \d*}` +type T struct { x int s string m map[int]int } var ( - tNil *T // want `nil value of type p.T may be unsafe, use constructor NewT instead` - tZero = T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` - tZeroPtr = &T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` - tNew = new(T) // want `zero value of type p.T may be unsafe, use constructor NewT instead` - tComposite = T{ // want `use constructor NewT for type p.T instead of a composite literal` + 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 p.T instead of a composite literal` + 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 p.T instead of a composite literal` - tPtrColl = []*T{&T{x: 1}} // want `use constructor NewT for type p.T instead of a composite literal` + 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` ) @@ -49,7 +49,7 @@ type structWithTField struct { var structWithT = structWithTField{ i: 1, - t: T{x: 1}, // want `use constructor NewT for type p.T instead of a composite literal` + t: T{x: 1}, // want `use constructor NewT for type command-line-arguments.T instead of a composite literal` } type structWithTPtrField struct { @@ -59,34 +59,34 @@ type structWithTPtrField struct { var structWithTPtr = structWithTPtrField{ i: 1, - t: &T{x: 1}, // want `use constructor NewT for type p.T instead of a composite literal` + 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 p.T may be unsafe, use constructor NewT instead` - x2 := &T{} // want `zero value of type p.T may be unsafe, use constructor NewT instead` - x3 := new(T) // want `zero value of type p.T may be unsafe, use constructor NewT instead` + 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 p.T instead of a composite literal` + 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 p.T instead of a composite literal` + 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 p.T may be unsafe, use constructor NewT instead` + var t *T // want `nil value of type command-line-arguments.T may be unsafe, use constructor NewT instead` return t } -type T2 struct { // want T2:`{NewT2 \d* \d*}` +type T2 struct { x int }