Merge pull request #7726 from planetscale/rewriter-closure

making fast AST rewriter faster
vitessio · Mar 23, 2021 · 3428d57 · 3428d57
2 parents 683d5c8 + ed6b0e5
commit 3428d57
Show file tree

Hide file tree

Showing 19 changed files with 1,869 additions and 1,863 deletions.
diff --git a/go/tools/asthelpergen/integration/ast_helper.go b/go/tools/asthelpergen/integration/ast_helper.go
diff --git a/go/tools/asthelpergen/integration/integration_rewriter_test.go b/go/tools/asthelpergen/integration/integration_rewriter_test.go
@@ -34,8 +34,7 @@ func TestRewriteVisitRefContainer(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(containerContainer, tv.pre, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, tv.pre, tv.post)
 
 	expected := []step{
 		Pre{containerContainer},
@@ -58,8 +57,7 @@ func TestRewriteVisitValueContainer(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(containerContainer, tv.pre, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, tv.pre, tv.post)
 
 	expected := []step{
 		Pre{containerContainer},
@@ -84,8 +82,7 @@ func TestRewriteVisitRefSliceContainer(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(containerContainer, tv.pre, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, tv.pre, tv.post)
 
 	tv.assertEquals(t, []step{
 		Pre{containerContainer},
@@ -113,8 +110,7 @@ func TestRewriteVisitValueSliceContainer(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(containerContainer, tv.pre, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, tv.pre, tv.post)
 
 	tv.assertEquals(t, []step{
 		Pre{containerContainer},
@@ -150,8 +146,7 @@ func TestRewriteVisitInterfaceSlice(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(ast, tv.pre, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(ast, tv.pre, tv.post)
 
 	tv.assertEquals(t, []step{
 		Pre{ast},
@@ -176,22 +171,20 @@ func TestRewriteVisitRefContainerReplace(t *testing.T) {
 	}
 
 	// rewrite field of type AST
-	_, err := Rewrite(ast, func(cursor *Cursor) bool {
+	_ = Rewrite(ast, func(cursor *Cursor) bool {
 		leaf, ok := cursor.node.(*RefContainer)
 		if ok && leaf.NotASTType == 12 {
 			cursor.Replace(&Leaf{99})
 		}
 		return true
 	}, nil)
-	require.NoError(t, err)
 
 	assert.Equal(t, &RefContainer{
 		ASTType:               &Leaf{99},
 		ASTImplementationType: &Leaf{2},
 	}, ast)
 
-	_, err = Rewrite(ast, rewriteLeaf(2, 55), nil)
-	require.NoError(t, err)
+	_ = Rewrite(ast, rewriteLeaf(2, 55), nil)
 
 	assert.Equal(t, &RefContainer{
 		ASTType:               &Leaf{99},
@@ -200,20 +193,25 @@ func TestRewriteVisitRefContainerReplace(t *testing.T) {
 }
 
 func TestRewriteVisitValueContainerReplace(t *testing.T) {
+
 	ast := ValueContainer{
 		ASTType:               ValueContainer{NotASTType: 12},
 		ASTImplementationType: &Leaf{2},
 	}
 
-	_, err := Rewrite(ast, func(cursor *Cursor) bool {
+	defer func() {
+		if r := recover(); r != nil {
+			require.Equal(t, "[BUG] tried to replace 'ASTType' on 'ValueContainer'", r)
+		}
+	}()
+	_ = Rewrite(ast, func(cursor *Cursor) bool {
 		leaf, ok := cursor.node.(ValueContainer)
 		if ok && leaf.NotASTType == 12 {
 			cursor.Replace(&Leaf{99})
 		}
 		return true
 	}, nil)
 
-	require.Error(t, err)
 }
 
 func TestRewriteVisitValueContainerReplace2(t *testing.T) {
@@ -222,8 +220,12 @@ func TestRewriteVisitValueContainerReplace2(t *testing.T) {
 		ASTImplementationType: &Leaf{2},
 	}
 
-	_, err := Rewrite(ast, rewriteLeaf(2, 10), nil)
-	require.Error(t, err)
+	defer func() {
+		if r := recover(); r != nil {
+			require.Equal(t, "[BUG] tried to replace 'ASTImplementationType' on 'ValueContainer'", r)
+		}
+	}()
+	_ = Rewrite(ast, rewriteLeaf(2, 10), nil)
 }
 
 func TestRewriteVisitRefContainerPreOrPostOnly(t *testing.T) {
@@ -234,8 +236,7 @@ func TestRewriteVisitRefContainerPreOrPostOnly(t *testing.T) {
 
 	tv := &rewriteTestVisitor{}
 
-	_, err := Rewrite(containerContainer, tv.pre, nil)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, tv.pre, nil)
 	tv.assertEquals(t, []step{
 		Pre{containerContainer},
 		Pre{container},
@@ -244,8 +245,7 @@ func TestRewriteVisitRefContainerPreOrPostOnly(t *testing.T) {
 	})
 
 	tv = &rewriteTestVisitor{}
-	_, err = Rewrite(containerContainer, nil, tv.post)
-	require.NoError(t, err)
+	_ = Rewrite(containerContainer, nil, tv.post)
 	tv.assertEquals(t, []step{
 		Post{leaf1},
 		Post{leaf2},
@@ -270,16 +270,14 @@ func TestRefSliceContainerReplace(t *testing.T) {
 		ASTImplementationElements: []*Leaf{{3}, {4}},
 	}
 
-	_, err := Rewrite(ast, rewriteLeaf(2, 42), nil)
-	require.NoError(t, err)
+	_ = Rewrite(ast, rewriteLeaf(2, 42), nil)
 
 	assert.Equal(t, &RefSliceContainer{
 		ASTElements:               []AST{&Leaf{1}, &Leaf{42}},
 		ASTImplementationElements: []*Leaf{{3}, {4}},
 	}, ast)
 
-	_, err = Rewrite(ast, rewriteLeaf(3, 88), nil)
-	require.NoError(t, err)
+	_ = Rewrite(ast, rewriteLeaf(3, 88), nil)
 
 	assert.Equal(t, &RefSliceContainer{
 		ASTElements:               []AST{&Leaf{1}, &Leaf{42}},

diff --git a/go/tools/asthelpergen/integration/test_helpers.go b/go/tools/asthelpergen/integration/test_helpers.go
@@ -39,8 +39,10 @@ func sliceStringLeaf(els ...*Leaf) string {
 
 // the methods below are what the generated code expected to be there in the package
 
+// ApplyFunc is apply function
 type ApplyFunc func(*Cursor) bool
 
+// Cursor is cursor
 type Cursor struct {
 	parent   AST
 	replacer replacerFunc
@@ -62,20 +64,18 @@ func (c *Cursor) Replace(newNode AST) {
 
 type replacerFunc func(newNode, parent AST)
 
-func Rewrite(node AST, pre, post ApplyFunc) (AST, error) {
+// Rewrite is the api.
+func Rewrite(node AST, pre, post ApplyFunc) AST {
 	outer := &struct{ AST }{node}
 
 	a := &application{
 		pre:  pre,
 		post: post,
 	}
 
-	err := a.rewriteAST(outer, node, func(newNode, parent AST) {
+	a.rewriteAST(outer, node, func(newNode, parent AST) {
 		outer.AST = newNode
 	})
 
-	if err != nil {
-		return nil, err
-	}
-	return outer.AST, nil
+	return outer.AST
 }
diff --git a/go/tools/asthelpergen/integration/types.go b/go/tools/asthelpergen/integration/types.go
@@ -174,8 +174,6 @@ func (r *NoCloneType) String() string {
 
 type Visit func(node AST) (bool, error)
 
-var errAbort = fmt.Errorf("this error is to abort the rewriter, it is not an actual error")
-
 type application struct {
 	pre, post ApplyFunc
 	cur       Cursor