From bafb4908c30562d21df38501d0dcd40988182a1c Mon Sep 17 00:00:00 2001 From: pebers Date: Fri, 8 Apr 2016 11:24:33 +0100 Subject: [PATCH 1/6] Update imports to use builtin versions of packages from 1.5 since relevant bits of x/tools have gone away (see https://groups.google.com/forum/#!topic/golang-announce/qu_rAphYdxY). --- parser/parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/parser.go b/parser/parser.go index ee1111c..9b8852b 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -17,7 +17,7 @@ import ( "strings" "go/constant" - "go/importer" + _ "go/importer" "go/types" ) From fb3be6d28ae94f9bf80f12904d11c18b5bcb6355 Mon Sep 17 00:00:00 2001 From: pebers Date: Fri, 8 Apr 2016 11:33:16 +0100 Subject: [PATCH 2/6] Need to update these paths too. --- jsonenums.go | 2 +- server/server.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jsonenums.go b/jsonenums.go index cf71df2..4635852 100644 --- a/jsonenums.go +++ b/jsonenums.go @@ -75,7 +75,7 @@ import ( "path/filepath" "strings" - "github.com/campoy/jsonenums/parser" + "github.com/thought-machine/jsonenums/parser" ) var ( diff --git a/server/server.go b/server/server.go index 20e5b8b..2110205 100644 --- a/server/server.go +++ b/server/server.go @@ -21,7 +21,7 @@ import ( "go/format" - "github.com/campoy/jsonenums/parser" + "github.com/thought-machine/jsonenums/parser" ) func init() { From 0c6ac79cc6108a657873380b0a4c903fe811113a Mon Sep 17 00:00:00 2001 From: pebers Date: Fri, 8 Apr 2016 11:41:22 +0100 Subject: [PATCH 3/6] Need to pass an explicit importer --- parser/parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parser/parser.go b/parser/parser.go index 9b8852b..ee1111c 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -17,7 +17,7 @@ import ( "strings" "go/constant" - _ "go/importer" + "go/importer" "go/types" ) From 7fbe89f60da342dc2870277795492f310e38b8c5 Mon Sep 17 00:00:00 2001 From: Benjamin Pflanz Date: Thu, 21 Apr 2016 09:13:48 -0400 Subject: [PATCH 4/6] Use static parsing instead of requiring a binary. Using go/parser requires a built binary of the code to be enum'ed to exist. This leads to two problems: 1. The binary may be very out of date with the code, for example, when pulling into an existing repo that was last built locally some time ago, running jsonenums on newly added files will fail; 2. If the binary has never been built, but the code already expects the output of jsonenums, it will be impossible to generate the enums, making it impossible to build the binary. In response to a similar issue, Alan Donovan of the Go team suggested that instead generating tools should be using (the not-yet stable) x/tools/go/loader package to parse the actual Go code itself rather than the compilation output: https://github.com/golang/go/issues/11415 This change uses the suggested approach to break the bootstrapping dependency cycle. --- jsonenums.go | 13 ++++++++---- parser/parser.go | 53 +++++++++++++++--------------------------------- server/server.go | 2 +- 3 files changed, 26 insertions(+), 42 deletions(-) diff --git a/jsonenums.go b/jsonenums.go index 4635852..51d780c 100644 --- a/jsonenums.go +++ b/jsonenums.go @@ -75,7 +75,7 @@ import ( "path/filepath" "strings" - "github.com/thought-machine/jsonenums/parser" + "github.com/adfin/jsonenums/parser" ) var ( @@ -92,14 +92,19 @@ func main() { types := strings.Split(*typeNames, ",") // Only one directory at a time can be processed, and the default is ".". - dir := "." + in_dir := "." if args := flag.Args(); len(args) == 1 { - dir = args[0] + in_dir = args[0] } else if len(args) > 1 { log.Fatalf("only one directory at a time") } + dir, err := filepath.Abs(in_dir) + if err != nil { + log.Fatalf("Unable to determine absolute filepath for requested path %s: %v", + in_dir, err) + } - pkg, err := parser.ParsePackage(dir, *outputPrefix, *outputSuffix+".go") + pkg, err := parser.ParsePackage(dir) if err != nil { log.Fatalf("parsing package: %v", err) } diff --git a/parser/parser.go b/parser/parser.go index ee1111c..26c524b 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -10,15 +10,14 @@ import ( "fmt" "go/ast" "go/build" - "go/parser" + "go/constant" "go/token" + "go/types" "log" "path/filepath" "strings" - "go/constant" - "go/importer" - "go/types" + "golang.org/x/tools/go/loader" ) // A Package contains all the information related to a parsed package. @@ -30,45 +29,25 @@ type Package struct { } // ParsePackage parses the package in the given directory and returns it. -func ParsePackage(directory, skipPrefix, skipSuffix string) (*Package, error) { - pkgDir, err := build.Default.ImportDir(directory, 0) +func ParsePackage(directory string) (*Package, error) { + relDir, err := filepath.Rel(filepath.Join(build.Default.GOPATH, "src"), directory) if err != nil { - return nil, fmt.Errorf("cannot process directory %s: %s", directory, err) + return nil, fmt.Errorf("provided directory not under GOPATH (%s): %v", + build.Default.GOPATH, err) } - var files []*ast.File - fs := token.NewFileSet() - for _, name := range pkgDir.GoFiles { - if !strings.HasSuffix(name, ".go") || - (skipSuffix != "" && strings.HasPrefix(name, skipPrefix) && - strings.HasSuffix(name, skipSuffix)) { - continue - } - if directory != "." { - name = filepath.Join(directory, name) - } - f, err := parser.ParseFile(fs, name, nil, 0) - if err != nil { - return nil, fmt.Errorf("parsing file %v: %v", name, err) - } - files = append(files, f) - } - if len(files) == 0 { - return nil, fmt.Errorf("%s: no buildable Go files", directory) - } - - // type-check the package - defs := make(map[*ast.Ident]types.Object) - config := types.Config{FakeImportC: true, Importer: importer.Default()} - info := &types.Info{Defs: defs} - if _, err := config.Check(directory, fs, files, info); err != nil { - return nil, fmt.Errorf("type-checking package: %v", err) + loaderConf := loader.Config{TypeChecker: types.Config{FakeImportC: true}} + loaderConf.Import(relDir) + program, err := loaderConf.Load() + if err != nil { + return nil, fmt.Errorf("Couldn't load package: %v", err) } + pkgInfo := program.Package(relDir) return &Package{ - Name: files[0].Name.Name, - files: files, - defs: defs, + Name: pkgInfo.Pkg.Name(), + files: pkgInfo.Files, + defs: pkgInfo.Defs, }, nil } diff --git a/server/server.go b/server/server.go index 2110205..c15810b 100644 --- a/server/server.go +++ b/server/server.go @@ -21,7 +21,7 @@ import ( "go/format" - "github.com/thought-machine/jsonenums/parser" + "github.com/adfin/jsonenums/parser" ) func init() { From b89a8c9df8b6b1172b7d92be099a81b91f0076e0 Mon Sep 17 00:00:00 2001 From: Benjamin Pflanz Date: Tue, 3 May 2016 18:50:41 -0400 Subject: [PATCH 5/6] Correct imports and variable naming. --- jsonenums.go | 10 +++++----- server/server.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/jsonenums.go b/jsonenums.go index 51d780c..507ecb7 100644 --- a/jsonenums.go +++ b/jsonenums.go @@ -75,7 +75,7 @@ import ( "path/filepath" "strings" - "github.com/adfin/jsonenums/parser" + "github.com/campoy/jsonenums/parser" ) var ( @@ -92,16 +92,16 @@ func main() { types := strings.Split(*typeNames, ",") // Only one directory at a time can be processed, and the default is ".". - in_dir := "." + inDir := "." if args := flag.Args(); len(args) == 1 { - in_dir = args[0] + inDir = args[0] } else if len(args) > 1 { log.Fatalf("only one directory at a time") } - dir, err := filepath.Abs(in_dir) + dir, err := filepath.Abs(inDir) if err != nil { log.Fatalf("Unable to determine absolute filepath for requested path %s: %v", - in_dir, err) + inDir, err) } pkg, err := parser.ParsePackage(dir) diff --git a/server/server.go b/server/server.go index c15810b..20e5b8b 100644 --- a/server/server.go +++ b/server/server.go @@ -21,7 +21,7 @@ import ( "go/format" - "github.com/adfin/jsonenums/parser" + "github.com/campoy/jsonenums/parser" ) func init() { From d79b636dff87fe5120f5bdb71874d532f4764591 Mon Sep 17 00:00:00 2001 From: Benjamin Pflanz Date: Wed, 11 May 2016 12:02:21 -0400 Subject: [PATCH 6/6] Use idiomatic error messaging and variable naming, and don't make unnecessary changes. --- jsonenums.go | 10 +++++----- parser/parser.go | 8 ++++---- server/server.go | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/jsonenums.go b/jsonenums.go index 507ecb7..60346ee 100644 --- a/jsonenums.go +++ b/jsonenums.go @@ -92,16 +92,16 @@ func main() { types := strings.Split(*typeNames, ",") // Only one directory at a time can be processed, and the default is ".". - inDir := "." + dir := "." if args := flag.Args(); len(args) == 1 { - inDir = args[0] + dir = args[0] } else if len(args) > 1 { log.Fatalf("only one directory at a time") } - dir, err := filepath.Abs(inDir) + dir, err := filepath.Abs(dir) if err != nil { - log.Fatalf("Unable to determine absolute filepath for requested path %s: %v", - inDir, err) + log.Fatalf("unable to determine absolute filepath for requested path %s: %v", + dir, err) } pkg, err := parser.ParsePackage(dir) diff --git a/parser/parser.go b/parser/parser.go index 26c524b..bc1d3eb 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -36,11 +36,11 @@ func ParsePackage(directory string) (*Package, error) { build.Default.GOPATH, err) } - loaderConf := loader.Config{TypeChecker: types.Config{FakeImportC: true}} - loaderConf.Import(relDir) - program, err := loaderConf.Load() + conf := loader.Config{TypeChecker: types.Config{FakeImportC: true}} + conf.Import(relDir) + program, err := conf.Load() if err != nil { - return nil, fmt.Errorf("Couldn't load package: %v", err) + return nil, fmt.Errorf("couldn't load package: %v", err) } pkgInfo := program.Package(relDir) diff --git a/server/server.go b/server/server.go index 20e5b8b..b7bb514 100644 --- a/server/server.go +++ b/server/server.go @@ -47,7 +47,7 @@ func generateHandler(w http.ResponseWriter, r *http.Request) error { } defer os.RemoveAll(dir) - pkg, err := parser.ParsePackage(dir, "", "") + pkg, err := parser.ParsePackage(dir) if err != nil { return fmt.Errorf("parse package: %v", err) }