From 73c7173a9f7dc30db1bb8ac66d4dc5ba652bc44e Mon Sep 17 00:00:00 2001
From: Muir Manders <muir@mnd.rs>
Date: Wed, 4 Dec 2019 19:50:46 -0800
Subject: [PATCH] internal/lsp: fix AST bookkeeping as we repair nodes

We weren't maintaining our ancestor node list correctly. This caused
us to fail to make AST repairs in certain cases. Now we are careful to
always append to the ancestors list when recursing.

Updates golang/go#34332.

Change-Id: I9b51ec70572170d9f592060d264c98b1f9720fb8
Reviewed-on: https://go-review.googlesource.com/c/tools/+/209966
Run-TryBot: Muir Manders <muir@mnd.rs>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
---
 internal/lsp/cache/parse.go                   | 23 +++++++++++--------
 .../keywords/accidental_keywords.go.in        |  6 +++++
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index 3ea1d9763e..2cee62e776 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -203,20 +203,25 @@ func isEllipsisArray(n ast.Expr) bool {
 func fix(ctx context.Context, n ast.Node, tok *token.File, src []byte) error {
 	var (
 		ancestors []ast.Node
-		parent    ast.Node
 		err       error
 	)
-	ast.Inspect(n, func(n ast.Node) bool {
-		if n == nil {
-			if len(ancestors) > 0 {
-				ancestors = ancestors[:len(ancestors)-1]
-				if len(ancestors) > 0 {
-					parent = ancestors[len(ancestors)-1]
-				}
+	ast.Inspect(n, func(n ast.Node) (recurse bool) {
+		defer func() {
+			if recurse {
+				ancestors = append(ancestors, n)
 			}
+		}()
+
+		if n == nil {
+			ancestors = ancestors[:len(ancestors)-1]
 			return false
 		}
 
+		var parent ast.Node
+		if len(ancestors) > 0 {
+			parent = ancestors[len(ancestors)-1]
+		}
+
 		switch n := n.(type) {
 		case *ast.BadStmt:
 			err = fixDeferOrGoStmt(n, parent, tok, src) // don't shadow err
@@ -260,8 +265,6 @@ func fix(ctx context.Context, n ast.Node, tok *token.File, src []byte) error {
 			fixPhantomSelector(n, tok, src)
 			return true
 		default:
-			ancestors = append(ancestors, n)
-			parent = n
 			return true
 		}
 	})
diff --git a/internal/lsp/testdata/keywords/accidental_keywords.go.in b/internal/lsp/testdata/keywords/accidental_keywords.go.in
index 1ae8ff39d4..22ad4e2948 100644
--- a/internal/lsp/testdata/keywords/accidental_keywords.go.in
+++ b/internal/lsp/testdata/keywords/accidental_keywords.go.in
@@ -4,16 +4,22 @@ package keywords
 var apple = "apple"
 
 func _() {
+	foo.bar() // insert some extra statements to excercise our AST surgery
 	variance := 123 //@item(kwVariance, "variance", "int", "var")
+	foo.bar()
 	println(var) //@complete(")", kwVariance)
 }
 
 func _() {
+	foo.bar()
 	var s struct { variance int } //@item(kwVarianceField, "variance", "int", "field")
+	foo.bar()
 	s.var //@complete(" //", kwVarianceField)
 }
 
 func _() {
+	foo.bar()
 	var typeName string //@item(kwTypeName, "typeName", "string", "var")
+	foo.bar()
 	type //@complete(" //", kwTypeName)
 }