From 113a15f07996ee4dc3aa3f38c637159b8861cb5e Mon Sep 17 00:00:00 2001 From: Anton Zinovyev Date: Thu, 23 Mar 2023 23:49:04 +0100 Subject: [PATCH] Ground-up rework of a linter (#41) Add support of generics Add support of nested structures Rework regexp matching Rework error message --- .github/workflows/ci.yml | 13 +- .golangci.yaml | 83 +++++- README.md | 21 +- analyzer/analyzer.go | 238 +++++++++++++++++ analyzer/analyzer_benchmark_test.go | 25 ++ analyzer/analyzer_test.go | 42 +++ {testdata => analyzer/testdata}/src/e/e.go | 1 + analyzer/testdata/src/i/i.go | 194 ++++++++++++++ cmd/exhaustruct/main.go | 4 +- go.mod | 9 +- go.sum | 23 +- internal/fields/struct.go | 124 +++++++++ internal/fields/struct_test.go | 160 ++++++++++++ internal/fields/testdata/structs.go | 30 +++ internal/pattern/list.go | 82 ++++++ internal/pattern/list_test.go | 68 +++++ pkg/analyzer/analyzer.go | 289 --------------------- pkg/analyzer/analyzer_test.go | 53 ---- pkg/analyzer/patterns-list.go | 68 ----- pkg/analyzer/struct-fields.go | 61 ----- testdata/src/s/s.go | 146 ----------- 21 files changed, 1079 insertions(+), 655 deletions(-) create mode 100644 analyzer/analyzer.go create mode 100644 analyzer/analyzer_benchmark_test.go create mode 100644 analyzer/analyzer_test.go rename {testdata => analyzer/testdata}/src/e/e.go (90%) create mode 100644 analyzer/testdata/src/i/i.go create mode 100644 internal/fields/struct.go create mode 100644 internal/fields/struct_test.go create mode 100644 internal/fields/testdata/structs.go create mode 100644 internal/pattern/list.go create mode 100644 internal/pattern/list_test.go delete mode 100644 pkg/analyzer/analyzer.go delete mode 100644 pkg/analyzer/analyzer_test.go delete mode 100644 pkg/analyzer/patterns-list.go delete mode 100644 pkg/analyzer/struct-fields.go delete mode 100644 testdata/src/s/s.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ea21478..72361e8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,11 @@ jobs: go-version: "1.20" check-latest: true - uses: actions/checkout@v3 - - uses: golangci/golangci-lint-action@v3 + - name: "Lint" + uses: golangci/golangci-lint-action@v3 + with: + only-new-issues: true + test: name: "Test" @@ -33,12 +37,7 @@ jobs: go-version: "1.20" check-latest: true - uses: actions/checkout@v3 - - name: "Run tests" - run: go test -json ./... > test.json - - name: "Annotate tests" - uses: guyarb/golang-test-annotations@v0.6.0 - with: - test-results: test.json + - uses: n8maninger/action-golang-test@v1 dependabot-merge: name: "Dependabot auto-merge" diff --git a/.golangci.yaml b/.golangci.yaml index bd7d37da..fb3c300d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,20 +1,95 @@ run: + timeout: 10s allow-parallel-runners: true linters: enable-all: true disable: - - varnamelen - - nonamedreturns - - gofumpt + - depguard - gci + - godox - gofmt + - gofumpt + - goheader - goimports + - varnamelen - exhaustivestruct # deprecated - - scopelint # deprecated + - nonamedreturns # we're using named returns quite often + - errname # naming is too strict and is not observed in many places - golint # deprecated + - importas # may be useful later, requires configuration - interfacer # deprecated - maligned # deprecated + - misspell # we have IDE speller + - scopelint # obsoleted, replaced by other linters + - govet # invoked by the goland internally + - tagliatelle # it isn't very handy to have such linter in a monorepo with a lot of different coding standards + - nlreturn # there is wsl linter what implements the same checks + - ifshort # deprecated in 1.48 + - structcheck # deprecated 1.49 + - varcheck # deprecated 1.49 + - nosnakecase # deprecated 1.48 + - deadcode # deprecated 1.49 + - lll # disabled in favor of revive + - funlen # disabled in favor of revive + - gocognit # disabled in favor of revive + - cyclop # disabled in favor of revive + - gocyclo # disabled in favor of revive + +linters-settings: + wsl: + force-case-trailing-whitespace: 1 + + revive: + enable-all-rules: true + confidence: 0.8 + rules: + - name: comment-spacings + severity: warning + disabled: false + arguments: [ "nolint" ] + - name: function-length + severity: warning + disabled: false + arguments: [ 50, 0 ] + - name: function-result-limit + severity: warning + disabled: false + arguments: [ 3 ] + - name: cognitive-complexity + severity: warning + disabled: false + arguments: [ 20 ] + - name: cyclomatic + severity: warning + disabled: false + arguments: [ 10 ] + - name: line-length-limit + severity: warning + disabled: false + arguments: [ 110 ] + - name: argument-limit + severity: warning + disabled: false + arguments: [ 6 ] + # disabled rules + - name: max-public-structs # quite annoying rule + disabled: true + - name: banned-characters # we don't have banned chars + disabled: true + - name: file-header # we don't have a file headers + disabled: true + - name: flag-parameter # extremely annoying linter, it is absolutely okay to have boolean args + disabled: true + - name: struct-tag # false-positive on tags implemented by other linters + disabled: true + - name: unhandled-error # dont have proper exclusions list ToDo: make a PR + disabled: true + - name: add-constant # dont have exclusions list ToDo: make a PR + disabled: true + - name: empty-lines # it false-positives on one-liners ToDo: make a PR + disabled: true + issues: max-issues-per-linter: 0 diff --git a/README.md b/README.md index 1d178189..3b49d6fe 100644 --- a/README.md +++ b/README.md @@ -14,25 +14,10 @@ `exhaustruct` is a golang analyzer that finds structures with uninitialized fields -#### The "why?" - -There is a similar linter [exhaustivestruct](https://github.com/mbilski/exhaustivestruct), but it is abandoned -and not -optimal. - -This linter can be called a successor of `exhaustivestruct`, and: - -- it is at least **2.5+ times faster**, due to better algorithm; -- can receive `include` and/or `exclude` patterns; -- allows to mark fields as optional (not required to be filled on struct init), via field - tag `exhaustruct:"optional"`; -- expects received patterns to be RegExp, therefore this package is not api-compatible - with `exhaustivestruct`. - ### Installation ```shell -go get -u github.com/GaijinEntertainment/go-exhaustruct/cmd/exhaustruct +go get -u github.com/GaijinEntertainment/go-exhaustruct/v3/cmd/exhaustruct ``` ### Usage @@ -42,9 +27,9 @@ exhaustruct [-flag] [package] Flags: -i value - Regular expression to match struct packages and names, can receive multiple flags + Regular expression to match structures, can receive multiple flags -e value - Regular expression to exclude struct packages and names, can receive multiple flags + Regular expression to exclude structures, can receive multiple flags ``` ### Example diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go new file mode 100644 index 00000000..e596bb65 --- /dev/null +++ b/analyzer/analyzer.go @@ -0,0 +1,238 @@ +package analyzer + +import ( + "flag" + "fmt" + "go/ast" + "go/token" + "go/types" + "sync" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" + + "github.com/GaijinEntertainment/go-exhaustruct/v3/internal/fields" + "github.com/GaijinEntertainment/go-exhaustruct/v3/internal/pattern" +) + +type analyzer struct { + include pattern.List `exhaustruct:"optional"` + exclude pattern.List `exhaustruct:"optional"` + + fieldsCache map[types.Type]fields.StructFields + fieldsCacheMu sync.RWMutex `exhaustruct:"optional"` + + typeProcessingNeed map[types.Type]bool + typeProcessingNeedMu sync.RWMutex `exhaustruct:"optional"` +} + +func NewAnalyzer(include, exclude []string) (*analysis.Analyzer, error) { + a := analyzer{ + fieldsCache: make(map[types.Type]fields.StructFields), + typeProcessingNeed: make(map[types.Type]bool), + } + + var err error + + a.include, err = pattern.NewList(include...) + if err != nil { + return nil, err //nolint:wrapcheck + } + + a.exclude, err = pattern.NewList(exclude...) + if err != nil { + return nil, err //nolint:wrapcheck + } + + return &analysis.Analyzer{ //nolint:exhaustruct + Name: "exhaustruct", + Doc: "Checks if all structure fields are initialized", + Run: a.run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Flags: a.newFlagSet(), + }, nil +} + +func (a *analyzer) newFlagSet() flag.FlagSet { + fs := flag.NewFlagSet("", flag.PanicOnError) + + fs.Var(&a.include, "i", "Regular expression to match structures, can receive multiple flags") + fs.Var(&a.exclude, "e", "Regular expression to exclude structures, can receive multiple flags") + + return *fs +} + +func (a *analyzer) run(pass *analysis.Pass) (any, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) //nolint:forcetypeassert + + insp.WithStack( + []ast.Node{ + (*ast.CompositeLit)(nil), + }, + a.newVisitor(pass), + ) + + return nil, nil //nolint:nilnil +} + +// newVisitor returns visitor that only expects [ast.CompositeLit] nodes. +func (a *analyzer) newVisitor(pass *analysis.Pass) func(n ast.Node, push bool, stack []ast.Node) bool { + return func(n ast.Node, push bool, stack []ast.Node) bool { + if !push { + return true + } + + lit, ok := n.(*ast.CompositeLit) + if !ok { + // this should never happen, but better be prepared + return true + } + + structTyp, namedTyp, ok := getStructType(pass, lit) + if !ok { + return true + } + + if len(lit.Elts) == 0 { + if ret, ok := stackParentIsReturn(stack); ok { + if returnContainsNonNilError(pass, ret) { + // it is okay to return uninitialized structure in case struct's direct parent is + // a return statement containing non-nil error + // + // we're unable to check if returned error is custom, but at leas we're able to + // cover str [error] type. + return true + } + } + } + + pos, msg := a.processStruct(pass, lit, structTyp, namedTyp) + if pos != nil { + pass.Reportf(*pos, msg) + } + + return true + } +} + +func getStructType(pass *analysis.Pass, lit *ast.CompositeLit) (*types.Struct, *types.Named, bool) { + switch typ := pass.TypesInfo.TypeOf(lit).(type) { + case *types.Named: // named type + if structTyp, ok := typ.Underlying().(*types.Struct); ok { + return structTyp, typ, true + } + + return nil, nil, false + + case *types.Struct: // anonymous struct + return typ, nil, true + + default: + return nil, nil, false + } +} + +func stackParentIsReturn(stack []ast.Node) (*ast.ReturnStmt, bool) { + // it is safe to skip boundary check, since stack always has at least one element + // - whole file. + ret, ok := stack[len(stack)-2].(*ast.ReturnStmt) + + return ret, ok +} + +func returnContainsNonNilError(pass *analysis.Pass, ret *ast.ReturnStmt) bool { + // errors are mostly located at the end of return statement, so we're starting + // from the end. + for i := len(ret.Results) - 1; i >= 0; i-- { + if pass.TypesInfo.TypeOf(ret.Results[i]).String() == "error" { + return true + } + } + + return false +} + +func (a *analyzer) processStruct( + pass *analysis.Pass, + lit *ast.CompositeLit, + structTyp *types.Struct, + namedTyp *types.Named, +) (*token.Pos, string) { + if !a.shouldProcessType(namedTyp) { + return nil, "" + } + + // unnamed structures are only defined in same package, along with types that has + // prefix identical to current package name. + isSamePackage := namedTyp == nil || pass.Pkg.Scope().Lookup(namedTyp.Obj().Name()) != nil + + if f := a.litSkippedFields(lit, structTyp, !isSamePackage); len(f) > 0 { + structName := "anonymous struct" + if namedTyp != nil { + structName = namedTyp.Obj().Pkg().Name() + "." + namedTyp.Obj().Name() + } + + pos := lit.Pos() + + if len(f) == 1 { + return &pos, fmt.Sprintf("%s is missing field %s", structName, f.String()) + } + + return &pos, fmt.Sprintf("%s is missing fields %s", structName, f.String()) + } + + return nil, "" +} + +// shouldProcessType returns true if type should be processed basing off include +// and exclude patterns, defined though constructor and\or flags. +func (a *analyzer) shouldProcessType(typ *types.Named) bool { + if typ == nil || (len(a.include) == 0 && len(a.exclude) == 0) { + // anonymous structs or in case no filtering configured + return true + } + + a.typeProcessingNeedMu.RLock() + res, ok := a.typeProcessingNeed[typ] + a.typeProcessingNeedMu.RUnlock() + + if !ok { + a.typeProcessingNeedMu.Lock() + typStr := typ.String() + res = true + + if a.include != nil && !a.include.MatchFullString(typStr) { + res = false + } + + if res && a.exclude != nil && a.exclude.MatchFullString(typStr) { + res = false + } + + a.typeProcessingNeed[typ] = res + a.typeProcessingNeedMu.Unlock() + } + + return res +} + +//revive:disable-next-line:unused-receiver +func (a *analyzer) litSkippedFields( + lit *ast.CompositeLit, + typ *types.Struct, + onlyExported bool, +) fields.StructFields { + a.fieldsCacheMu.RLock() + f, ok := a.fieldsCache[typ] + a.fieldsCacheMu.RUnlock() + + if !ok { + a.fieldsCacheMu.Lock() + f = fields.NewStructFields(typ) + a.fieldsCache[typ] = f + a.fieldsCacheMu.Unlock() + } + + return f.SkippedFields(lit, onlyExported) +} diff --git a/analyzer/analyzer_benchmark_test.go b/analyzer/analyzer_benchmark_test.go new file mode 100644 index 00000000..73eed2e1 --- /dev/null +++ b/analyzer/analyzer_benchmark_test.go @@ -0,0 +1,25 @@ +package analyzer_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer" +) + +func BenchmarkAnalyzer(b *testing.B) { + a, err := analyzer.NewAnalyzer( + []string{`.*[Tt]est.*`, `.*External`, `.*Embedded`}, + []string{`.*Excluded$`}, + ) + require.NoError(b, err) + + b.ResetTimer() + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + _ = analysistest.Run(b, testdataPath, a, "i") + } +} diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go new file mode 100644 index 00000000..2e6ee783 --- /dev/null +++ b/analyzer/analyzer_test.go @@ -0,0 +1,42 @@ +package analyzer_test + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer" +) + +var testdataPath, _ = filepath.Abs("./testdata/") //nolint:gochecknoglobals + +func TestAnalyzer(t *testing.T) { + t.Parallel() + + a, err := analyzer.NewAnalyzer([]string{""}, nil) + assert.Nil(t, a) + assert.Error(t, err) + + a, err = analyzer.NewAnalyzer([]string{"["}, nil) + assert.Nil(t, a) + assert.Error(t, err) + + a, err = analyzer.NewAnalyzer(nil, []string{""}) + assert.Nil(t, a) + assert.Error(t, err) + + a, err = analyzer.NewAnalyzer(nil, []string{"["}) + assert.Nil(t, a) + assert.Error(t, err) + + a, err = analyzer.NewAnalyzer( + []string{`.*[Tt]est.*`, `.*External`, `.*Embedded`}, + []string{`.*Excluded$`}, + ) + require.NoError(t, err) + + analysistest.Run(t, testdataPath, a, "i") +} diff --git a/testdata/src/e/e.go b/analyzer/testdata/src/e/e.go similarity index 90% rename from testdata/src/e/e.go rename to analyzer/testdata/src/e/e.go index c285f268..858d1130 100644 --- a/testdata/src/e/e.go +++ b/analyzer/testdata/src/e/e.go @@ -1,3 +1,4 @@ +//nolint:all package e type External struct { diff --git a/analyzer/testdata/src/i/i.go b/analyzer/testdata/src/i/i.go new file mode 100644 index 00000000..b0fa2823 --- /dev/null +++ b/analyzer/testdata/src/i/i.go @@ -0,0 +1,194 @@ +//nolint:all +package i + +import ( + "errors" + + "e" +) + +type Embedded struct { + E string + F string + g string + H string +} + +type Test struct { + A string + B int + C float32 + D bool + E string `exhaustruct:"optional"` +} + +type Test2 struct { + Embedded + External e.External +} + +func shouldPassFullyDefined() { + _ = Test{ + A: "", + B: 0, + C: 0.0, + D: false, + E: "", + } +} + +func shouldPassPointer() { + _ = &Test{ + A: "", + B: 0, + C: 0.0, + D: false, + E: "", + } +} + +func shouldPassOnlyOptionalOmitted() { + _ = Test{ + A: "", + B: 0, + C: 0.0, + D: false, + } +} + +func shouldFailRequiredOmitted() { + _ = Test{ // want "i.Test is missing field D" + A: "", + B: 0, + C: 0.0, + } +} + +func shouldPassEmptyStructWithNonNilErr() (Test, error) { + return Test{}, errors.New("some error") +} + +func shouldFailEmptyStructWithNilErr() (Test, error) { + return Test{}, nil // want "i.Test is missing fields A, B, C, D" +} + +func shouldFailEmptyNestedStructWithNonNilErr() ([]Test, error) { + return []Test{{}}, nil // want "i.Test is missing fields A, B, C, D" +} + +func shouldPassUnnamed() { + _ = []Test{{"", 0, 0.0, false, ""}} +} + +func shouldPassEmbedded() { + _ = Test2{ + External: e.External{ + A: "", + B: "", + }, + Embedded: Embedded{ + E: "", + F: "", + H: "", + g: "", + }, + } +} + +func shouldFailEmbedded() { + _ = Test2{ + External: e.External{ + A: "", + B: "", + }, + Embedded: Embedded{ // want "Embedded is missing field g" + E: "", + F: "", + H: "", + }, + } +} + +func shouldFailEmbeddedCompletelyMissing() { + _ = Test2{ // want "i.Test2 is missing field Embedded" + External: e.External{ // want "e.External is missing field B" + A: "", + }, + } +} + +type testGenericStruct[T any] struct { + A T + B string +} + +func shouldPassGeneric() { + _ = testGenericStruct[int]{ + A: 42, + B: "the answer", + } +} + +func shouldFailGeneric() { + _ = testGenericStruct[int]{} // want "i.testGenericStruct is missing fields A, B" + _ = testGenericStruct[int]{ // want "i.testGenericStruct is missing field B" + A: 42, + } +} + +type TestExcluded struct { + A string + B int +} + +func shouldPassExcluded() { + _ = TestExcluded{} +} + +type NotIncluded struct { + A string + B int +} + +func shouldPassNotIncluded() { + _ = NotIncluded{} +} + +type Test3 struct { + A string + B int `exhaustruct:"optional"` +} + +func shouldPassSlicesOfStructs() { + _ = []Test3{ + {"a", 1}, + {A: "a"}, + Test3{A: "b"}, + } +} + +func shouldFailSlicesOfStructs() { + _ = []Test3{ + {}, // want "i.Test3 is missing field A" + Test3{B: 123}, // want "i.Test3 is missing field A" + } +} + +func shouldPassMapOfStructs() { + _ = map[string]Test3{ + "a": {"a", 1}, + "b": {A: "a"}, + "c": Test3{A: "b"}, + } +} + +func shouldFailMapOfStructs() { + _ = map[string]Test3{ + "a": {}, // want "i.Test3 is missing field A" + "b": Test3{B: 123}, // want "i.Test3 is missing field A" + } +} + +func shouldPassSlice() { + _ = []string{"a", "b"} +} diff --git a/cmd/exhaustruct/main.go b/cmd/exhaustruct/main.go index 3fc7e3e9..b2e20435 100644 --- a/cmd/exhaustruct/main.go +++ b/cmd/exhaustruct/main.go @@ -5,13 +5,13 @@ import ( "golang.org/x/tools/go/analysis/singlechecker" - "github.com/GaijinEntertainment/go-exhaustruct/v2/pkg/analyzer" + "github.com/GaijinEntertainment/go-exhaustruct/v3/analyzer" ) func main() { flag.Bool("unsafeptr", false, "") - a, err := analyzer.NewAnalyzer([]string{}, []string{}) + a, err := analyzer.NewAnalyzer(nil, nil) if err != nil { panic(err) } diff --git a/go.mod b/go.mod index 7dae6e30..6c40f5f4 100644 --- a/go.mod +++ b/go.mod @@ -1,13 +1,16 @@ -module github.com/GaijinEntertainment/go-exhaustruct/v2 +module github.com/GaijinEntertainment/go-exhaustruct/v3 go 1.20 require ( - golang.org/x/exp v0.0.0-20230303215020-44a13b063f3e - golang.org/x/tools v0.7.0 + github.com/stretchr/testify v1.8.2 + golang.org/x/tools v0.6.0 ) require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect golang.org/x/mod v0.9.0 // indirect golang.org/x/sys v0.6.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 287cc4ef..bb0ce0ee 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,24 @@ -golang.org/x/exp v0.0.0-20230303215020-44a13b063f3e h1:S8xf0d0OEmWrClvbMiUSp+7cGD00txONylwExlf9wR0= -golang.org/x/exp v0.0.0-20230303215020-44a13b063f3e/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= golang.org/x/mod v0.9.0 h1:KENHtAZL2y3NLMYZeHY9DW8HW8V+kQyJsY/V9JlKvCs= golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= -golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= +golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/fields/struct.go b/internal/fields/struct.go new file mode 100644 index 00000000..af2390e8 --- /dev/null +++ b/internal/fields/struct.go @@ -0,0 +1,124 @@ +package fields + +import ( + "go/ast" + "go/types" + "reflect" +) + +const ( + TagName = "exhaustruct" + OptionalTagValue = "optional" +) + +type StructField struct { + Name string + Exported bool + Optional bool +} + +type StructFields []*StructField + +// NewStructFields creates a new [StructFields] from a given struct type. +// StructFields items are listed in order they appear in the struct. +func NewStructFields(strct *types.Struct) StructFields { + sf := make(StructFields, 0, strct.NumFields()) + + for i := 0; i < strct.NumFields(); i++ { + f := strct.Field(i) + + sf = append(sf, &StructField{ + Name: f.Name(), + Exported: f.Exported(), + Optional: HasOptionalTag(strct.Tag(i)), + }) + } + + return sf +} + +func HasOptionalTag(tags string) bool { + return reflect.StructTag(tags).Get(TagName) == OptionalTagValue +} + +// String returns a comma-separated list of field names. +func (sf StructFields) String() (res string) { + for i := 0; i < len(sf); i++ { + if res != "" { + res += ", " + } + + res += sf[i].Name + } + + return res +} + +// SkippedFields returns a list of fields that are not present in the given +// literal, but expected to. +// +//revive:disable-next-line:cyclomatic +func (sf StructFields) SkippedFields(lit *ast.CompositeLit, onlyExported bool) StructFields { + if len(lit.Elts) != 0 && !isNamedLiteral(lit) { + if len(lit.Elts) == len(sf) { + return nil + } + + return sf[len(lit.Elts):] + } + + em := sf.existenceMap() + res := make(StructFields, 0, len(sf)) + + for i := 0; i < len(lit.Elts); i++ { + kv, ok := lit.Elts[i].(*ast.KeyValueExpr) + if !ok { + continue + } + + k, ok := kv.Key.(*ast.Ident) + if !ok { + continue + } + + em[k.Name] = true + } + + for i := 0; i < len(sf); i++ { + if em[sf[i].Name] || (!sf[i].Exported && onlyExported) || sf[i].Optional { + continue + } + + res = append(res, sf[i]) + } + + if len(res) == 0 { + return nil + } + + return res +} + +func (sf StructFields) existenceMap() map[string]bool { + m := make(map[string]bool, len(sf)) + + for i := 0; i < len(sf); i++ { + m[sf[i].Name] = false + } + + return m +} + +// isNamedLiteral returns true if the given literal is unnamed. +// +// The logic is basing on the principle that literal is named or unnamed, +// therefore is literal's first element is a [ast.KeyValueExpr], it is named. +// +// Method will panic if the given literal is empty. +func isNamedLiteral(lit *ast.CompositeLit) bool { + if _, ok := lit.Elts[0].(*ast.KeyValueExpr); !ok { + return false + } + + return true +} diff --git a/internal/fields/struct_test.go b/internal/fields/struct_test.go new file mode 100644 index 00000000..bd9be620 --- /dev/null +++ b/internal/fields/struct_test.go @@ -0,0 +1,160 @@ +package fields_test + +import ( + "go/ast" + "go/types" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + "golang.org/x/tools/go/packages" + + "github.com/GaijinEntertainment/go-exhaustruct/v3/internal/fields" +) + +func Test_HasOptionalTag(t *testing.T) { + t.Parallel() + + assert.True(t, fields.HasOptionalTag(`exhaustruct:"optional"`)) + assert.False(t, fields.HasOptionalTag(`exhaustruct:"required"`)) +} + +func TestStructFields(t *testing.T) { + t.Parallel() + + suite.Run(t, new(StructFieldsSuite)) +} + +type StructFieldsSuite struct { + suite.Suite + + scope *ast.Scope + pkg *packages.Package +} + +func (s *StructFieldsSuite) SetupSuite() { + pkgs, err := packages.Load(&packages.Config{ //nolint:exhaustruct + Mode: packages.NeedTypes | packages.NeedTypesInfo | packages.NeedTypesSizes | packages.NeedSyntax, + Dir: "testdata", + }, "") + s.Require().NoError(err) + s.Require().Len(pkgs, 1) + + s.pkg = pkgs[0] + s.Require().NotNil(s.pkg) + + s.scope = s.pkg.Syntax[0].Scope + s.Require().NotNil(s.scope) +} + +func (s *StructFieldsSuite) getReferenceStructFields() fields.StructFields { + s.T().Helper() + + obj := s.scope.Lookup("testStruct") + s.Require().NotNil(obj) + + typ := s.pkg.TypesInfo.TypeOf(obj.Decl.(*ast.TypeSpec).Type) //nolint:forcetypeassert + s.Require().NotNil(typ) + + return fields.NewStructFields(typ.Underlying().(*types.Struct)) //nolint:forcetypeassert +} + +func (s *StructFieldsSuite) TestNewStructFields() { + sf := s.getReferenceStructFields() + + s.Assert().Len(sf, 4) + s.Assert().Equal(fields.StructFields{ + { + Name: "ExportedRequired", + Exported: true, + Optional: false, + }, + { + Name: "unexportedRequired", + Exported: false, + Optional: false, + }, + { + Name: "ExportedOptional", + Exported: true, + Optional: true, + }, + { + Name: "unexportedOptional", + Exported: false, + Optional: true, + }, + }, sf) +} + +func (s *StructFieldsSuite) TestStructFields_String() { + sf := s.getReferenceStructFields() + + s.Assert().Equal( + "ExportedRequired, unexportedRequired, ExportedOptional, unexportedOptional", + sf.String(), + ) +} + +func (s *StructFieldsSuite) TestStructFields_SkippedFields_Unnamed() { + sf := s.getReferenceStructFields() + + unnamed := s.scope.Lookup("_unnamed") + if s.Assert().NotNil(unnamed) { + lit := unnamed.Decl.(*ast.ValueSpec).Values[0].(*ast.CompositeLit) //nolint:forcetypeassert + if s.Assert().NotNil(lit) { + s.Assert().Nil(sf.SkippedFields(lit, true)) + s.Assert().Nil(sf.SkippedFields(lit, false)) + } + } + + unnamedIncomplete := s.scope.Lookup("_unnamedIncomplete") + if s.Assert().NotNil(unnamedIncomplete) { + lit := unnamedIncomplete.Decl.(*ast.ValueSpec).Values[0].(*ast.CompositeLit) //nolint:forcetypeassert + if s.Assert().NotNil(lit) { + s.Assert().Equal(fields.StructFields{ + {"unexportedRequired", false, false}, + {"ExportedOptional", true, true}, + {"unexportedOptional", false, true}, + }, sf.SkippedFields(lit, true)) + } + } +} + +func (s *StructFieldsSuite) TestStructFields_SkippedFields_Named() { + sf := s.getReferenceStructFields() + + named := s.scope.Lookup("_named") + if s.Assert().NotNil(named) { + lit := named.Decl.(*ast.ValueSpec).Values[0].(*ast.CompositeLit) //nolint:forcetypeassert + if s.Assert().NotNil(lit) { + s.Assert().Nil(sf.SkippedFields(lit, true)) + s.Assert().Nil(sf.SkippedFields(lit, false)) + } + } + + namedIncomplete1 := s.scope.Lookup("_namedIncomplete1") + if s.Assert().NotNil(namedIncomplete1) { + lit := namedIncomplete1.Decl.(*ast.ValueSpec).Values[0].(*ast.CompositeLit) //nolint:forcetypeassert + if s.Assert().NotNil(lit) { + s.Assert().Nil(sf.SkippedFields(lit, true)) + s.Assert().Equal(fields.StructFields{ + {"unexportedRequired", false, false}, + }, sf.SkippedFields(lit, false)) + } + } + + namedIncomplete2 := s.scope.Lookup("_namedIncomplete2") + if s.Assert().NotNil(namedIncomplete2) { + lit := namedIncomplete2.Decl.(*ast.ValueSpec).Values[0].(*ast.CompositeLit) //nolint:forcetypeassert + if s.Assert().NotNil(lit) { + s.Assert().Equal(fields.StructFields{ + {"ExportedRequired", true, false}, + }, sf.SkippedFields(lit, true)) + s.Assert().Equal(fields.StructFields{ + {"ExportedRequired", true, false}, + {"unexportedRequired", false, false}, + }, sf.SkippedFields(lit, false)) + } + } +} diff --git a/internal/fields/testdata/structs.go b/internal/fields/testdata/structs.go new file mode 100644 index 00000000..2768c63a --- /dev/null +++ b/internal/fields/testdata/structs.go @@ -0,0 +1,30 @@ +package testdata + +type testStruct struct { + // some random comment + + ExportedRequired int + unexportedRequired int + + ExportedOptional int `exhaustruct:"optional"` + unexportedOptional int `exhaustruct:"optional"` +} + +var ( + _unnamed = testStruct{1, 2, 3, 4} + _named = testStruct{ + ExportedRequired: 1, + unexportedRequired: 2, + ExportedOptional: 3, + unexportedOptional: 4, + } + _unnamedIncomplete = testStruct{1} + _namedIncomplete1 = testStruct{ + ExportedRequired: 1, + ExportedOptional: 3, + } + _namedIncomplete2 = testStruct{ + ExportedOptional: 3, + unexportedOptional: 4, + } +) diff --git a/internal/pattern/list.go b/internal/pattern/list.go new file mode 100644 index 00000000..a16e5058 --- /dev/null +++ b/internal/pattern/list.go @@ -0,0 +1,82 @@ +package pattern + +import ( + "fmt" + "regexp" + "strings" +) + +var ( + ErrEmptyPattern = fmt.Errorf("pattern can't be empty") + ErrCompilationFailed = fmt.Errorf("pattern compilation failed") +) + +// List is a list of regular expressions. +type List []*regexp.Regexp + +// NewList parses slice of strings to a slice of compiled regular expressions. +func NewList(strs ...string) (List, error) { + if len(strs) == 0 { + return nil, nil + } + + l := make(List, 0, len(strs)) + + for _, str := range strs { + re, err := strToRe(str) + if err != nil { + return nil, err + } + + l = append(l, re) + } + + return l, nil +} + +// MatchFullString matches provided string against all regexps in a slice and returns +// true if any of them matches whole string. +func (l List) MatchFullString(str string) bool { + for i := 0; i < len(l); i++ { + if m := l[i].FindStringSubmatch(str); len(m) > 0 && m[0] == str { + return true + } + } + + return false +} + +func (l *List) Set(value string) error { + re, err := strToRe(value) + if err != nil { + return err + } + + *l = append(*l, re) + + return nil +} + +func (l *List) String() string { + res := make([]string, 0, len(*l)) + + for _, re := range *l { + res = append(res, `"`+re.String()+`"`) + } + + return strings.Join(res, ", ") +} + +// strToRe parses string to a compiled regular expression that matches full string. +func strToRe(str string) (*regexp.Regexp, error) { + if str == "" { + return nil, ErrEmptyPattern + } + + re, err := regexp.Compile(str) + if err != nil { + return nil, fmt.Errorf("%w: %s: %w", ErrCompilationFailed, str, err) + } + + return re, nil +} diff --git a/internal/pattern/list_test.go b/internal/pattern/list_test.go new file mode 100644 index 00000000..df9b1e28 --- /dev/null +++ b/internal/pattern/list_test.go @@ -0,0 +1,68 @@ +package pattern_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/GaijinEntertainment/go-exhaustruct/v3/internal/pattern" +) + +func TestList_MatchFullString(t *testing.T) { + t.Parallel() + + l, err := pattern.NewList() + assert.NoError(t, err) + assert.Nil(t, l) + + l, err = pattern.NewList("a", "b", "c") + require.NoError(t, err) + assert.Len(t, l, 3) + + assert.True(t, l.MatchFullString("a")) + assert.True(t, l.MatchFullString("b")) + assert.True(t, l.MatchFullString("c")) + assert.False(t, l.MatchFullString("d")) + + l, err = pattern.NewList("") + assert.Nil(t, l) + assert.ErrorIs(t, err, pattern.ErrEmptyPattern) + + l, err = pattern.NewList("a", "b", "c[") + assert.Nil(t, l) + assert.ErrorIs(t, err, pattern.ErrCompilationFailed) + + l, err = pattern.NewList("abc") + require.NoError(t, err) + assert.Len(t, l, 1) + + assert.False(t, l.MatchFullString("a")) + assert.False(t, l.MatchFullString("abcdef")) + assert.True(t, l.MatchFullString("abc")) +} + +func TestList_Set(t *testing.T) { + t.Parallel() + + l, err := pattern.NewList("a", "b", "c") + require.NoError(t, err) + + assert.NoError(t, l.Set("d")) + assert.Len(t, l, 4) + + assert.ErrorIs(t, l.Set("e["), pattern.ErrCompilationFailed) + assert.Len(t, l, 4) +} + +func TestList_String(t *testing.T) { + t.Parallel() + + l, err := pattern.NewList("a", "b", "c") + require.NoError(t, err) + assert.Equal(t, `"a", "b", "c"`, l.String()) + + l, err = pattern.NewList() + require.NoError(t, err) + assert.Equal(t, "", l.String()) +} diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go deleted file mode 100644 index 279f2189..00000000 --- a/pkg/analyzer/analyzer.go +++ /dev/null @@ -1,289 +0,0 @@ -package analyzer - -import ( - "errors" - "flag" - "go/ast" - "go/types" - "strings" - "sync" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" - "golang.org/x/tools/go/ast/inspector" -) - -var ( - ErrEmptyPattern = errors.New("pattern can't be empty") -) - -type analyzer struct { - include PatternsList - exclude PatternsList - - typesProcessCache map[types.Type]bool - typesProcessCacheMu sync.RWMutex - - structFieldsCache map[types.Type]*StructFields - structFieldsCacheMu sync.RWMutex -} - -// NewAnalyzer returns a go/analysis-compatible analyzer. -// -i arguments adds include patterns -// -e arguments adds exclude patterns -func NewAnalyzer(include []string, exclude []string) (*analysis.Analyzer, error) { - a := analyzer{ //nolint:exhaustruct - typesProcessCache: map[types.Type]bool{}, - - structFieldsCache: map[types.Type]*StructFields{}, - } - - var err error - - a.include, err = newPatternsList(include) - if err != nil { - return nil, err - } - - a.exclude, err = newPatternsList(exclude) - if err != nil { - return nil, err - } - - return &analysis.Analyzer{ //nolint:exhaustruct - Name: "exhaustruct", - Doc: "Checks if all structure fields are initialized", - Run: a.run, - Requires: []*analysis.Analyzer{inspect.Analyzer}, - Flags: a.newFlagSet(), - }, nil -} - -func (a *analyzer) newFlagSet() flag.FlagSet { - fs := flag.NewFlagSet("exhaustruct flags", flag.PanicOnError) - - fs.Var( - &reListVar{values: &a.include}, - "i", - "Regular expression to match struct packages and names, can receive multiple flags", - ) - fs.Var( - &reListVar{values: &a.exclude}, - "e", - "Regular expression to exclude struct packages and names, can receive multiple flags", - ) - - return *fs -} - -func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) { - insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) //nolint:forcetypeassert - - nodeTypes := []ast.Node{ - (*ast.CompositeLit)(nil), - (*ast.ReturnStmt)(nil), - } - - insp.Preorder(nodeTypes, a.newVisitor(pass)) - - return nil, nil //nolint:nilnil -} - -//nolint:cyclop -func (a *analyzer) newVisitor(pass *analysis.Pass) func(node ast.Node) { - var ret *ast.ReturnStmt - - return func(node ast.Node) { - if retLit, ok := node.(*ast.ReturnStmt); ok { - // save return statement for future (to detect error-containing returns) - ret = retLit - - return - } - - lit, _ := node.(*ast.CompositeLit) - if lit.Type == nil { - // we're not interested in non-typed literals - return - } - - typ := pass.TypesInfo.TypeOf(lit.Type) - if typ == nil { - return - } - - strct, ok := typ.Underlying().(*types.Struct) - if !ok { - // we also not interested in non-structure literals - return - } - - strctName := exprName(lit.Type) - if strctName == "" { - return - } - - if !a.shouldProcessType(typ) { - return - } - - if len(lit.Elts) == 0 && ret != nil { - if ret.End() < lit.Pos() { - // we're outside last return statement - ret = nil - } else if returnContainsLiteral(ret, lit) && returnContainsError(ret, pass) { - // we're okay with empty literals in return statements with non-nil errors, like - // `return my.Struct{}, fmt.Errorf("non-nil error!")` - return - } - } - - missingFields := a.structMissingFields(lit, strct, strings.HasPrefix(typ.String(), pass.Pkg.Path()+".")) - - if len(missingFields) == 1 { - pass.Reportf(node.Pos(), "%s is missing in %s", missingFields[0], strctName) - } else if len(missingFields) > 1 { - pass.Reportf(node.Pos(), "%s are missing in %s", strings.Join(missingFields, ", "), strctName) - } - } -} - -func (a *analyzer) shouldProcessType(typ types.Type) bool { - if len(a.include) == 0 && len(a.exclude) == 0 { - // skip whole part with cache, since we have no restrictions and have to check everything - return true - } - - a.typesProcessCacheMu.RLock() - v, ok := a.typesProcessCache[typ] - a.typesProcessCacheMu.RUnlock() - - if !ok { - a.typesProcessCacheMu.Lock() - defer a.typesProcessCacheMu.Unlock() - - v = true - typStr := typ.String() - - if len(a.include) > 0 && !a.include.MatchesAny(typStr) { - v = false - } - - if v && a.exclude.MatchesAny(typStr) { - v = false - } - - a.typesProcessCache[typ] = v - } - - return v -} - -func (a *analyzer) structMissingFields(lit *ast.CompositeLit, strct *types.Struct, private bool) []string { - keys, unnamed := literalKeys(lit) - fields := a.structFields(strct) - - if unnamed { - if private { - return fields.All[len(keys):] - } - - return fields.Public[len(keys):] - } - - if private { - return difference(fields.AllRequired, keys) - } - - return difference(fields.PublicRequired, keys) -} - -func (a *analyzer) structFields(strct *types.Struct) *StructFields { - typ := strct.Underlying() - - a.structFieldsCacheMu.RLock() - fields, ok := a.structFieldsCache[typ] - a.structFieldsCacheMu.RUnlock() - - if !ok { - a.structFieldsCacheMu.Lock() - defer a.structFieldsCacheMu.Unlock() - - fields = NewStructFields(strct) - a.structFieldsCache[typ] = fields - } - - return fields -} - -func returnContainsLiteral(ret *ast.ReturnStmt, lit *ast.CompositeLit) bool { - for _, result := range ret.Results { - if l, ok := result.(*ast.CompositeLit); ok { - if lit == l { - return true - } - } - } - - return false -} - -func returnContainsError(ret *ast.ReturnStmt, pass *analysis.Pass) bool { - for _, result := range ret.Results { - if pass.TypesInfo.TypeOf(result).String() == "error" { - return true - } - } - - return false -} - -func literalKeys(lit *ast.CompositeLit) (keys []string, unnamed bool) { - for _, elt := range lit.Elts { - if k, ok := elt.(*ast.KeyValueExpr); ok { - if ident, ok := k.Key.(*ast.Ident); ok { - keys = append(keys, ident.Name) - } - - continue - } - - // in case we deal with unnamed initialization - no need to iterate over all - // elements - simply create slice with proper size - unnamed = true - keys = make([]string, len(lit.Elts)) - - return - } - - return -} - -// difference returns elements that are in `a` and not in `b`. -func difference(a, b []string) (diff []string) { - mb := make(map[string]struct{}, len(b)) - for _, x := range b { - mb[x] = struct{}{} - } - - for _, x := range a { - if _, found := mb[x]; !found { - diff = append(diff, x) - } - } - - return diff -} - -func exprName(expr ast.Expr) string { - if i, ok := expr.(*ast.Ident); ok { - return i.Name - } - - s, ok := expr.(*ast.SelectorExpr) - if !ok { - return "" - } - - return s.Sel.Name -} diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go deleted file mode 100644 index 129e1397..00000000 --- a/pkg/analyzer/analyzer_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package analyzer_test - -import ( - "os" - "path/filepath" - "testing" - - "golang.org/x/tools/go/analysis/analysistest" - - "github.com/GaijinEntertainment/go-exhaustruct/v2/pkg/analyzer" -) - -func TestAll(t *testing.T) { - t.Parallel() - - wd, err := os.Getwd() - if err != nil { - t.Fatalf("Failed to get wd: %s", err) - } - - testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata") - - a, err := analyzer.NewAnalyzer( - []string{".*\\.Test", ".*\\.Test2", ".*\\.Embedded", ".*\\.External"}, - []string{".*Excluded$"}, - ) - if err != nil { - t.Error(err) - } - - analysistest.Run(t, testdata, a, "s") -} - -func BenchmarkAll(b *testing.B) { - wd, err := os.Getwd() - if err != nil { - b.Fatalf("Failed to get wd: %s", err) - } - - testdata := filepath.Join(filepath.Dir(filepath.Dir(wd)), "testdata") - - a, err := analyzer.NewAnalyzer( - []string{".*\\.Test", ".*\\.Test2", ".*\\.Embedded", ".*\\.External"}, - []string{".*Excluded$"}, - ) - if err != nil { - b.Error(err) - } - - for i := 0; i < b.N; i++ { - analysistest.Run(b, testdata, a, "s") - } -} diff --git a/pkg/analyzer/patterns-list.go b/pkg/analyzer/patterns-list.go deleted file mode 100644 index 2884cab6..00000000 --- a/pkg/analyzer/patterns-list.go +++ /dev/null @@ -1,68 +0,0 @@ -package analyzer - -import ( - "fmt" - "regexp" -) - -type PatternsList []*regexp.Regexp - -// MatchesAny matches provided string against all regexps in a slice. -func (l PatternsList) MatchesAny(str string) bool { - for _, r := range l { - if r.MatchString(str) { - return true - } - } - - return false -} - -// newPatternsList parses slice of strings to a slice of compiled regular -// expressions. -func newPatternsList(in []string) (PatternsList, error) { - list := PatternsList{} - - for _, str := range in { - re, err := strToRegexp(str) - if err != nil { - return nil, err - } - - list = append(list, re) - } - - return list, nil -} - -type reListVar struct { - values *PatternsList -} - -func (v *reListVar) Set(value string) error { - re, err := strToRegexp(value) - if err != nil { - return err - } - - *v.values = append(*v.values, re) - - return nil -} - -func (v *reListVar) String() string { - return "" -} - -func strToRegexp(str string) (*regexp.Regexp, error) { - if str == "" { - return nil, ErrEmptyPattern - } - - re, err := regexp.Compile(str) - if err != nil { - return nil, fmt.Errorf("unable to compile %s as regular expression: %w", str, err) - } - - return re, nil -} diff --git a/pkg/analyzer/struct-fields.go b/pkg/analyzer/struct-fields.go deleted file mode 100644 index 56eda05f..00000000 --- a/pkg/analyzer/struct-fields.go +++ /dev/null @@ -1,61 +0,0 @@ -package analyzer - -import ( - "go/types" - "reflect" - - "golang.org/x/exp/slices" -) - -type StructFields struct { - All []string - AllRequired []string - - Public []string - PublicRequired []string -} - -func NewStructFields(strct *types.Struct) *StructFields { - sf := StructFields{ - All: make([]string, strct.NumFields()), - AllRequired: make([]string, 0, strct.NumFields()), - Public: make([]string, 0, strct.NumFields()), - PublicRequired: make([]string, 0, strct.NumFields()), - } - - for i := 0; i < strct.NumFields(); i++ { - f := strct.Field(i) - isOptional := isFieldOptional(strct.Tag(i)) - - sf.All[i] = f.Name() - if !isOptional { - sf.AllRequired = append(sf.AllRequired, f.Name()) - } - - if f.Exported() { - sf.Public = append(sf.Public, f.Name()) - - if !isOptional { - sf.PublicRequired = append(sf.PublicRequired, f.Name()) - } - } - } - - sf.All = slices.Clip(sf.All) - sf.AllRequired = slices.Clip(sf.AllRequired) - sf.Public = slices.Clip(sf.Public) - sf.PublicRequired = slices.Clip(sf.PublicRequired) - - return &sf -} - -const ( - TagName = "exhaustruct" - OptionalTagValue = "optional" -) - -// isFieldOptional checks if field tags has an optional tag, and therefore can -// be omitted during structure initialization. -func isFieldOptional(tags string) bool { - return reflect.StructTag(tags).Get(TagName) == OptionalTagValue -} diff --git a/testdata/src/s/s.go b/testdata/src/s/s.go deleted file mode 100644 index 55d387f6..00000000 --- a/testdata/src/s/s.go +++ /dev/null @@ -1,146 +0,0 @@ -package s - -import ( - "fmt" - - "e" -) - -type Embedded struct { - E string - F string - g string - H string -} - -type Test struct { - A string - B int - C float32 - D bool - E string `exhaustruct:"optional"` -} - -type Test2 struct { - Embedded - External e.External -} - -func shouldPass() Test { - return Test{ - A: "a", - B: 1, - C: 0.0, - D: false, - } -} - -func shouldPassPrivateLocalTypeCorrect1() { - type myTpe struct { - a string - b string - } - - _ = myTpe{"", ""} -} - -func shouldPassPrivateLocalTypeCorrect2() { - type myTpe struct { - a string - b string - c string - } - - _ = myTpe{"", "", ""} -} - -func shouldPass2() Test2 { - return Test2{ - External: e.External{ - A: "", - B: "", - }, - Embedded: Embedded{ - E: "", - F: "", - H: "", - g: "", - }, - } -} - -func shouldPassWithReturn() (Test, error) { - if true { - // Empty structs in return statements are ignored if also returning an error - return Test{}, fmt.Errorf("error") - } - - _ = Test{} // want "A, B, C, D are missing in Test" - - return Test{}, fmt.Errorf("error") -} -func shouldPass3() { - // Checking to make sure state from tracking the previous return statement doesn't affect this struct - _ = Test{} // want "A, B, C, D are missing in Test" -} - -func shouldPassWithoutNames() Test { - return Test{"", 0, 0, false, ""} -} - -func shouldFailWithReturn() (Test, error) { - // Empty structs in return statements are not ignored if returning nil error - return Test{}, nil // want "A, B, C, D are missing in Test" -} - -func shouldFailWithMissingFields() Test { - return Test{ // want "C is missing in Test" - A: "a", - B: 1, - D: false, - } -} - -// Unchecked is a struct not listed in StructPatternList -type Unchecked struct { - A int - B int -} - -func unchecked() { - // This struct is not listed in StructPatternList so the linter won't complain that it's not filled out - _ = Unchecked{ - A: 1, - } -} - -func excluded() { - // this struct is excluded therefore should not be linted - _ = e.ExternalExcluded{} -} - -func shouldFailOnEmbedded() Test2 { - return Test2{ - Embedded: Embedded{ // want "E, g, H are missing in Embedded" - F: "", - }, - External: e.External{ - A: "", - B: "", - }, - } -} - -func shoildFailOnExternal() Test2 { - return Test2{ - External: e.External{ // want "A is missing in External" - B: "", - }, - Embedded: Embedded{ - E: "", - F: "", - H: "", - g: "", - }, - } -}