We were recursing infinitely in cases like this:
switch true {
case true:
go foo.F<>
}
There were three things that came together to cause this:
1. We recently starting recursively fixing broken go/defer statements.
2. In this case we were failing to swap in the correct ast.Node in for
the *ast.BadStmt because we were only looking
for *ast.BlockStmt (and *ast.CaseStmt has no block).
3. After 2), we weren't returning an error so the fix() code thought
it should recurse.
Fix 2) by using reflection to swap AST nodes in a generic way. Perhaps
a bit overkill in this case, but I happened to have already written
this for an upcoming change, so I just pulled it in to fix this bug.
Fix 3) by returning an error if we fail to swap the AST nodes.
Fixesgolang/go#34353.
Change-Id: I17ff1afd52ae165c0ba9de5820dcec4cb7d756cb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196137
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Now when trying to fix *ast.BadStmt, we parse the manually extracted
expression using parser.ParseFile instead of parser.ParseExpr.
ParseFile will yield *ast.BadStmt nodes for any bad statements nested
in our first bad statement, allowing us to fix them recursively.
To turn our expression into a "valid" file we can pass to
parser.ParseFile, I wrapped it thusly:
package fake
func _() {
<our expression>
}
Change-Id: I0d4fd4ebce6450021da8e03caa11d0ae5152ea8d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194342
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Improve the existing fix-the-AST code to better identify the
expression following the "go" or "defer" keywords:
- Don't slurp the expression start outside the loop since the
expression might only have a single token.
- Set expression end to the position after the final token, not the
position of the final token.
- Track curly brace nesting to properly capture an entire "func() {}"
expression.
- Fix parent node detection to work when BadStmt isn't first statement
of block.
- Add special case to detect dangling period, e.g. "defer fmt.". We
insert phantom "_" selectors like go/parser does to prevent the
dangling "." from messing up the AST.
- Use reflect in offsetPositions so it updates positions in all node
types. This code shouldn't be called often, so I don't think
performance is a concern.
I also tweaked the function snippet code so it properly expands
"defer" and "go" expressions to function calls. It thought it didn't
have to expand since there was already a *ast.CallExpr, but the
CallExpr was faked by us and the source doesn't actually contain the
"()" calling parens.
Note that this does not work for nested go/defer statements. For
example, completions won't work properly in cases like this:
go func() {
defer fmt.<>
}
I think we can fix this as well with some more work.
Change-Id: I8f9753fda76909b0e3a83489cdea69ad04ee237a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/193997
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change adds support for completion of incomplete selectors after a
defer or go statement. We modify the AST before type-checking it with a
fake *ast.CallExpr.
Updates golang/go#29313
Change-Id: Ic9e8c9c49aa569cd7874791692c70a28c3146251
Reviewed-on: https://go-review.googlesource.com/c/tools/+/172974
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
