From 378b8492a54ae876518b3beee55a8d84de5ce402 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 27 Jan 2020 10:45:26 +0100 Subject: [PATCH 1/4] Cleanup of main() code Signed-off-by: Andres Taylor --- go/visitorgen/main/main.go | 68 +++++++++++++++----------------------- 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/go/visitorgen/main/main.go b/go/visitorgen/main/main.go index ae09ad7efdf..3ba2bca047b 100644 --- a/go/visitorgen/main/main.go +++ b/go/visitorgen/main/main.go @@ -17,12 +17,17 @@ limitations under the License. package main import ( + "bytes" "flag" "fmt" "go/parser" "go/token" + "io/ioutil" "os" + "vitess.io/vitess/go/exit" + "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/visitorgen" ) @@ -33,28 +38,24 @@ var ( const usage = `Usage of visitorgen: -go run go/visitorgen/main/main.go -input=/path/to/ast.go -output=/path/to/rewriter.go +go run ./go/visitorgen/main -input=/path/to/ast.go -output=/path/to/rewriter.go ` func main() { - flag.Usage = func() { - os.Stderr.WriteString(usage) - os.Stderr.WriteString("\nOptions:\n") - flag.PrintDefaults() - - } + defer exit.Recover() + flag.Usage = printUsage flag.Parse() if *inputFile == "" || *outputFile == "" { - fmt.Println("> " + *inputFile) - fmt.Println("> " + *outputFile) - panic("need input and output file") + printUsage() + exit.Return(1) } fs := token.NewFileSet() file, err := parser.ParseFile(fs, *inputFile, nil, parser.DeclarationErrors) if err != nil { - panic(err) + log.Error(err) + exit.Return(1) } astWalkResult := visitorgen.Walk(file) @@ -64,42 +65,25 @@ func main() { replacementMethods := visitorgen.EmitReplacementMethods(vd) typeSwitch := visitorgen.EmitTypeSwitches(vd) - fw := newFileWriter(*outputFile) - defer fw.Close() - - fw.writeln(fileHeader) - fw.writeln(replacementMethods) - fw.write(applyHeader) - fw.writeln(typeSwitch) - fw.writeln(fileFooter) -} - -type fileWriter struct { - file *os.File -} - -func newFileWriter(file string) *fileWriter { - f, err := os.Create(file) - if err != nil { - panic(err) - } - return &fileWriter{file: f} -} - -func (fw *fileWriter) writeln(s string) { - fw.write(s) - fw.write("\n") -} + b := &bytes.Buffer{} + fmt.Fprint(b, fileHeader) + fmt.Fprintln(b) + fmt.Fprintln(b, replacementMethods) + fmt.Fprint(b, applyHeader) + fmt.Fprintln(b, typeSwitch) + fmt.Fprintln(b, fileFooter) -func (fw *fileWriter) write(s string) { - _, err := fw.file.WriteString(s) + err = ioutil.WriteFile(*outputFile, b.Bytes(), 0644) if err != nil { - panic(err) + log.Error(err) + exit.Return(1) } } -func (fw *fileWriter) Close() { - fw.file.Close() +func printUsage() { + os.Stderr.WriteString(usage) + os.Stderr.WriteString("\nOptions:\n") + flag.PrintDefaults() } const fileHeader = `// Code generated by visitorgen/main/main.go. DO NOT EDIT. From e581793d462cb17dd27503e5aeb9c9c0c6df2ce7 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 27 Jan 2020 11:16:37 +0100 Subject: [PATCH 2/4] Clean up Makefile and pre-commit check Signed-off-by: Andres Taylor --- Makefile | 4 +--- go/visitorgen/main/main.go | 24 +++++++++++++++++++----- go/vt/sqlparser/rewriter.go | 2 +- misc/git/hooks/visitorgen | 9 +-------- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index acf5363a00b..b71fa046b99 100644 --- a/Makefile +++ b/Makefile @@ -89,9 +89,7 @@ parser: make -C go/vt/sqlparser visitor: - go build -o visitorgen go/visitorgen/main/main.go - ./visitorgen -input=go/vt/sqlparser/ast.go -output=$(REWRITER) - rm ./visitorgen + go generate go/vt/sqlparser/rewriter.go # To pass extra flags, run test.go manually. # For example: go run test.go -docker=false -- --extra-flag diff --git a/go/visitorgen/main/main.go b/go/visitorgen/main/main.go index 3ba2bca047b..e51bce9396d 100644 --- a/go/visitorgen/main/main.go +++ b/go/visitorgen/main/main.go @@ -34,6 +34,7 @@ import ( var ( inputFile = flag.String("input", "", "input file to use") outputFile = flag.String("output", "", "output file") + compare = flag.Bool("compareOnly", false, "instead of writing to the output file, compare if the generated visitor is still valid for this ast.go") ) const usage = `Usage of visitorgen: @@ -73,11 +74,24 @@ func main() { fmt.Fprintln(b, typeSwitch) fmt.Fprintln(b, fileFooter) - err = ioutil.WriteFile(*outputFile, b.Bytes(), 0644) - if err != nil { - log.Error(err) - exit.Return(1) + if *compare { + currentFile, err := ioutil.ReadFile(*outputFile) + if err != nil { + log.Error(err) + exit.Return(1) + } + if !bytes.Equal(b.Bytes(), currentFile) { + fmt.Println("rewriter needs to be re-generated: go generate " + *outputFile) + exit.Return(1) + } + } else { + err = ioutil.WriteFile(*outputFile, b.Bytes(), 0644) + if err != nil { + log.Error(err) + exit.Return(1) + } } + } func printUsage() { @@ -90,7 +104,7 @@ const fileHeader = `// Code generated by visitorgen/main/main.go. DO NOT EDIT. package sqlparser -//go:generate make visitor +//go:generate go run ../../visitorgen/main -input=ast.go -output=rewriter.go import ( "reflect" diff --git a/go/vt/sqlparser/rewriter.go b/go/vt/sqlparser/rewriter.go index 28d5ba52688..2f8da297fed 100644 --- a/go/vt/sqlparser/rewriter.go +++ b/go/vt/sqlparser/rewriter.go @@ -2,7 +2,7 @@ package sqlparser -//go:generate make visitor +//go:generate go run ../../visitorgen/main -input=ast.go -output=rewriter.go import ( "reflect" diff --git a/misc/git/hooks/visitorgen b/misc/git/hooks/visitorgen index 7eb1c41360f..5fa203e9c79 100755 --- a/misc/git/hooks/visitorgen +++ b/misc/git/hooks/visitorgen @@ -15,11 +15,4 @@ # this script, which should run before committing code, makes sure that the visitor is re-generated when the ast changes -make visitor REWRITER=tmp_rewriter.go -if ! cmp -s "tmp_rewriter.go" "go/vt/sqlparser/rewriter.go"; then - echo "The ast.go has changed, but not rewriter.go" - echo "You should 'make visitor' to update the generated rewriter" - rm -f tmp_rewriter.go - exit 1 -fi -rm -f tmp_rewriter.go \ No newline at end of file +go run ./go/visitorgen/main -compareOnly=true -input=go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go \ No newline at end of file From 86a3ce61ff3e780c45dbcfc809b3aa3eef066d07 Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 27 Jan 2020 11:25:39 +0100 Subject: [PATCH 3/4] Moved visitorgen to be next to the AST Signed-off-by: Andres Taylor --- go/vt/sqlparser/rewriter.go | 2 +- go/{ => vt/sqlparser}/visitorgen/ast_walker.go | 0 go/{ => vt/sqlparser}/visitorgen/ast_walker_test.go | 0 go/{ => vt/sqlparser}/visitorgen/main/main.go | 6 +++--- go/{ => vt/sqlparser}/visitorgen/sast.go | 0 go/{ => vt/sqlparser}/visitorgen/struct_producer.go | 0 go/{ => vt/sqlparser}/visitorgen/struct_producer_test.go | 0 go/{ => vt/sqlparser}/visitorgen/transformer.go | 0 go/{ => vt/sqlparser}/visitorgen/transformer_test.go | 0 go/{ => vt/sqlparser}/visitorgen/visitor_emitter.go | 0 go/{ => vt/sqlparser}/visitorgen/visitor_emitter_test.go | 0 go/{ => vt/sqlparser}/visitorgen/visitorgen.go | 0 misc/git/hooks/visitorgen | 2 +- 13 files changed, 5 insertions(+), 5 deletions(-) rename go/{ => vt/sqlparser}/visitorgen/ast_walker.go (100%) rename go/{ => vt/sqlparser}/visitorgen/ast_walker_test.go (100%) rename go/{ => vt/sqlparser}/visitorgen/main/main.go (94%) rename go/{ => vt/sqlparser}/visitorgen/sast.go (100%) rename go/{ => vt/sqlparser}/visitorgen/struct_producer.go (100%) rename go/{ => vt/sqlparser}/visitorgen/struct_producer_test.go (100%) rename go/{ => vt/sqlparser}/visitorgen/transformer.go (100%) rename go/{ => vt/sqlparser}/visitorgen/transformer_test.go (100%) rename go/{ => vt/sqlparser}/visitorgen/visitor_emitter.go (100%) rename go/{ => vt/sqlparser}/visitorgen/visitor_emitter_test.go (100%) rename go/{ => vt/sqlparser}/visitorgen/visitorgen.go (100%) diff --git a/go/vt/sqlparser/rewriter.go b/go/vt/sqlparser/rewriter.go index 2f8da297fed..2f6c22879fe 100644 --- a/go/vt/sqlparser/rewriter.go +++ b/go/vt/sqlparser/rewriter.go @@ -2,7 +2,7 @@ package sqlparser -//go:generate go run ../../visitorgen/main -input=ast.go -output=rewriter.go +//go:generate go run visitorgen/main -input=ast.go -output=rewriter.go import ( "reflect" diff --git a/go/visitorgen/ast_walker.go b/go/vt/sqlparser/visitorgen/ast_walker.go similarity index 100% rename from go/visitorgen/ast_walker.go rename to go/vt/sqlparser/visitorgen/ast_walker.go diff --git a/go/visitorgen/ast_walker_test.go b/go/vt/sqlparser/visitorgen/ast_walker_test.go similarity index 100% rename from go/visitorgen/ast_walker_test.go rename to go/vt/sqlparser/visitorgen/ast_walker_test.go diff --git a/go/visitorgen/main/main.go b/go/vt/sqlparser/visitorgen/main/main.go similarity index 94% rename from go/visitorgen/main/main.go rename to go/vt/sqlparser/visitorgen/main/main.go index e51bce9396d..5b53b3d268f 100644 --- a/go/visitorgen/main/main.go +++ b/go/vt/sqlparser/visitorgen/main/main.go @@ -28,7 +28,7 @@ import ( "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" - "vitess.io/vitess/go/visitorgen" + "vitess.io/vitess/go/vt/sqlparser/visitorgen" ) var ( @@ -39,7 +39,7 @@ var ( const usage = `Usage of visitorgen: -go run ./go/visitorgen/main -input=/path/to/ast.go -output=/path/to/rewriter.go +go run /path/to/visitorgen/main -input=/path/to/ast.go -output=/path/to/rewriter.go ` func main() { @@ -104,7 +104,7 @@ const fileHeader = `// Code generated by visitorgen/main/main.go. DO NOT EDIT. package sqlparser -//go:generate go run ../../visitorgen/main -input=ast.go -output=rewriter.go +//go:generate go run visitorgen/main -input=ast.go -output=rewriter.go import ( "reflect" diff --git a/go/visitorgen/sast.go b/go/vt/sqlparser/visitorgen/sast.go similarity index 100% rename from go/visitorgen/sast.go rename to go/vt/sqlparser/visitorgen/sast.go diff --git a/go/visitorgen/struct_producer.go b/go/vt/sqlparser/visitorgen/struct_producer.go similarity index 100% rename from go/visitorgen/struct_producer.go rename to go/vt/sqlparser/visitorgen/struct_producer.go diff --git a/go/visitorgen/struct_producer_test.go b/go/vt/sqlparser/visitorgen/struct_producer_test.go similarity index 100% rename from go/visitorgen/struct_producer_test.go rename to go/vt/sqlparser/visitorgen/struct_producer_test.go diff --git a/go/visitorgen/transformer.go b/go/vt/sqlparser/visitorgen/transformer.go similarity index 100% rename from go/visitorgen/transformer.go rename to go/vt/sqlparser/visitorgen/transformer.go diff --git a/go/visitorgen/transformer_test.go b/go/vt/sqlparser/visitorgen/transformer_test.go similarity index 100% rename from go/visitorgen/transformer_test.go rename to go/vt/sqlparser/visitorgen/transformer_test.go diff --git a/go/visitorgen/visitor_emitter.go b/go/vt/sqlparser/visitorgen/visitor_emitter.go similarity index 100% rename from go/visitorgen/visitor_emitter.go rename to go/vt/sqlparser/visitorgen/visitor_emitter.go diff --git a/go/visitorgen/visitor_emitter_test.go b/go/vt/sqlparser/visitorgen/visitor_emitter_test.go similarity index 100% rename from go/visitorgen/visitor_emitter_test.go rename to go/vt/sqlparser/visitorgen/visitor_emitter_test.go diff --git a/go/visitorgen/visitorgen.go b/go/vt/sqlparser/visitorgen/visitorgen.go similarity index 100% rename from go/visitorgen/visitorgen.go rename to go/vt/sqlparser/visitorgen/visitorgen.go diff --git a/misc/git/hooks/visitorgen b/misc/git/hooks/visitorgen index 5fa203e9c79..65c04d613db 100755 --- a/misc/git/hooks/visitorgen +++ b/misc/git/hooks/visitorgen @@ -15,4 +15,4 @@ # this script, which should run before committing code, makes sure that the visitor is re-generated when the ast changes -go run ./go/visitorgen/main -compareOnly=true -input=go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go \ No newline at end of file +go run ./go/vt/sqlparser/visitorgen/main -compareOnly=true -input=go/vt/sqlparser/ast.go -output=go/vt/sqlparser/rewriter.go \ No newline at end of file From 2e66f6f0dd97117691ff90006fd9867b0b1d5b4b Mon Sep 17 00:00:00 2001 From: Andres Taylor Date: Mon, 27 Jan 2020 19:54:33 +0100 Subject: [PATCH 4/4] Add check that the visitor has been updated to the CI build Signed-off-by: Andres Taylor --- test/config.json | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/config.json b/test/config.json index a61ce65eab0..e493c43cac8 100644 --- a/test/config.json +++ b/test/config.json @@ -38,6 +38,17 @@ "RetryMax": 1, "Tags": [] }, + "check_make_visitor": { + "File": "", + "Args": [], + "Command": [ + "misc/git/hooks/visitorgen" + ], + "Manual": false, + "Shard": 5, + "RetryMax": 1, + "Tags": [] + }, "java": { "File": "", "Args": [],