Add "-apply" flags for applying suggested fix

By using github.com/dave/dst to decorate original ast.File

Fixes #37

Author: Cuong Manh Le

README.md

@@ -124,6 +124,10 @@ struct {
 }
 ```
 
+**Note**
+
+For applying suggested fix, use `-apply` flag, instead of `-fix`.
+
 ## Development
 
 Go 1.15+

go.mod

@@ -2,4 +2,8 @@ module github.com/orijtech/structslop
 
 go 1.15
 
-require golang.org/x/tools v0.0.0-20200917221617-d56e4e40bc9d
+require (
+	github.com/dave/dst v0.26.2
+	github.com/stretchr/testify v1.6.1 // indirect
+	golang.org/x/tools v0.0.0-20200917221617-d56e4e40bc9d
+)

go.sum

@@ -1,22 +1,59 @@
+github.com/dave/dst v0.26.2 h1:lnxLAKI3tx7MgLNVDirFCsDTlTG9nKTk7GcptKcWSwY=
+github.com/dave/dst v0.26.2/go.mod h1:UMDJuIRPfyUCC78eFuB+SV/WI8oDeyFDvM/JR6NI3IU=
+github.com/dave/gopackages v0.0.0-20170318123100-46e7023ec56e/go.mod h1:i00+b/gKdIDIxuLDFob7ustLAVqhsZRk2qVZrArELGQ=
+github.com/dave/jennifer v1.2.0/go.mod h1:fIb+770HOpJ2fmN9EPPKOqm1vMGhB+TwXKMZhrIygKg=
+github.com/dave/kerr v0.0.0-20170318121727-bc25dd6abe8e/go.mod h1:qZqlPyPvfsDJt+3wHJ1EvSXDuVjFTK0j2p/ca+gtsb8=
+github.com/dave/rebecca v0.9.1/go.mod h1:N6XYdMD/OKw3lkF3ywh8Z6wPGuwNFDNtWYEMFWEmXBA=
+github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/google/pprof v0.0.0-20181127221834-b4f47329b966/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
+github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
+github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
+github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
+github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
+github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
+github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+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/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
+github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
+github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
+golang.org/x/arch v0.0.0-20180920145803-b19384d3c130/go.mod h1:cYlCBUl1MsqxdiKgmc4uh7TxZfWSFLOGSRR090WDxt8=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
+golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20180903190138-2b024373dcd9/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200917221617-d56e4e40bc9d h1:y39d97JVttj+rkTXITl1nf9Vsk+VoRuNzIDLFldUSB4=
 golang.org/x/tools v0.0.0-20200917221617-d56e4e40bc9d/go.mod h1:z6u4i615ZeAfBE4XtMziQW1fSVJXACjjbWkB/mvPzlU=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
+gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/src-d/go-billy.v4 v4.3.0 h1:KtlZ4c1OWbIs4jCv5ZXrTqG8EQocr0g/d4DjNg70aek=
+gopkg.in/src-d/go-billy.v4 v4.3.0/go.mod h1:tm33zBoOwxjYHZIE+OV8bxTWFMJLrconzFMd38aARFk=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

structslop.go

@@ -23,9 +23,13 @@ import (
 	"go/parser"
 	"go/token"
 	"go/types"
+	"io/ioutil"
+	"os"
 	"sort"
 	"strings"
 
+	"github.com/dave/dst"
+	"github.com/dave/dst/decorator"
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
 	"golang.org/x/tools/go/ast/inspector"
@@ -34,11 +38,13 @@ import (
 var (
 	includeTestFiles bool
 	verbose          bool
+	apply            bool
 )
 
 func init() {
 	Analyzer.Flags.BoolVar(&includeTestFiles, "include-test-files", includeTestFiles, "also check test files")
 	Analyzer.Flags.BoolVar(&verbose, "verbose", verbose, "print all information, even when struct is not sloppy")
+	Analyzer.Flags.BoolVar(&apply, "apply", apply, "apply suggested fixes (using -fix won't work)")
 }
 
 const Doc = `check for structs that can be rearrange fields to provide for maximum space/allocation efficiency`
@@ -60,15 +66,26 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		maxAlign: pass.TypesSizes.Alignof(types.Typ[types.UnsafePointer]),
 	}
 
+	dec := decorator.NewDecorator(pass.Fset)
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-
 	nodeFilter := []ast.Node{
+		(*ast.File)(nil),
 		(*ast.StructType)(nil),
 	}
+
+	fileDiags := make(map[string][]byte)
+	var af *ast.File
+	var df *dst.File
+
 	inspect.Preorder(nodeFilter, func(n ast.Node) {
 		if strings.HasSuffix(pass.Fset.File(n.Pos()).Name(), "_test.go") && !includeTestFiles {
 			return
 		}
+		if f, ok := n.(*ast.File); ok {
+			af = f
+			df, _ = dec.DecorateFile(af)
+			return
+		}
 		atyp := n.(*ast.StructType)
 		styp, ok := pass.TypesInfo.Types[atyp].Type.(*types.Struct)
 		// Type information may be incomplete.
@@ -119,13 +136,61 @@ func run(pass *analysis.Pass) (interface{}, error) {
 			}
 		}
 
+		dtyp := dec.Dst.Nodes[atyp].(*dst.StructType)
+		fields := make([]*dst.Field, 0, len(r.optIdx))
+		dummy := &dst.Field{}
+		for _, f := range dtyp.Fields.List {
+			fields = append(fields, f)
+			if len(f.Names) == 0 {
+				continue
+			}
+			for range f.Names[1:] {
+				fields = append(fields, dummy)
+			}
+		}
+		optFields := make([]*dst.Field, 0, len(r.optIdx))
+		for _, i := range r.optIdx {
+			f := fields[i]
+			if f == dummy {
+				continue
+			}
+			optFields = append(optFields, f)
+		}
+		dtyp.Fields.List = optFields
+
+		var suggested bytes.Buffer
+		if err := decorator.Fprint(&suggested, df); err != nil {
+			return
+		}
 		pass.Report(analysis.Diagnostic{
 			Pos:            n.Pos(),
 			End:            n.End(),
 			Message:        msg,
 			SuggestedFixes: nil,
 		})
+		f := pass.Fset.File(n.Pos())
+		fileDiags[f.Name()] = suggested.Bytes()
 	})
+
+	if !apply {
+		return nil, nil
+	}
+	for fn, content := range fileDiags {
+		fi, err := os.Open(fn)
+		if err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "failed to open file: %v", err)
+		}
+		st, err := fi.Stat()
+		if err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "failed to get file stat: %v", err)
+		}
+		if err := fi.Close(); err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "failed to close file: %v", err)
+		}
+		if err := ioutil.WriteFile(fn, content, st.Mode()); err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "failed to write suggested fix to file: %v", err)
+		}
+	}
 	return nil, nil
 }
 

structslop_test.go

@@ -15,6 +15,11 @@
 package structslop_test
 
 import (
+	"bytes"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"golang.org/x/tools/go/analysis/analysistest"
@@ -27,14 +32,45 @@ func Test(t *testing.T) {
 	analysistest.Run(t, testdata, structslop.Analyzer, "struct")
 }
 
+func TestApply(t *testing.T) {
+	dir := strings.Join([]string{".", "testdata", "src"}, string(os.PathSeparator))
+	tmpdir, err := ioutil.TempDir(dir, "structslop-test-apply-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+	fn := filepath.Join(tmpdir, "p.go")
+	src, _ := ioutil.ReadFile(filepath.Join(".", "testdata", "src", "struct", "p.go"))
+	if err := ioutil.WriteFile(fn, src, 0644); err != nil {
+		t.Fatal(err)
+	}
+	testdata := analysistest.TestData()
+	_ = structslop.Analyzer.Flags.Set("apply", "true")
+	defer func() {
+		_ = structslop.Analyzer.Flags.Set("apply", "false")
+	}()
+	analysistest.Run(t, testdata, structslop.Analyzer, filepath.Base(tmpdir))
+	got, _ := ioutil.ReadFile(fn)
+	expected, _ := ioutil.ReadFile(filepath.Join(".", "testdata", "src", "struct", "p.go.golden"))
+	if !bytes.Equal(expected, got) {
+		t.Errorf("unexpected suggested fix, want:\n%s\ngot:\n%s\n", string(expected), string(got))
+	}
+}
+
 func TestIncludeTestFiles(t *testing.T) {
 	testdata := analysistest.TestData()
 	_ = structslop.Analyzer.Flags.Set("include-test-files", "true")
+	defer func() {
+		_ = structslop.Analyzer.Flags.Set("include-test-files", "false")
+	}()
 	analysistest.Run(t, testdata, structslop.Analyzer, "include-test-files")
 }
 
 func TestVerboseMode(t *testing.T) {
 	testdata := analysistest.TestData()
 	_ = structslop.Analyzer.Flags.Set("verbose", "true")
+	defer func() {
+		_ = structslop.Analyzer.Flags.Set("verbose", "false")
+	}()
 	analysistest.Run(t, testdata, structslop.Analyzer, "verbose")
 }

testdata/src/struct/p.go

@@ -79,3 +79,11 @@ type s9 struct { // want `struct has size 40 \(size class 48\), could be 24 \(si
 	a3     [3]bool
 	_      [0]func()
 }
+
+// Preserve comments.
+type s10 struct { // want `struct has size 40 \(size class 48\), could be 24 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\t_  \[0\]func\(\)\n\ti1 int\n\ti2 int\n\ta3 \[3\]bool\n\tb  bool\n}`
+	b      bool    // b is bool
+	i1, i2 int     // i1, i2 are int
+	a3     [3]bool // a3 is array of bool
+	_      [0]func()
+}

testdata/src/struct/p.go.golden

@@ -0,0 +1,89 @@
+// Copyright 2020 Orijtech, Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package p
+
+import "net/http/httptest"
+
+type s struct{}
+
+type s1 struct {
+	i int
+}
+
+type s2 struct {
+	i int
+	j int
+}
+
+type s3 struct { // want `struct has size 24 \(size class 32\), could be 16 \(size class 16\), you'll save 50.00% if you rearrange it to:\nstruct {\n\ty uint64\n\tx uint32\n\tz uint32\n}`
+	y uint64
+	x uint32
+	z uint32
+}
+
+type s4 struct { // want `struct has size 40 \(size class 48\), could be 24 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\t_  \[0\]func\(\)\n\ti1 int\n\ti2 int\n\ta3 \[3\]bool\n\tb  bool\n}`
+	_  [0]func()
+	i1 int
+	i2 int
+	a3 [3]bool
+	b  bool
+}
+
+// should be good, the struct has size 32, can be rearrange to have size 24, but runtime allocator
+// allocate the same size class 32.
+type s5 struct {
+	x uint32
+	y uint64
+	z *s
+	t uint32
+}
+
+type s6 struct { // should be good, see #16
+	bytep *uint8
+	mask  uint8
+	index uintptr
+}
+
+type s7 struct { // want `struct has size 40 \(size class 48\), could be 32 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\ty uint64\n\tt \*httptest.Server\n\tw uint64\n\tx uint32\n\tz uint32\n}`
+	y uint64
+	t *httptest.Server
+	w uint64
+	x uint32
+	z uint32
+}
+
+type s8 struct { // want `struct has size 40 \(size class 48\), could be 32 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\ty uint64\n\tt \*s\n\tw uint64\n\tx uint32\n\tz uint32\n}`
+	y uint64
+	t *s
+	w uint64
+	x uint32
+	z uint32
+}
+
+// Struct which combines multiple fields of the same type, see issue #41.
+type s9 struct { // want `struct has size 40 \(size class 48\), could be 24 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\t_  \[0\]func\(\)\n\ti1 int\n\ti2 int\n\ta3 \[3\]bool\n\tb  bool\n}`
+	_      [0]func()
+	i1, i2 int
+	a3     [3]bool
+	b      bool
+}
+
+// Preserve comments.
+type s10 struct { // want `struct has size 40 \(size class 48\), could be 24 \(size class 32\), you'll save 33.33% if you rearrange it to:\nstruct {\n\t_  \[0\]func\(\)\n\ti1 int\n\ti2 int\n\ta3 \[3\]bool\n\tb  bool\n}`
+	_      [0]func()
+	i1, i2 int     // i1, i2 are int
+	a3     [3]bool // a3 is array of bool
+	b      bool    // b is bool
+}