From 2bc4eaa8ae5e4c8dfbae94af7c39fa55d94b9070 Mon Sep 17 00:00:00 2001 From: golangci Date: Sun, 6 May 2018 21:08:53 +0300 Subject: [PATCH] support maligned --- Gopkg.lock | 8 +- internal/commands/run.go | 2 + pkg/config/config.go | 3 + pkg/enabled_linters.go | 3 +- pkg/golinters/maligned.go | 34 +++ pkg/golinters/structcheck.go | 2 +- pkg/golinters/testdata/maligned.go | 7 + pkg/golinters/testdata/structcheck.go | 2 +- pkg/golinters/utils.go | 8 + vendor/github.com/golangci/maligned/LICENSE | 27 ++ vendor/github.com/golangci/maligned/README | 7 + .../github.com/golangci/maligned/maligned.go | 253 ++++++++++++++++++ 12 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 pkg/golinters/maligned.go create mode 100644 pkg/golinters/testdata/maligned.go create mode 100644 vendor/github.com/golangci/maligned/LICENSE create mode 100644 vendor/github.com/golangci/maligned/README create mode 100644 vendor/github.com/golangci/maligned/maligned.go diff --git a/Gopkg.lock b/Gopkg.lock index a26bbe0b..0786da58 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -86,6 +86,12 @@ ] revision = "31bebf17d90d40b728c41fecdd5acf5fb8654fc7" +[[projects]] + branch = "master" + name = "github.com/golangci/maligned" + packages = ["."] + revision = "b1d89398deca2fd3f8578e5a9551e819bd01ca5f" + [[projects]] name = "github.com/inconshreveable/mousetrap" packages = ["."] @@ -210,6 +216,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "ccff501e3f2717b30a1883f46bda277e4438080025f03b8070c968b7ee99549a" + inputs-digest = "41c8a896b25eff9ad6bdcac8176d363561481e3d985dc37952af54185fa57759" solver-name = "gps-cdcl" solver-version = 1 diff --git a/internal/commands/run.go b/internal/commands/run.go index 807ad807..79cddcbb 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -50,6 +50,8 @@ func (e *Executor) initRun() { runCmd.Flags().BoolVar(&rc.Structcheck.CheckExportedFields, "structcheck.exported-fields", false, "Structcheck: report about unused exported struct fields") runCmd.Flags().BoolVar(&rc.Varcheck.CheckExportedFields, "varcheck.exported-fields", false, "Varcheck: report about unused exported variables") + runCmd.Flags().BoolVar(&rc.Maligned.SuggestNewOrder, "maligned.suggest-new", false, "Maligned: print suggested more optimal struct fields ordering") + runCmd.Flags().StringSliceVarP(&rc.EnabledLinters, "enable", "E", []string{}, "Enable specific linter") runCmd.Flags().StringSliceVarP(&rc.DisabledLinters, "disable", "D", []string{}, "Disable specific linter") runCmd.Flags().BoolVar(&rc.EnableAllLinters, "enable-all", false, "Enable all linters") diff --git a/pkg/config/config.go b/pkg/config/config.go index 3780ce2a..517e8d8e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -53,6 +53,9 @@ type Run struct { Structcheck struct { CheckExportedFields bool } + Maligned struct { + SuggestNewOrder bool + } EnabledLinters []string DisabledLinters []string diff --git a/pkg/enabled_linters.go b/pkg/enabled_linters.go index 27205d11..81366d3e 100644 --- a/pkg/enabled_linters.go +++ b/pkg/enabled_linters.go @@ -11,9 +11,9 @@ import ( ) type LinterConfig struct { - EnabledByDefault bool Desc string Linter Linter + EnabledByDefault bool DoesFullImport bool } @@ -66,6 +66,7 @@ func GetAllSupportedLinterConfigs() []LinterConfig { disabledByDefault(golinters.Gofmt{}, "Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification", false), disabledByDefault(golinters.Gofmt{UseGoimports: true}, "Goimports does everything that gofmt does. Additionally it checks unused imports", false), + disabledByDefault(golinters.Maligned{}, "Tool to detect Go structs that would take less memory if their fields were sorted", true), } } diff --git a/pkg/golinters/maligned.go b/pkg/golinters/maligned.go new file mode 100644 index 00000000..9dbc1b24 --- /dev/null +++ b/pkg/golinters/maligned.go @@ -0,0 +1,34 @@ +package golinters + +import ( + "context" + "fmt" + + "github.com/golangci/golangci-lint/pkg/result" + malignedAPI "github.com/golangci/maligned" +) + +type Maligned struct{} + +func (Maligned) Name() string { + return "maligned" +} + +func (m Maligned) Run(ctx context.Context, lintCtx *Context) (*result.Result, error) { + issues := malignedAPI.Run(lintCtx.Program) + + res := &result.Result{} + for _, i := range issues { + text := fmt.Sprintf("struct of size %d bytes could be of size %d bytes", i.OldSize, i.NewSize) + if lintCtx.RunCfg().Maligned.SuggestNewOrder { + text += fmt.Sprintf(":\n%s", formatCodeBlock(i.NewStructDef, lintCtx.RunCfg())) + } + res.Issues = append(res.Issues, result.Issue{ + File: i.Pos.Filename, + LineNumber: i.Pos.Line, + Text: text, + FromLinter: m.Name(), + }) + } + return res, nil +} diff --git a/pkg/golinters/structcheck.go b/pkg/golinters/structcheck.go index aed231ca..6a707087 100644 --- a/pkg/golinters/structcheck.go +++ b/pkg/golinters/structcheck.go @@ -22,7 +22,7 @@ func (s Structcheck) Run(ctx context.Context, lintCtx *Context) (*result.Result, res.Issues = append(res.Issues, result.Issue{ File: i.Pos.Filename, LineNumber: i.Pos.Line, - Text: fmt.Sprintf("%s is unused", formatCode(fmt.Sprintf("%s.%s", i.Type, i.FieldName), lintCtx.RunCfg())), + Text: fmt.Sprintf("%s is unused", formatCode(i.FieldName, lintCtx.RunCfg())), FromLinter: s.Name(), }) } diff --git a/pkg/golinters/testdata/maligned.go b/pkg/golinters/testdata/maligned.go new file mode 100644 index 00000000..d77e706a --- /dev/null +++ b/pkg/golinters/testdata/maligned.go @@ -0,0 +1,7 @@ +package testdata + +type BadAlignedStruct struct { // ERROR "struct of size 24 bytes could be of size 16 bytes" + B bool + I int + B2 bool +} diff --git a/pkg/golinters/testdata/structcheck.go b/pkg/golinters/testdata/structcheck.go index 643c7a89..e0162c83 100644 --- a/pkg/golinters/testdata/structcheck.go +++ b/pkg/golinters/testdata/structcheck.go @@ -1,5 +1,5 @@ package testdata type t struct { // ERROR "`t` is unused" - unusedField int // ERROR "`testdata.t.unusedField` is unused" + unusedField int // ERROR "`unusedField` is unused" } diff --git a/pkg/golinters/utils.go b/pkg/golinters/utils.go index e1c744f1..ce90f3b0 100644 --- a/pkg/golinters/utils.go +++ b/pkg/golinters/utils.go @@ -14,3 +14,11 @@ func formatCode(code string, cfg *config.Run) string { return fmt.Sprintf("`%s`", code) } + +func formatCodeBlock(code string, cfg *config.Run) string { + if strings.Contains(code, "`") { + return code // TODO: properly escape or remove + } + + return fmt.Sprintf("```\n%s\n```", code) +} diff --git a/vendor/github.com/golangci/maligned/LICENSE b/vendor/github.com/golangci/maligned/LICENSE new file mode 100644 index 00000000..74487567 --- /dev/null +++ b/vendor/github.com/golangci/maligned/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2012 The Go Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/golangci/maligned/README b/vendor/github.com/golangci/maligned/README new file mode 100644 index 00000000..4e57f6ea --- /dev/null +++ b/vendor/github.com/golangci/maligned/README @@ -0,0 +1,7 @@ +Install: + + go get github.com/mdempsky/maligned + +Usage: + + maligned cmd/compile/internal/gc cmd/link/internal/ld diff --git a/vendor/github.com/golangci/maligned/maligned.go b/vendor/github.com/golangci/maligned/maligned.go new file mode 100644 index 00000000..c2492b2f --- /dev/null +++ b/vendor/github.com/golangci/maligned/maligned.go @@ -0,0 +1,253 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package maligned + +import ( + "fmt" + "go/ast" + "go/build" + "go/token" + "go/types" + "sort" + "strings" + + "golang.org/x/tools/go/loader" +) + +var fset = token.NewFileSet() + +type Issue struct { + OldSize, NewSize int + NewStructDef string + Pos token.Position +} + +func Run(prog *loader.Program) []Issue { + flagVerbose := true + fset = prog.Fset + + var issues []Issue + + for _, pkg := range prog.InitialPackages() { + for _, file := range pkg.Files { + ast.Inspect(file, func(node ast.Node) bool { + if s, ok := node.(*ast.StructType); ok { + i := malign(node.Pos(), pkg.Types[s].Type.(*types.Struct), flagVerbose) + if i != nil { + issues = append(issues, *i) + } + } + return true + }) + } + } + + return issues +} + +func malign(pos token.Pos, str *types.Struct, verbose bool) *Issue { + wordSize := int64(8) + maxAlign := int64(8) + switch build.Default.GOARCH { + case "386", "arm": + wordSize, maxAlign = 4, 4 + case "amd64p32": + wordSize = 4 + } + + s := gcSizes{wordSize, maxAlign} + sz := s.Sizeof(str) + opt, fields := optimalSize(str, &s, verbose) + if sz == opt { + return nil + } + + newStructDefParts := []string{"struct{"} + + var w int + for _, f := range fields { + if n := len(f.Name()); n > w { + w = n + } + } + spaces := strings.Repeat(" ", w) + for _, f := range fields { + line := fmt.Sprintf("\t%s%s\t%s,", f.Name(), spaces[len(f.Name()):], f.Type().String()) + newStructDefParts = append(newStructDefParts, line) + } + newStructDefParts = append(newStructDefParts, "}") + + return &Issue{ + OldSize: int(sz), + NewSize: int(opt), + NewStructDef: strings.Join(newStructDefParts, "\n"), + Pos: fset.Position(pos), + } +} + +func optimalSize(str *types.Struct, sizes *gcSizes, stable bool) (int64, []*types.Var) { + nf := str.NumFields() + fields := make([]*types.Var, nf) + alignofs := make([]int64, nf) + sizeofs := make([]int64, nf) + for i := 0; i < nf; i++ { + fields[i] = str.Field(i) + ft := fields[i].Type() + alignofs[i] = sizes.Alignof(ft) + sizeofs[i] = sizes.Sizeof(ft) + } + if stable { // Stable keeps as much of the order as possible, but slower + sort.Stable(&byAlignAndSize{fields, alignofs, sizeofs}) + } else { + sort.Sort(&byAlignAndSize{fields, alignofs, sizeofs}) + } + return sizes.Sizeof(types.NewStruct(fields, nil)), fields +} + +type byAlignAndSize struct { + fields []*types.Var + alignofs []int64 + sizeofs []int64 +} + +func (s *byAlignAndSize) Len() int { return len(s.fields) } +func (s *byAlignAndSize) Swap(i, j int) { + s.fields[i], s.fields[j] = s.fields[j], s.fields[i] + s.alignofs[i], s.alignofs[j] = s.alignofs[j], s.alignofs[i] + s.sizeofs[i], s.sizeofs[j] = s.sizeofs[j], s.sizeofs[i] +} + +func (s *byAlignAndSize) Less(i, j int) bool { + // Place zero sized objects before non-zero sized objects. + if s.sizeofs[i] == 0 && s.sizeofs[j] != 0 { + return true + } + if s.sizeofs[j] == 0 && s.sizeofs[i] != 0 { + return false + } + + // Next, place more tightly aligned objects before less tightly aligned objects. + if s.alignofs[i] != s.alignofs[j] { + return s.alignofs[i] > s.alignofs[j] + } + + // Lastly, order by size. + if s.sizeofs[i] != s.sizeofs[j] { + return s.sizeofs[i] > s.sizeofs[j] + } + + return false +} + +// Code below based on go/types.StdSizes. + +type gcSizes struct { + WordSize int64 + MaxAlign int64 +} + +func (s *gcSizes) Alignof(T types.Type) int64 { + // NOTE: On amd64, complex64 is 8 byte aligned, + // even though float32 is only 4 byte aligned. + + // For arrays and structs, alignment is defined in terms + // of alignment of the elements and fields, respectively. + switch t := T.Underlying().(type) { + case *types.Array: + // spec: "For a variable x of array type: unsafe.Alignof(x) + // is the same as unsafe.Alignof(x[0]), but at least 1." + return s.Alignof(t.Elem()) + case *types.Struct: + // spec: "For a variable x of struct type: unsafe.Alignof(x) + // is the largest of the values unsafe.Alignof(x.f) for each + // field f of x, but at least 1." + max := int64(1) + for i, nf := 0, t.NumFields(); i < nf; i++ { + if a := s.Alignof(t.Field(i).Type()); a > max { + max = a + } + } + return max + } + a := s.Sizeof(T) // may be 0 + // spec: "For a variable x of any type: unsafe.Alignof(x) is at least 1." + if a < 1 { + return 1 + } + if a > s.MaxAlign { + return s.MaxAlign + } + return a +} + +var basicSizes = [...]byte{ + types.Bool: 1, + types.Int8: 1, + types.Int16: 2, + types.Int32: 4, + types.Int64: 8, + types.Uint8: 1, + types.Uint16: 2, + types.Uint32: 4, + types.Uint64: 8, + types.Float32: 4, + types.Float64: 8, + types.Complex64: 8, + types.Complex128: 16, +} + +func (s *gcSizes) Sizeof(T types.Type) int64 { + switch t := T.Underlying().(type) { + case *types.Basic: + k := t.Kind() + if int(k) < len(basicSizes) { + if s := basicSizes[k]; s > 0 { + return int64(s) + } + } + if k == types.String { + return s.WordSize * 2 + } + case *types.Array: + n := t.Len() + if n == 0 { + return 0 + } + a := s.Alignof(t.Elem()) + z := s.Sizeof(t.Elem()) + return align(z, a)*(n-1) + z + case *types.Slice: + return s.WordSize * 3 + case *types.Struct: + nf := t.NumFields() + if nf == 0 { + return 0 + } + + var o int64 + max := int64(1) + for i := 0; i < nf; i++ { + ft := t.Field(i).Type() + a, sz := s.Alignof(ft), s.Sizeof(ft) + if a > max { + max = a + } + if i == nf-1 && sz == 0 && o != 0 { + sz = 1 + } + o = align(o, a) + sz + } + return align(o, max) + case *types.Interface: + return s.WordSize * 2 + } + return s.WordSize // catch-all +} + +// align returns the smallest y >= x such that y % a == 0. +func align(x, a int64) int64 { + y := x + a - 1 + return y - y%a +}