From 115afd299448895e333b77c3001bce7f1879e4d3 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 08:49:58 +0200 Subject: [PATCH 01/14] Install the "command" not the root package Thanks Andrew, fixes #3 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c643887..4d0be76 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ This tool will "reorder" your sources: There are several possibilities: -- If you have "go" on your machine, simply install using `go install github.com/metal3d/goreorder@latest` (you can replace "latest" by a known version) +- If you have "go" on your machine, simply install using `go install github.com/metal3d/goreorder/cmd/goreorder@latest` (you can replace "latest" by a known version) - Visit the [release page](https://github.com/metal3d/goreorder/releases) to download the latest version (to place un you `$PATH`) - Use the installer: ```bash From 29700c227e0c384c1f2d86c21afc1a3bce3a9347 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 11:18:35 +0200 Subject: [PATCH 02/14] Add documentation to check signature --- README.md | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4d0be76..cdd8214 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,11 @@ This tool will "reorder" your sources: There are several possibilities: -- If you have "go" on your machine, simply install using `go install github.com/metal3d/goreorder/cmd/goreorder@latest` (you can replace "latest" by a known version) -- Visit the [release page](https://github.com/metal3d/goreorder/releases) to download the latest version (to place un you `$PATH`) +- If you have "go" on your machine, simply install using (you can replace "latest" by a known tag): + ```bash + go install github.com/metal3d/goreorder/cmd/goreorder@latest` + ``` +- Visit the [release page](https://github.com/metal3d/goreorder/releases) to download the desired version (to place un you `$PATH`) - Use the installer: ```bash curl -sSL https://raw.githubusercontent.com/metal3d/goreorder/main/repo-tools/install.sh | bash -s @@ -32,7 +35,7 @@ cd goreorder make install ``` -## Basic Usage +# Basic Usage ``` goreorder reorders the structs (optional), methods and constructors in a Go @@ -78,8 +81,44 @@ patch -p1 < ./reorder.patch patch -p1 -R < ./reorder.patch ``` +# Releases are GPG signed + +The released binaries are signed with GPG. If you want to verify that the release comes from this repository and was built by the author: + +```bash + +## Optional, you can get and trust the owner GPG key +# this is the repo owner key: +_OWNERKEY="F3702E3FAD8F76DC" + +# you can import the repository owner key +gpg --keyserver hkps://keys.openpgp.org/ --recv-keys ${_OWNERKEY} + +# optoinal, trust owner key +_FPR=$(gpg -k --with-colons --fingerprint "${_OWNERKEY}" | awk -F: '/fpr/{print $10; exit}') +echo ${_FPR}:6: | gpg --import-ownertrust + +unset _OWNERKEY _FPR + +## Verification +# get the signature of the right binary +_REL="goreorder-linux-amd64" +_SIGNURL=https://github.com/metal3d/goreorder/releases/download/${_REL}.asc +curl ${_SIGNURL} -o /tmp/goreorder.asc +unset _SIGNURL _REL + +# get or set the path to the binary file you downloaded / installed +# _GOREORDERBIN=/path/to/the/binary +_GOREORDERBIN=$(command -v goreorder) + +# check the signature +gpg --verify /tmp/goreorder.asc $_GOREORDERBIN +rm /tmp/goreorder.asc +``` + # Contribute Please fill an issue to create a bug report. If you want to participate, please fork the repository and propose a pull request **on the "develop" branch**. + From d211fb693fdfdca22273f67df18c164de0e4c7d9 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 11:30:17 +0200 Subject: [PATCH 03/14] Enhance gpg check --- README.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index cdd8214..5cbdfff 100644 --- a/README.md +++ b/README.md @@ -89,16 +89,19 @@ The released binaries are signed with GPG. If you want to verify that the releas ## Optional, you can get and trust the owner GPG key # this is the repo owner key: -_OWNERKEY="F3702E3FAD8F76DC" +_KEY="F3702E3FAD8F76DC" +# You can get it with this command: +_KEY=$(curl -s https://api.github.com/users/metal3d/gpg_keys | \ + awk -F'"' '/"key_id"/{print $4; exit}') +echo ${_KEY} -# you can import the repository owner key -gpg --keyserver hkps://keys.openpgp.org/ --recv-keys ${_OWNERKEY} +# you can import the repository owner key from keyserver +gpg --keyserver hkps://keys.openpgp.org/ --recv-keys ${_KEY} # optoinal, trust owner key -_FPR=$(gpg -k --with-colons --fingerprint "${_OWNERKEY}" | awk -F: '/fpr/{print $10; exit}') +_FPR=$(gpg -k --with-colons --fingerprint "${_KEY}" | awk -F: '/fpr/{print $10; exit}') echo ${_FPR}:6: | gpg --import-ownertrust - -unset _OWNERKEY _FPR +unset _KEY _FPR ## Verification # get the signature of the right binary From dc4ca86896ed5985d3bbbf688c71f08dc7ba1507 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 14:04:45 +0200 Subject: [PATCH 04/14] Refactorisation and start working on const/vars --- cmd/goreorder/main.go | 16 ++- ordering/main.go | 53 ++++---- ordering/main_test.go | 96 +++++++++++++-- ordering/parser.go | 266 ++++++++++++++++++++++++---------------- ordering/parser_test.go | 95 ++++++++++++++ 5 files changed, 393 insertions(+), 133 deletions(-) create mode 100644 ordering/parser_test.go diff --git a/cmd/goreorder/main.go b/cmd/goreorder/main.go index 7c74a25..62d9ba4 100644 --- a/cmd/goreorder/main.go +++ b/cmd/goreorder/main.go @@ -178,7 +178,13 @@ func processFile(fileOrDirectoryName string, formatToolName string, reorderStruc if input != nil && len(input) != 0 { // process stdin - content, err := ordering.ReorderSource(fileOrDirectoryName, formatToolName, reorderStructs, input, diff) + content, err := ordering.ReorderSource(ordering.ReorderConfig{ + Filename: fileOrDirectoryName, + FormatCommand: formatToolName, + ReorderStructs: reorderStructs, + Src: input, + Diff: diff, + }) if err != nil { log.Fatal(err) } @@ -213,7 +219,13 @@ func processFile(fileOrDirectoryName string, formatToolName string, reorderStruc } log.Println("Processing file: " + fileOrDirectoryName) - output, err := ordering.ReorderSource(fileOrDirectoryName, formatToolName, reorderStructs, input, diff) + output, err := ordering.ReorderSource(ordering.ReorderConfig{ + Filename: fileOrDirectoryName, + FormatCommand: formatToolName, + ReorderStructs: reorderStructs, + Src: input, + Diff: diff, + }) if err != nil { log.Println("ERR: Ordering error:", err) return diff --git a/ordering/main.go b/ordering/main.go index 73fe531..b7edcfb 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -11,49 +11,60 @@ import ( "strings" ) +type ReorderConfig struct { + Filename string + FormatCommand string + ReorderStructs bool + Diff bool + Src interface{} +} + // ReorderSource reorders the source code in the given filename. It will be helped by the formatCommand (gofmt or goimports). The method is to // use the Parse() function to extract types, methods and constructors. Then we replace the original source code with a comment containing the // sha256 of the source. This is made to not lose the original source code "lenght" while we reinject the ordered source code. Then, we finally // remove thses lines from the source code. -func ReorderSource(filename, formatCommand string, reorderStructs bool, src interface{}, diff bool) (string, error) { +func ReorderSource(opt ReorderConfig) (string, error) { // in all cases, we must return the original source code if an error occurs // get the content of the file + filename := opt.Filename + reorderStructs := opt.ReorderStructs + var content []byte var err error - if src == nil || len(src.([]byte)) == 0 { + if opt.Src == nil || len(opt.Src.([]byte)) == 0 { content, err = ioutil.ReadFile(filename) if err != nil { return "", err } } else { - content = src.([]byte) + content = opt.Src.([]byte) } - methods, constructors, structs, err := Parse(filename, formatCommand, content) + info, err := Parse(filename, content) if err != nil { return string(content), errors.New("Error parsing source: " + err.Error()) } - if len(structs) == 0 { + if len(info.Structs) == 0 { return string(content), errors.New("No structs found in " + filename + ", cannot reorder") } // sort methods by name - for _, method := range methods { + for _, method := range info.Methods { sort.Slice(method, func(i, j int) bool { return method[i].Name < method[j].Name }) } - for _, method := range constructors { - sort.Slice(method, func(i, j int) bool { - return method[i].Name < method[j].Name + for _, constructor := range info.Constructors { + sort.Slice(constructor, func(i, j int) bool { + return constructor[i].Name < constructor[j].Name }) } - structNames := make([]string, 0, len(methods)) - for _, s := range structs { + structNames := make([]string, 0, len(info.Methods)) + for _, s := range info.Structs { structNames = append(structNames, s.Name) } if reorderStructs { @@ -73,35 +84,35 @@ func ReorderSource(filename, formatCommand string, reorderStructs bool, src inte removedLines := 0 for _, typename := range structNames { if removedLines == 0 { - lineNumberWhereInject = structs[typename].OpeningLine + lineNumberWhereInject = info.Structs[typename].OpeningLine } // replace the definitions by "// -- line to remove - for ln := structs[typename].OpeningLine - 1; ln < structs[typename].ClosingLine; ln++ { + for ln := info.Structs[typename].OpeningLine - 1; ln < info.Structs[typename].ClosingLine; ln++ { originalContent[ln] = "// -- " + sign } - removedLines += structs[typename].ClosingLine - structs[typename].OpeningLine + removedLines += info.Structs[typename].ClosingLine - info.Structs[typename].OpeningLine // add the struct definition to "source" - source = append(source, "\n\n"+structs[typename].SourceCode) + source = append(source, "\n\n"+info.Structs[typename].SourceCode) // same for constructors - for _, constructor := range constructors[typename] { + for _, constructor := range info.Constructors[typename] { for ln := constructor.OpeningLine - 1; ln < constructor.ClosingLine; ln++ { originalContent[ln] = "// -- " + sign } // add the constructor to "source" source = append(source, "\n"+constructor.SourceCode) } - removedLines += len(constructors[typename]) + removedLines += len(info.Constructors[typename]) // same for methods - for _, method := range methods[typename] { + for _, method := range info.Methods[typename] { for ln := method.OpeningLine - 1; ln < method.ClosingLine; ln++ { originalContent[ln] = "// -- " + sign } // add the method to "source" source = append(source, "\n"+method.SourceCode) } - removedLines += len(methods[typename]) + removedLines += len(info.Methods[typename]) } // add the "source" at the found lineNumberWhereInject @@ -131,7 +142,7 @@ func ReorderSource(filename, formatCommand string, reorderStructs bool, src inte return string(content), errors.New("Failed to write to temporary file: " + err.Error()) } - cmd := exec.Command(formatCommand, "-w", tmpfile.Name()) + cmd := exec.Command(opt.FormatCommand, "-w", tmpfile.Name()) if err := cmd.Run(); err != nil { return string(content), err } @@ -142,7 +153,7 @@ func ReorderSource(filename, formatCommand string, reorderStructs bool, src inte return string(content), errors.New("Read Temporary File error: " + err.Error()) } - if diff { + if opt.Diff { return doDiff(content, newcontent, filename) } return string(newcontent), nil diff --git a/ordering/main_test.go b/ordering/main_test.go index 26b0efb..4a5fa5b 100644 --- a/ordering/main_test.go +++ b/ordering/main_test.go @@ -121,7 +121,13 @@ func TestReorder(t *testing.T) { defer teardown(filename, tmpdir) // reorder the file - content, err := ReorderSource(filename, "gofmt", true, nil, false) + content, err := ReorderSource(ReorderConfig{ + Filename: filename, + FormatCommand: "gofmt", + ReorderStructs: true, + Src: nil, + Diff: false, + }) if err != nil { t.Error(err) } @@ -139,7 +145,13 @@ func TestNoStruct(t *testing.T) { fmt.Println("nothing") } ` - content, err := ReorderSource(source, "gofmt", true, []byte(source), false) + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: []byte(source), + Diff: false, + }) if err == nil { t.Error("Expected error for no found struct") } @@ -149,7 +161,14 @@ func TestNoStruct(t *testing.T) { } func TestBadFile(t *testing.T) { - _, err := ReorderSource("/tmp/foo.go", "gofmt", true, nil, false) + //_, err := ReorderSource("/tmp/foo.go", "gofmt", true, nil, false) + _, err := ReorderSource(ReorderConfig{ + Filename: "/tmp/foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: nil, + Diff: false, + }) if err == nil { t.Error("Expected error") } @@ -164,7 +183,13 @@ func TestSpecialTypes(t *testing.T) { fmt.Println("nothing") } ` - content, err := ReorderSource(source, "gofmt", true, []byte(source), false) + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: []byte(source), + Diff: false, + }) if err == nil { t.Error("Expected error") } @@ -191,7 +216,13 @@ func main() { func (f *Bar) FooMethod1() { fmt.Println("FooMethod1") }` - content, err := ReorderSource("foo.go", "gofmt", true, []byte(source), false) + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: []byte(source), + Diff: false, + }) if err != nil { t.Error(err) } @@ -264,7 +295,13 @@ func bar() { } ` - content, err := ReorderSource("foo.go", "gofmt", true, []byte(source), false) + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: []byte(source), + Diff: false, + }) if err != nil { t.Error(err) } @@ -278,7 +315,52 @@ func TestDiff(t *testing.T) { defer teardown(filename, tmpdir) // for now, only test that no error is returned - if _, err := ReorderSource(filename, "gofmt", true, nil, true); err != nil { + if _, err := ReorderSource(ReorderConfig{ + Filename: filename, + FormatCommand: "gofmt", + ReorderStructs: true, + Src: nil, + Diff: true, + }); err != nil { + t.Error(err) + } +} + +func TestGlobalVarPlace(t *testing.T) { + const globalVarSource = `package main + +var _ Foo = (*Bar)(nil) + +const ( + baz1 = 1 + baz2 = 2 +) + +var ( + bar2 = 2 + bar1 = 1 +) + +// comment 1 +// comment 2 +type Foo struct { + // comment 3 + idfoo int +} + +func (f *Foo) FooMethod1() { + print("FooMethod1") +} +` + _, err := ReorderSource(ReorderConfig{ + Filename: "test.go", + FormatCommand: "gofmt", + ReorderStructs: false, + Src: []byte(globalVarSource), + Diff: false, + }) + + if err != nil { t.Error(err) } } diff --git a/ordering/parser.go b/ordering/parser.go index de8ffa0..4c3daf0 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -20,6 +20,15 @@ type GoType struct { ClosingLine int } +// ParsedInfo contains information we need to sort in the source file. +type ParsedInfo struct { + Methods map[string][]*GoType + Constructors map[string][]*GoType + Structs map[string]*GoType + Constants map[string]*GoType + Variables map[string]*GoType +} + // GetMethodComments returns the comments for the given method. func GetMethodComments(d *ast.FuncDecl) (comments []string) { if d == nil || d.Doc == nil || d.Doc.List == nil { @@ -45,19 +54,23 @@ func GetTypeComments(d *ast.GenDecl) (comments []string) { } // Parse the given file and return the methods, constructors and structs. -func Parse(filename, formatCommand string, src interface{}) (map[string][]*GoType, map[string][]*GoType, map[string]*GoType, error) { +func Parse(filename string, src interface{}) (*ParsedInfo, error) { fset := token.NewFileSet() f, err := parser.ParseFile(fset, filename, src, parser.ParseComments) if err != nil { - return nil, nil, nil, err + return nil, err } - methods := make(map[string][]*GoType) - constructors := make(map[string][]*GoType) - structTypes := make(map[string]*GoType) + var ( + methods = make(map[string][]*GoType) + constructors = make(map[string][]*GoType) + structTypes = make(map[string]*GoType) + varTypes = make(map[string]*GoType) + constTypes = make(map[string]*GoType) + sourceCode []byte + ) - var sourceCode []byte if src == nil { // error should never happen as Parse() worked sourceCode, _ = ioutil.ReadFile(filename) @@ -66,120 +79,167 @@ func Parse(filename, formatCommand string, src interface{}) (map[string][]*GoTyp } sourceLines := strings.Split(string(sourceCode), "\n") - // Iterate over all the top-level declarations in the file. Only find methods for types, set the type as key and the method as value + // Iterate over all the top-level declarations in the file. + // We're looking for type declarations and function declarations. Not constructors yet. for _, decl := range f.Decls { switch d := decl.(type) { case *ast.FuncDecl: - if d.Recv != nil { - // Method + findMethods(d, fset, sourceLines, methods) + // find struct declarations + case *ast.GenDecl: + findStructs(d, fset, sourceLines, structTypes) + findGlobalVarsAndConsts(d, fset, sourceLines, varTypes, constTypes) + } + } - if d.Recv.List == nil || len(d.Recv.List) == 0 { - continue - } - if d.Recv.List[0].Type == nil { - continue - } - if _, ok := d.Recv.List[0].Type.(*ast.StarExpr); !ok { - continue - } - if d.Recv.List[0].Type.(*ast.StarExpr).X == nil { - continue - } - if _, ok := d.Recv.List[0].Type.(*ast.StarExpr).X.(*ast.Ident); !ok { - continue - } + // Now that we have found types and methods, we will try to find constructors + for _, decl := range f.Decls { + switch d := decl.(type) { + case *ast.FuncDecl: + findConstructors(d, fset, sourceLines, methods, constructors) + } + } + + return &ParsedInfo{ + Structs: structTypes, + Methods: methods, + Constructors: constructors, + Variables: varTypes, + Constants: constTypes, + }, nil +} - // in "func (T) Method(...) ..." get the type T name - structName := d.Recv.List[0].Type.(*ast.StarExpr).X.(*ast.Ident).Name +func findStructs(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, structTypes map[string]*GoType) { + if d.Tok != token.TYPE { + return + } + for _, spec := range d.Specs { + if s, ok := spec.(*ast.TypeSpec); ok { + // is it a struct? + if _, ok := s.Type.(*ast.StructType); !ok { + // no... skip + continue + } + typeDef := &GoType{ + Name: s.Name.Name, + OpeningLine: fset.Position(d.Pos()).Line, + ClosingLine: fset.Position(d.End()).Line, + } + comments := GetTypeComments(d) + typeDef.SourceCode = strings.Join(comments, "\n") + + "\n" + + strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") + typeDef.OpeningLine -= len(comments) - // Create a new method - method := &GoType{ - Name: d.Name.Name, - OpeningLine: fset.Position(d.Pos()).Line, - ClosingLine: fset.Position(d.End()).Line, - } + structTypes[s.Name.Name] = typeDef + } + } +} - // Get the method source code - comments := GetMethodComments(d) - method.SourceCode = strings.Join(comments, "\n") + - "\n" + - strings.Join(sourceLines[method.OpeningLine-1:method.ClosingLine], "\n") - method.OpeningLine -= len(comments) +func findMethods(d *ast.FuncDecl, fset *token.FileSet, sourceLines []string, methods map[string][]*GoType) { + + if d.Recv == nil { + return + } + // Method + if d.Recv.List == nil || len(d.Recv.List) == 0 { // not a method + return + } + if d.Recv.List[0].Type == nil { // no receiver type... weird but skip + return + } + if _, ok := d.Recv.List[0].Type.(*ast.StarExpr); !ok { // not a pointer receiver + return + } + if d.Recv.List[0].Type.(*ast.StarExpr).X == nil { // no receiver type... weird but skip + return + } + if _, ok := d.Recv.List[0].Type.(*ast.StarExpr).X.(*ast.Ident); !ok { // not named receiver, skip + return + } + + // in "func (T) Method(...) ..." get the type T name + structName := d.Recv.List[0].Type.(*ast.StarExpr).X.(*ast.Ident).Name - // Add the method to the map - methods[structName] = append(methods[structName], method) + // Create a new method + method := &GoType{ + Name: d.Name.Name, + OpeningLine: fset.Position(d.Pos()).Line, + ClosingLine: fset.Position(d.End()).Line, + } + + // Get the method source code + comments := GetMethodComments(d) + method.SourceCode = strings.Join(comments, "\n") + + "\n" + + strings.Join(sourceLines[method.OpeningLine-1:method.ClosingLine], "\n") + method.OpeningLine -= len(comments) + + // Add the method to the map + methods[structName] = append(methods[structName], method) +} + +func findConstructors(d *ast.FuncDecl, fset *token.FileSet, sourceLines []string, methods, constructors map[string][]*GoType) { + + if d.Type == nil || d.Type.Results == nil || len(d.Type.Results.List) == 0 { // no return type + return + } + // in "func Something(...) x, T" get the type T name and check if it's in "methods" map + // Get the return types + for _, r := range d.Type.Results.List { + if exp, ok := r.Type.(*ast.StarExpr); ok { + if _, ok := exp.X.(*ast.Ident); !ok { + continue } - // find struct declarations - case *ast.GenDecl: - if d.Tok == token.TYPE { - for _, spec := range d.Specs { - if s, ok := spec.(*ast.TypeSpec); ok { - // is it a struct? - if _, ok := s.Type.(*ast.StructType); !ok { - // no... skip - continue - } - typeDef := &GoType{ - Name: s.Name.Name, - OpeningLine: fset.Position(d.Pos()).Line, - ClosingLine: fset.Position(d.End()).Line, - } - comments := GetTypeComments(d) - typeDef.SourceCode = strings.Join(comments, "\n") + - "\n" + - strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") - typeDef.OpeningLine -= len(comments) - - structTypes[s.Name.Name] = typeDef - } - } + returnType := exp.X.(*ast.Ident).Name + if _, ok := methods[returnType]; !ok { + // not a contructor for detected types above, skip + continue } + // Create a new method + method := &GoType{ + Name: d.Name.Name, + OpeningLine: fset.Position(d.Pos()).Line, + ClosingLine: fset.Position(d.End()).Line, + } + + // Get the method source code + comments := GetMethodComments(d) + method.SourceCode = strings.Join(comments, "\n") + + "\n" + + strings.Join(sourceLines[method.OpeningLine-1:method.ClosingLine], "\n") + method.OpeningLine -= len(comments) + + // Add the method to the constructors map + constructors[returnType] = append(constructors[returnType], method) } } +} - // now that we have found types and methods, we will try to find constructors - for _, decl := range f.Decls { - switch d := decl.(type) { - case *ast.FuncDecl: - if d.Recv == nil { - if d.Type == nil || d.Type.Results == nil || len(d.Type.Results.List) == 0 { - continue +func findGlobalVarsAndConsts(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, varTypes, constTypes map[string]*GoType) { + if d.Tok != token.VAR && d.Tok != token.CONST { + return + } + for _, spec := range d.Specs { + if s, ok := spec.(*ast.ValueSpec); ok { + for _, name := range s.Names { + typeDef := &GoType{ + Name: name.Name, + OpeningLine: fset.Position(d.Pos()).Line, + ClosingLine: fset.Position(d.End()).Line, } - - // in "func Something(...) x, T" get the type T name and check if it's in "methods" map - // Get the return types - for _, r := range d.Type.Results.List { - if exp, ok := r.Type.(*ast.StarExpr); ok { - if _, ok := exp.X.(*ast.Ident); !ok { - continue - } - returnType := exp.X.(*ast.Ident).Name - if _, ok := methods[returnType]; !ok { - // not a contructor for detected types above, skip - continue - } - // Create a new method - method := &GoType{ - Name: d.Name.Name, - OpeningLine: fset.Position(d.Pos()).Line, - ClosingLine: fset.Position(d.End()).Line, - } - - // Get the method source code - comments := GetMethodComments(d) - method.SourceCode = strings.Join(comments, "\n") + - "\n" + - strings.Join(sourceLines[method.OpeningLine-1:method.ClosingLine], "\n") - method.OpeningLine -= len(comments) - - // Add the method to the constructors map - constructors[returnType] = append(constructors[returnType], method) - } + comments := GetTypeComments(d) + typeDef.SourceCode = strings.Join(comments, "\n") + + "\n" + + strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") + typeDef.OpeningLine -= len(comments) + if d.Tok == token.CONST { + constTypes[name.Name] = typeDef + } else { + varTypes[name.Name] = typeDef } } } } - - return methods, constructors, structTypes, nil } diff --git a/ordering/parser_test.go b/ordering/parser_test.go new file mode 100644 index 0000000..1fe0240 --- /dev/null +++ b/ordering/parser_test.go @@ -0,0 +1,95 @@ +package ordering + +import "testing" + +func TestParse(t *testing.T) { + const source = `package main + + const C1 = 1 + const C2 = 2 + + var V1 = 1 + var V2 = 2 + + + const ( + C3 = 3 + C4 = 4 + ) + + var ( + V3 = 3 + V4 = 4 + ) + + func main() { + + } + + type C struct {} + func NewC() *C { return &C{} } + func (c *C) C1() {} + + type A struct {} + type B struct {} + + func (a *A) A1() {} + func (a *A) A2() {} + func (a *A) A3() {} + + func (b *B) B1() {} + func (b *B) B2() {} + func (b *B) B3() {} + + + ` + parsed, err := Parse("test.go", []byte(source)) + if err != nil { + t.Error(err) + } + if parsed == nil { + t.Error("Expected parsed info") + } + if len(parsed.Constants) != 4 { + t.Errorf("Expected 4 consts, got %d", len(parsed.Constants)) + } + if len(parsed.Variables) != 4 { + t.Errorf("Expected 4 vars, got %d", len(parsed.Variables)) + } + if len(parsed.Structs) != 3 { + t.Errorf("Expected 2 structs, got %d", len(parsed.Structs)) + } + if len(parsed.Constructors) != 1 { + t.Errorf("Expected 1 constructor, got %d", len(parsed.Constructors)) + } + + for n, m := range parsed.Methods { + switch n { + case "A": + if len(m) != 3 { + t.Errorf("Expected 3 methods for A, got %d", len(m)) + } + case "B": + if len(m) != 3 { + t.Errorf("Expected 3 methods for B, got %d", len(m)) + } + case "C": + if len(m) != 1 { + t.Errorf("Expected 1 method for C, got %d", len(m)) + } + default: + t.Errorf("Unexpected struct %s", n) + } + } + + for n, m := range parsed.Constructors { + switch n { + case "C": + if len(m) != 1 { + t.Errorf("Expected 1 constructor for C, got %d", len(m)) + } + default: + t.Errorf("Unexpected struct %s", n) + } + } +} From 7f6d7c9d3e8718d5a61b5cc9cb8871d80a4ecccf Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 14:09:07 +0200 Subject: [PATCH 05/14] Allow filtering test --- Makefile | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f77aee3..dc51e29 100644 --- a/Makefile +++ b/Makefile @@ -16,6 +16,10 @@ SIGNER=metal3d@gmail.com TARGETS=dist/goreorder-linux-amd64 dist/goreorder-darwin-amd64 dist/goreorder-windows-amd64.exe dist/goreorder-freebsd-amd64 +# TEST selection +# e.g. TEST="TestFoo TestBar" +TEST= + install: go install -v $(CC_OPTS) $(PACKAGE) @@ -80,8 +84,12 @@ clean-dist: clean: clean-dist rm -f ./goreorder -test: +test: +ifeq ($(strip $(TEST)),) go test -v -race -cover -short ./... +else + go test -v -race -run $(TEST) ./... +endif test-cover-html: go test -cover -coverprofile=coverage.out ./... From cc8fc0f57593ef91a12b30ade83ed4325177d483 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 14:12:00 +0200 Subject: [PATCH 06/14] Use switch/case to avoid too generic "else" --- ordering/parser.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ordering/parser.go b/ordering/parser.go index 4c3daf0..e11f1e3 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -234,9 +234,10 @@ func findGlobalVarsAndConsts(d *ast.GenDecl, fset *token.FileSet, sourceLines [] "\n" + strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") typeDef.OpeningLine -= len(comments) - if d.Tok == token.CONST { + switch { + case d.Tok == token.CONST: constTypes[name.Name] = typeDef - } else { + case d.Tok == token.VAR: varTypes[name.Name] = typeDef } } From d0c14705b84eeb9323d4a610f01017695013f789 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 14:12:57 +0200 Subject: [PATCH 07/14] What a bad syntax I used... --- ordering/parser.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ordering/parser.go b/ordering/parser.go index e11f1e3..2df2945 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -234,10 +234,10 @@ func findGlobalVarsAndConsts(d *ast.GenDecl, fset *token.FileSet, sourceLines [] "\n" + strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") typeDef.OpeningLine -= len(comments) - switch { - case d.Tok == token.CONST: + switch d.Tok { + case token.CONST: constTypes[name.Name] = typeDef - case d.Tok == token.VAR: + case token.VAR: varTypes[name.Name] = typeDef } } From d0469a4d4ccd4a9778aac9680a50267e92f3ea8f Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 15:35:05 +0200 Subject: [PATCH 08/14] Bad call order on close and remove --- ordering/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ordering/main.go b/ordering/main.go index b7edcfb..cd64b37 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -134,8 +134,9 @@ func ReorderSource(opt ReorderConfig) (string, error) { return string(content), errors.New("Failed to create temp file: " + err.Error()) } defer func() { - os.Remove(tmpfile.Name()) // clean up + // close and remove the temporary file tmpfile.Close() + os.Remove(tmpfile.Name()) }() if _, err := tmpfile.Write([]byte(output)); err != nil { From 69d87dece9c2347b370f5a36ec69ba641dae60dc Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 15:38:28 +0200 Subject: [PATCH 09/14] Avoid some var aliases --- ordering/main.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/ordering/main.go b/ordering/main.go index cd64b37..921306b 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/ioutil" + "log" "os" "os/exec" "sort" @@ -26,13 +27,11 @@ type ReorderConfig struct { func ReorderSource(opt ReorderConfig) (string, error) { // in all cases, we must return the original source code if an error occurs // get the content of the file - filename := opt.Filename - reorderStructs := opt.ReorderStructs var content []byte var err error if opt.Src == nil || len(opt.Src.([]byte)) == 0 { - content, err = ioutil.ReadFile(filename) + content, err = ioutil.ReadFile(opt.Filename) if err != nil { return "", err } @@ -40,14 +39,14 @@ func ReorderSource(opt ReorderConfig) (string, error) { content = opt.Src.([]byte) } - info, err := Parse(filename, content) + info, err := Parse(opt.Filename, content) if err != nil { return string(content), errors.New("Error parsing source: " + err.Error()) } if len(info.Structs) == 0 { - return string(content), errors.New("No structs found in " + filename + ", cannot reorder") + return string(content), errors.New("No structs found in " + opt.Filename + ", cannot reorder") } // sort methods by name @@ -65,9 +64,10 @@ func ReorderSource(opt ReorderConfig) (string, error) { structNames := make([]string, 0, len(info.Methods)) for _, s := range info.Structs { + log.Println("s.Name", s.Name) structNames = append(structNames, s.Name) } - if reorderStructs { + if opt.ReorderStructs { sort.Strings(structNames) } @@ -155,7 +155,7 @@ func ReorderSource(opt ReorderConfig) (string, error) { } if opt.Diff { - return doDiff(content, newcontent, filename) + return doDiff(content, newcontent, opt.Filename) } return string(newcontent), nil } From 55fb02300202e33f0197be846a223131dbb85a53 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 16:30:30 +0200 Subject: [PATCH 10/14] Make better checks and fix ordering Structs - See #6 - in progress to resolve - Zee #4 - in propose to resolve --- ordering/main.go | 60 ++++++++++++-------- ordering/main_test.go | 126 +++++++++++++++++++++++++++++++++++++++++ ordering/parser.go | 8 ++- ordering/sortString.go | 35 ++++++++++++ 4 files changed, 203 insertions(+), 26 deletions(-) create mode 100644 ordering/sortString.go diff --git a/ordering/main.go b/ordering/main.go index 921306b..34e4033 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -4,8 +4,8 @@ import ( "crypto/sha256" "errors" "fmt" + "go/format" "io/ioutil" - "log" "os" "os/exec" "sort" @@ -62,13 +62,8 @@ func ReorderSource(opt ReorderConfig) (string, error) { }) } - structNames := make([]string, 0, len(info.Methods)) - for _, s := range info.Structs { - log.Println("s.Name", s.Name) - structNames = append(structNames, s.Name) - } if opt.ReorderStructs { - sort.Strings(structNames) + info.StructNames.Sort() } // Get the source code signature - we will use this to mark the lines to remove later @@ -82,7 +77,7 @@ func ReorderSource(opt ReorderConfig) (string, error) { lineNumberWhereInject := 0 removedLines := 0 - for _, typename := range structNames { + for _, typename := range *info.StructNames { if removedLines == 0 { lineNumberWhereInject = info.Structs[typename].OpeningLine } @@ -129,33 +124,50 @@ func ReorderSource(opt ReorderConfig) (string, error) { output := strings.Join(originalContent, "\n") // write in a temporary file and use "gofmt" to format it + newcontent := []byte(output) + switch opt.FormatCommand { + case "gofmt": + // format the temporary file + newcontent, err = format.Source([]byte(output)) + if err != nil { + return string(content), errors.New("Failed to format source: " + err.Error()) + } + default: + if newcontent, err = formatWithCommand(content, output, opt); err != nil { + return string(content), errors.New("Failed to format source: " + err.Error()) + } + } + + if opt.Diff { + return doDiff(content, newcontent, opt.Filename) + } + return string(newcontent), nil +} + +func formatWithCommand(content []byte, output string, opt ReorderConfig) (newcontent []byte, err error) { + // we use the format command given by the user + // on a temporary file we need to create and remove tmpfile, err := ioutil.TempFile("", "") if err != nil { - return string(content), errors.New("Failed to create temp file: " + err.Error()) + return content, errors.New("Failed to create temp file: " + err.Error()) } - defer func() { - // close and remove the temporary file - tmpfile.Close() - os.Remove(tmpfile.Name()) - }() + defer os.Remove(tmpfile.Name()) + // write the temporary file if _, err := tmpfile.Write([]byte(output)); err != nil { - return string(content), errors.New("Failed to write to temporary file: " + err.Error()) + return content, errors.New("Failed to write temp file: " + err.Error()) } + tmpfile.Close() + // format the temporary file cmd := exec.Command(opt.FormatCommand, "-w", tmpfile.Name()) if err := cmd.Run(); err != nil { - return string(content), err + return content, err } - // read the temporary file - newcontent, err := ioutil.ReadFile(tmpfile.Name()) + newcontent, err = ioutil.ReadFile(tmpfile.Name()) if err != nil { - return string(content), errors.New("Read Temporary File error: " + err.Error()) + return content, errors.New("Read Temporary File error: " + err.Error()) } - - if opt.Diff { - return doDiff(content, newcontent, opt.Filename) - } - return string(newcontent), nil + return newcontent, nil } diff --git a/ordering/main_test.go b/ordering/main_test.go index 4a5fa5b..1a44c61 100644 --- a/ordering/main_test.go +++ b/ordering/main_test.go @@ -364,3 +364,129 @@ func (f *Foo) FooMethod1() { t.Error(err) } } + +func TestNoOrderStructs(t *testing.T) { + const source = `package main +type grault struct {} +type xyzzy struct {} +type bar struct {} +type qux struct {} +type quux struct {} +type corge struct {} +type garply struct {} +type baz struct {} +type waldo struct {} +type fred struct {} +type plugh struct {} +type foo struct {} +` + const expected = `package main + +type grault struct{} + +type xyzzy struct{} + +type bar struct{} + +type qux struct{} + +type quux struct{} + +type corge struct{} + +type garply struct{} + +type baz struct{} + +type waldo struct{} + +type fred struct{} + +type plugh struct{} + +type foo struct{} +` + + const orderedSource = `package main + +type bar struct{} + +type baz struct{} + +type corge struct{} + +type foo struct{} + +type fred struct{} + +type garply struct{} + +type grault struct{} + +type plugh struct{} + +type quux struct{} + +type qux struct{} + +type waldo struct{} + +type xyzzy struct{} +` + + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: false, + Src: []byte(source), + Diff: false, + }) + if err != nil { + t.Error(err) + } + if content != expected { + t.Errorf("Expected UNORDERED:\n%s\nGot:\n%s\n", expected, content) + } + + content, err = ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "gofmt", + ReorderStructs: true, + Src: []byte(source), + Diff: false, + }) + if err != nil { + t.Error(err) + } + if content != orderedSource { + t.Errorf("Expected ORDERED:\n%s\nGot:\n%s\n", orderedSource, content) + } + +} + +func TestBadFormatCommand(t *testing.T) { + const source = `package main + +import ( + "os" + "fmt" +) +type grault struct {} +type xyzzy struct {} +type bar struct {} +` + content, err := ReorderSource(ReorderConfig{ + Filename: "foo.go", + FormatCommand: "wthcommand", + ReorderStructs: false, + Src: []byte(source), + Diff: false, + }) + + if err == nil { + t.Error("Expected error, got nil") + } + if content != source { + t.Errorf("Expected:\n%s\nGot:\n%s\n", source, content) + } +} diff --git a/ordering/parser.go b/ordering/parser.go index 2df2945..84586d5 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -27,6 +27,7 @@ type ParsedInfo struct { Structs map[string]*GoType Constants map[string]*GoType Variables map[string]*GoType + StructNames *StingList } // GetMethodComments returns the comments for the given method. @@ -66,6 +67,7 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { methods = make(map[string][]*GoType) constructors = make(map[string][]*GoType) structTypes = make(map[string]*GoType) + structNames = &StingList{} varTypes = make(map[string]*GoType) constTypes = make(map[string]*GoType) sourceCode []byte @@ -87,7 +89,7 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { findMethods(d, fset, sourceLines, methods) // find struct declarations case *ast.GenDecl: - findStructs(d, fset, sourceLines, structTypes) + findStructs(d, fset, sourceLines, structNames, structTypes) findGlobalVarsAndConsts(d, fset, sourceLines, varTypes, constTypes) } } @@ -102,6 +104,7 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { return &ParsedInfo{ Structs: structTypes, + StructNames: structNames, Methods: methods, Constructors: constructors, Variables: varTypes, @@ -109,7 +112,7 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { }, nil } -func findStructs(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, structTypes map[string]*GoType) { +func findStructs(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, stuctNames *StingList, structTypes map[string]*GoType) { if d.Tok != token.TYPE { return } @@ -132,6 +135,7 @@ func findStructs(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, stru typeDef.OpeningLine -= len(comments) structTypes[s.Name.Name] = typeDef + stuctNames.Add(s.Name.Name) } } } diff --git a/ordering/sortString.go b/ordering/sortString.go new file mode 100644 index 0000000..3b020a5 --- /dev/null +++ b/ordering/sortString.go @@ -0,0 +1,35 @@ +package ordering + +import "sort" + +var _ sort.Interface = (*StingList)(nil) + +// StingList is a list of strings that *can* be sorted. +// +// Implement sort.Interface +type StingList []string + +// Len returns the length of the list. +func (s *StingList) Len() int { + return len(*s) +} + +// Swap swaps the elements with indexes i and j. +func (s StingList) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +// Less reports whether the element with index i should sort before the element with index j. +func (s StingList) Less(i, j int) bool { + return s[i] < s[j] +} + +// Sort sorts the list. +func (s *StingList) Sort() { + sort.Sort(s) +} + +// Add adds a string to the list. +func (s *StingList) Add(str string) { + *s = append(*s, str) +} From 22dab784ec8541223ae71be162f513486f7912ba Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Thu, 22 Jun 2023 18:02:30 +0200 Subject: [PATCH 11/14] Only code comment format --- ordering/main.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ordering/main.go b/ordering/main.go index 34e4033..44e5252 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -20,14 +20,16 @@ type ReorderConfig struct { Src interface{} } -// ReorderSource reorders the source code in the given filename. It will be helped by the formatCommand (gofmt or goimports). The method is to -// use the Parse() function to extract types, methods and constructors. Then we replace the original source code with a comment containing the -// sha256 of the source. This is made to not lose the original source code "lenght" while we reinject the ordered source code. Then, we finally +// ReorderSource reorders the source code in the given filename. +// It will be helped by the formatCommand (gofmt or goimports). +// If gofmt is used, the source code will be formatted with the go/fmt package in memory. +// +// This function calls the Parse() function to extract types, methods, vars, consts and constructors. +// Then it replaces the original source code with a comment containing the +// sha256 of the source. This is made to not lose the original source code "lenght" +// while we reinject the ordered source code. Then, we finally // remove thses lines from the source code. func ReorderSource(opt ReorderConfig) (string, error) { - // in all cases, we must return the original source code if an error occurs - // get the content of the file - var content []byte var err error if opt.Src == nil || len(opt.Src.([]byte)) == 0 { From cd89890bef3ed87df09ffe9eae7f64c1334086b1 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Fri, 23 Jun 2023 10:31:33 +0200 Subject: [PATCH 12/14] Rewrite vars and const on top of file This could not be the expected behavior but this actually helps to make the source code better. See #4 for details. --- ordering/main.go | 30 ++++++++++++++++ ordering/main_test.go | 12 +++---- ordering/parser.go | 78 +++++++++++++++++++++++++++++++++++++---- ordering/parser_test.go | 4 +-- 4 files changed, 109 insertions(+), 15 deletions(-) diff --git a/ordering/main.go b/ordering/main.go index 44e5252..71e5476 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -79,6 +79,26 @@ func ReorderSource(opt ReorderConfig) (string, error) { lineNumberWhereInject := 0 removedLines := 0 + for i, sourceCode := range info.Constants { + if removedLines == 0 { + lineNumberWhereInject = info.Constants[i].OpeningLine + } + for ln := sourceCode.OpeningLine - 1; ln < sourceCode.ClosingLine; ln++ { + originalContent[ln] = "// -- " + sign + } + source = append(source, "\n"+sourceCode.SourceCode) + removedLines += len(info.Constants) + } + for i, sourceCode := range info.Variables { + if removedLines == 0 { + lineNumberWhereInject = info.Variables[i].OpeningLine + } + for ln := sourceCode.OpeningLine - 1; ln < sourceCode.ClosingLine; ln++ { + originalContent[ln] = "// -- " + sign + } + source = append(source, "\n"+sourceCode.SourceCode) + removedLines += len(info.Variables) + } for _, typename := range *info.StructNames { if removedLines == 0 { lineNumberWhereInject = info.Structs[typename].OpeningLine @@ -111,6 +131,16 @@ func ReorderSource(opt ReorderConfig) (string, error) { } removedLines += len(info.Methods[typename]) } + for i, sourceCode := range info.Functions { + if removedLines == 0 { + lineNumberWhereInject = info.Functions[i].OpeningLine + } + for ln := sourceCode.OpeningLine - 1; ln < sourceCode.ClosingLine; ln++ { + originalContent[ln] = "// -- " + sign + } + source = append(source, "\n"+sourceCode.SourceCode) + removedLines += len(info.Functions) + } // add the "source" at the found lineNumberWhereInject originalContent = append(originalContent[:lineNumberWhereInject], append(source, originalContent[lineNumberWhereInject:]...)...) diff --git a/ordering/main_test.go b/ordering/main_test.go index 1a44c61..176050e 100644 --- a/ordering/main_test.go +++ b/ordering/main_test.go @@ -268,10 +268,6 @@ func (f *Foo) FooMethod4() {} // orphan comment 1 here -func main() { - fmt.Println("nothing") -} - // orphan comment 2 here type Foo struct{} @@ -284,15 +280,19 @@ func (f *Foo) FooMethod3() {} func (f *Foo) FooMethod4() {} +func main() { + fmt.Println("nothing") +} + // foo comment func foo() { } -// orphan comment 3 here - // bar comment func bar() { } + +// orphan comment 3 here ` content, err := ReorderSource(ReorderConfig{ diff --git a/ordering/parser.go b/ordering/parser.go index 84586d5..4b4499a 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -1,6 +1,7 @@ package ordering import ( + "fmt" "go/ast" "go/parser" "go/token" @@ -22,6 +23,7 @@ type GoType struct { // ParsedInfo contains information we need to sort in the source file. type ParsedInfo struct { + Functions map[string]*GoType Methods map[string][]*GoType Constructors map[string][]*GoType Structs map[string]*GoType @@ -65,6 +67,7 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { var ( methods = make(map[string][]*GoType) + functions = make(map[string]*GoType) constructors = make(map[string][]*GoType) structTypes = make(map[string]*GoType) structNames = &StingList{} @@ -75,7 +78,10 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { if src == nil { // error should never happen as Parse() worked - sourceCode, _ = ioutil.ReadFile(filename) + sourceCode, err = ioutil.ReadFile(filename) + if err != nil { + return nil, err + } } else { sourceCode = src.([]byte) } @@ -101,8 +107,16 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { findConstructors(d, fset, sourceLines, methods, constructors) } } + // and now functions + for _, decl := range f.Decls { + switch d := decl.(type) { + case *ast.FuncDecl: + findFunctions(d, fset, sourceLines, functions, constructors) + } + } return &ParsedInfo{ + Functions: functions, Structs: structTypes, StructNames: structNames, Methods: methods, @@ -197,10 +211,11 @@ func findConstructors(d *ast.FuncDecl, fset *token.FileSet, sourceLines []string continue } returnType := exp.X.(*ast.Ident).Name - if _, ok := methods[returnType]; !ok { - // not a contructor for detected types above, skip - continue - } + // Bug: constructors are not detected if the type is not a method receiver + //if _, ok := methods[returnType]; !ok { + // // not a contructor for detected types above, skip + // continue + //} // Create a new method method := &GoType{ Name: d.Name.Name, @@ -228,6 +243,7 @@ func findGlobalVarsAndConsts(d *ast.GenDecl, fset *token.FileSet, sourceLines [] for _, spec := range d.Specs { if s, ok := spec.(*ast.ValueSpec); ok { for _, name := range s.Names { + // log the source code for the variable or constant typeDef := &GoType{ Name: name.Name, OpeningLine: fset.Position(d.Pos()).Line, @@ -238,13 +254,61 @@ func findGlobalVarsAndConsts(d *ast.GenDecl, fset *token.FileSet, sourceLines [] "\n" + strings.Join(sourceLines[typeDef.OpeningLine-1:typeDef.ClosingLine], "\n") typeDef.OpeningLine -= len(comments) + + // this time, if const or vars are defined in a parenthesis, the source code is the same for all + // found var or const. So, what we do is to check if the source code is already in the map, and if + // so, we skip it. + // we will use the source code signature as the key for the map + signature := fmt.Sprintf("%d-%d", typeDef.OpeningLine, typeDef.ClosingLine) + if _, ok := varTypes[signature]; ok { + continue + } + switch d.Tok { case token.CONST: - constTypes[name.Name] = typeDef + constTypes[signature] = typeDef case token.VAR: - varTypes[name.Name] = typeDef + varTypes[signature] = typeDef } } } } } + +func findFunctions(d *ast.FuncDecl, fset *token.FileSet, sourceLines []string, functions map[string]*GoType, constructors map[string][]*GoType) { + if d.Recv != nil { + return // because it's a method + } + if d.Name == nil { + return + } + if d.Name.Name == "" { + return + } + + if inConstructors(constructors, d.Name.Name) { + return + } + + functions[d.Name.Name] = &GoType{ + Name: d.Name.Name, + OpeningLine: fset.Position(d.Pos()).Line, + ClosingLine: fset.Position(d.End()).Line, + } + comments := GetMethodComments(d) + functions[d.Name.Name].SourceCode = strings.Join(comments, "\n") + + "\n" + + strings.Join(sourceLines[functions[d.Name.Name].OpeningLine-1:functions[d.Name.Name].ClosingLine], "\n") + functions[d.Name.Name].OpeningLine -= len(comments) +} + +func inConstructors(constructorMap map[string][]*GoType, funcname string) bool { + for _, constructors := range constructorMap { + for _, constructor := range constructors { + if constructor.Name == funcname { + return true + } + } + } + return false +} diff --git a/ordering/parser_test.go b/ordering/parser_test.go index 1fe0240..6cbb85a 100644 --- a/ordering/parser_test.go +++ b/ordering/parser_test.go @@ -50,10 +50,10 @@ func TestParse(t *testing.T) { if parsed == nil { t.Error("Expected parsed info") } - if len(parsed.Constants) != 4 { + if len(parsed.Constants) != 3 { // There is 3 blocs, not 4 consts t.Errorf("Expected 4 consts, got %d", len(parsed.Constants)) } - if len(parsed.Variables) != 4 { + if len(parsed.Variables) != 3 { // There is 3 blocs, not 4 vars t.Errorf("Expected 4 vars, got %d", len(parsed.Variables)) } if len(parsed.Structs) != 3 { From 1551ddbea9d54982588eb1c943216bd0e7930b0c Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Fri, 23 Jun 2023 10:38:39 +0200 Subject: [PATCH 13/14] Use goreorder on goreorder :) And it worked ! --- ordering/parser.go | 48 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/ordering/parser.go b/ordering/parser.go index 4b4499a..983b9c3 100644 --- a/ordering/parser.go +++ b/ordering/parser.go @@ -32,30 +32,6 @@ type ParsedInfo struct { StructNames *StingList } -// GetMethodComments returns the comments for the given method. -func GetMethodComments(d *ast.FuncDecl) (comments []string) { - if d == nil || d.Doc == nil || d.Doc.List == nil { - return - } - - for _, comment := range d.Doc.List { - comments = append(comments, comment.Text) - } - return -} - -// GetTypeComments returns the comments for the given type. -func GetTypeComments(d *ast.GenDecl) (comments []string) { - if d == nil || d.Doc == nil || d.Doc.List == nil { - return - } - - for _, comment := range d.Doc.List { - comments = append(comments, comment.Text) - } - return -} - // Parse the given file and return the methods, constructors and structs. func Parse(filename string, src interface{}) (*ParsedInfo, error) { fset := token.NewFileSet() @@ -126,6 +102,30 @@ func Parse(filename string, src interface{}) (*ParsedInfo, error) { }, nil } +// GetMethodComments returns the comments for the given method. +func GetMethodComments(d *ast.FuncDecl) (comments []string) { + if d == nil || d.Doc == nil || d.Doc.List == nil { + return + } + + for _, comment := range d.Doc.List { + comments = append(comments, comment.Text) + } + return +} + +// GetTypeComments returns the comments for the given type. +func GetTypeComments(d *ast.GenDecl) (comments []string) { + if d == nil || d.Doc == nil || d.Doc.List == nil { + return + } + + for _, comment := range d.Doc.List { + comments = append(comments, comment.Text) + } + return +} + func findStructs(d *ast.GenDecl, fset *token.FileSet, sourceLines []string, stuctNames *StingList, structTypes map[string]*GoType) { if d.Tok != token.TYPE { return From c342bb9ba7224369537f1ab63d04cea7184499f1 Mon Sep 17 00:00:00 2001 From: Patrice Ferlet Date: Fri, 23 Jun 2023 10:57:36 +0200 Subject: [PATCH 14/14] Forgotten to sort functions... --- ordering/main.go | 11 +++++++++-- ordering/main_test.go | 8 ++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/ordering/main.go b/ordering/main.go index 71e5476..c409db6 100644 --- a/ordering/main.go +++ b/ordering/main.go @@ -64,6 +64,12 @@ func ReorderSource(opt ReorderConfig) (string, error) { }) } + functionNames := make([]string, 0, len(info.Functions)) + for functionName := range info.Functions { + functionNames = append(functionNames, functionName) + } + sort.Strings(functionNames) + if opt.ReorderStructs { info.StructNames.Sort() } @@ -131,9 +137,10 @@ func ReorderSource(opt ReorderConfig) (string, error) { } removedLines += len(info.Methods[typename]) } - for i, sourceCode := range info.Functions { + for _, name := range functionNames { + sourceCode := info.Functions[name] if removedLines == 0 { - lineNumberWhereInject = info.Functions[i].OpeningLine + lineNumberWhereInject = info.Functions[name].OpeningLine } for ln := sourceCode.OpeningLine - 1; ln < sourceCode.ClosingLine; ln++ { originalContent[ln] = "// -- " + sign diff --git a/ordering/main_test.go b/ordering/main_test.go index 176050e..762366d 100644 --- a/ordering/main_test.go +++ b/ordering/main_test.go @@ -280,16 +280,16 @@ func (f *Foo) FooMethod3() {} func (f *Foo) FooMethod4() {} -func main() { - fmt.Println("nothing") +// bar comment +func bar() { } // foo comment func foo() { } -// bar comment -func bar() { +func main() { + fmt.Println("nothing") } // orphan comment 3 here