Modified AST is weirdly indented after formatting

[AndreasLinz]

I am working a small refactoring tool that allows to replace values of specific struct fields in a Go repository. Modifying the AST works and the appropriate fields are replaced, but after replacing the original source file with the formatted AST, the replaced part is weirdly indented.

I wrote a minimal example that demonstrates the issue in this playground: https://go.dev/play/p/je4wZaGRZjq.

Any idea on how to fix this is appreciated since I’m was out of luck and could not find a solution to this problem.

[skillian (Sean Killian)]

It might be a little silly, but can you just run gofmt on the result?

[AndreasLinz]

No, that’s not a silly question. The output is actually properly gofmt’ed, even though it’s weirdly indented.
For clarification, running gofmt will keep the indentantion.

[skillian (Sean Killian)]

What!? I couldn’t believe you until I ran it, and sure enough, you’re right! That is unfortunate. I don’t know what to do about it! I just tried messing with the whitespace in the rawReplacement, but it didn’t fix anything.

I just tried gofumpt which produced what I consider more consistent output:

package main

import (
        "fmt"
        "strings"
)

func main() {
        s := struct{ A string }{
                A: strings.Join(
                        []string{
                                "Hello",
                                " World!",
                        },
                        ","),
        }

        fmt.Println(s.A)
}

[AndreasLinz]

That leaves me with the question if I should open a ticket on github.com/golang/go because it might be a bug? What makes me wonder is, if this is a bug, then someone else should already have had the problem since refactoring by AST is nothing totally new.

[AndreasLinz]

I opened an issue on GitHub if anybody wants to follow: https://github.com/golang/go/issues/49846.

[telo_tade]

Hi @AndreasLinz ,

The issue is not with AST replacement, your replacement works just fine. The node you create, with which you replace that field, is wrongly formatted (wrongly created) from its initial creation. Keep in mind you are creating an ast Node, then replacing nodes in a tree with this newly created node; not replacing a string with another string.

This talk of replacement adds another layer of obfuscation to your question. In reality your question is:
Node you produce:

     0  *ast.CallExpr {
     1  .  Fun: *ast.SelectorExpr {
     2  .  .  X: *ast.Ident {
     3  .  .  .  NamePos: example.go:1:1
     4  .  .  .  Name: "strings"
     5  .  .  }
     6  .  .  Sel: *ast.Ident {
     7  .  .  .  NamePos: example.go:1:9
     8  .  .  .  Name: "Join"
     9  .  .  }
    10  .  }
    11  .  Lparen: example.go:1:13
    12  .  Args: []ast.Expr (len = 2) {
    13  .  .  0: *ast.CompositeLit {
    14  .  .  .  Type: *ast.ArrayType {
    15  .  .  .  .  Lbrack: example.go:2:1
    16  .  .  .  .  Elt: *ast.Ident {
    17  .  .  .  .  .  NamePos: example.go:3:2
    18  .  .  .  .  .  Name: "string"
    19  .  .  .  .  }
    20  .  .  .  }
    21  .  .  .  Lbrace: example.go:3:8
    22  .  .  .  Elts: []ast.Expr (len = 2) {
    23  .  .  .  .  0: *ast.BasicLit {
    24  .  .  .  .  .  ValuePos: example.go:3:9
    25  .  .  .  .  .  Kind: STRING
    26  .  .  .  .  .  Value: "\"Hello\""
    27  .  .  .  .  }
    28  .  .  .  .  1: *ast.BasicLit {
    29  .  .  .  .  .  ValuePos: example.go:5:4
    30  .  .  .  .  .  Kind: STRING
    31  .  .  .  .  .  Value: "\"World!\""
    32  .  .  .  .  }
    33  .  .  .  }
    34  .  .  .  Rbrace: example.go:5:12
    35  .  .  .  Incomplete: false
    36  .  .  }
    37  .  .  1: *ast.BasicLit {
    38  .  .  .  ValuePos: example.go:6:1
    39  .  .  .  Kind: STRING
    40  .  .  .  Value: "\",\""
    41  .  .  }
    42  .  }
    43  .  Ellipsis: -
    44  .  Rparen: example.go:6:4
    45  } 

Actually expected node:

    94  .  .  .  .  .  .  .  .  .  .  Value: *ast.CallExpr {
    95  .  .  .  .  .  .  .  .  .  .  .  Fun: *ast.SelectorExpr {
    96  .  .  .  .  .  .  .  .  .  .  .  .  X: *ast.Ident {
    97  .  .  .  .  .  .  .  .  .  .  .  .  .  NamePos: example.go:7:6
    98  .  .  .  .  .  .  .  .  .  .  .  .  .  Name: "strings"
    99  .  .  .  .  .  .  .  .  .  .  .  .  }
   100  .  .  .  .  .  .  .  .  .  .  .  .  Sel: *ast.Ident {
   101  .  .  .  .  .  .  .  .  .  .  .  .  .  NamePos: example.go:7:14
   102  .  .  .  .  .  .  .  .  .  .  .  .  .  Name: "Join"
   103  .  .  .  .  .  .  .  .  .  .  .  .  }
   104  .  .  .  .  .  .  .  .  .  .  .  }
   105  .  .  .  .  .  .  .  .  .  .  .  Lparen: example.go:7:18
   106  .  .  .  .  .  .  .  .  .  .  .  Args: []ast.Expr (len = 2) {
   107  .  .  .  .  .  .  .  .  .  .  .  .  0: *ast.CompositeLit {
   108  .  .  .  .  .  .  .  .  .  .  .  .  .  Type: *ast.ArrayType {
   109  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Lbrack: example.go:7:19
   110  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Elt: *ast.Ident {
   111  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  NamePos: example.go:7:21
   112  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Name: "string"
   113  .  .  .  .  .  .  .  .  .  .  .  .  .  .  }
   114  .  .  .  .  .  .  .  .  .  .  .  .  .  }
   115  .  .  .  .  .  .  .  .  .  .  .  .  .  Lbrace: example.go:7:27
   116  .  .  .  .  .  .  .  .  .  .  .  .  .  Elts: []ast.Expr (len = 2) {
   117  .  .  .  .  .  .  .  .  .  .  .  .  .  .  0: *ast.BasicLit {
   118  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  ValuePos: example.go:7:28
   119  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Kind: STRING
   120  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Value: "\"Hello\""
   121  .  .  .  .  .  .  .  .  .  .  .  .  .  .  }
   122  .  .  .  .  .  .  .  .  .  .  .  .  .  .  1: *ast.BasicLit {
   123  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  ValuePos: example.go:7:36
   124  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Kind: STRING
   125  .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  Value: "\" World!\""
   126  .  .  .  .  .  .  .  .  .  .  .  .  .  .  }
   127  .  .  .  .  .  .  .  .  .  .  .  .  .  }
   128  .  .  .  .  .  .  .  .  .  .  .  .  .  Rbrace: example.go:7:45
   129  .  .  .  .  .  .  .  .  .  .  .  .  .  Incomplete: false
   130  .  .  .  .  .  .  .  .  .  .  .  .  }
   131  .  .  .  .  .  .  .  .  .  .  .  .  1: *ast.BasicLit {
   132  .  .  .  .  .  .  .  .  .  .  .  .  .  ValuePos: example.go:7:48
   133  .  .  .  .  .  .  .  .  .  .  .  .  .  Kind: STRING
   134  .  .  .  .  .  .  .  .  .  .  .  .  .  Value: "\",\""
   135  .  .  .  .  .  .  .  .  .  .  .  .  }
   136  .  .  .  .  .  .  .  .  .  .  .  }
   137  .  .  .  .  .  .  .  .  .  .  .  Ellipsis: -
   138  .  .  .  .  .  .  .  .  .  .  .  Rparen: example.go:7:51
   139  .  .  .  .  .  .  .  .  .  .  }

The difference between them is lines (17 and 29) versus (118 and 123). These two lines (17 and 29) in your node are wrong.

I do not fully understand why they are wrong, it is related to the fact that you are not parsing a file but and expression, and seems normal. Not a bug of any kind.

Solution to the issue is to correctly produce the replacement node. You are already very close to it being correct, why not produce it just like now, then modify it (line 17 and 29) to become what you actually expect from it ?

If this does not work, then you could try this: write a small golang file containing the `strings.Join([]string{“Hello”, " World!"}, “,”)``, parse that file to an ast tree, identify the node for this expression and use that.

Hope you manage to fix this, and share your results.

[AndreasLinz]

@telo_tade Thank you for the detailed answer!

Solution to the issue is to correctly produce the replacement node. You are already very close to it being correct, why not produce it just like now, then modify it (line 17 and 29) to become what you actually expect from it ?

The playground I shared is a simplified version of the tool I’m working on which reads a Go expression from a file and uses that to replace some node in an AST with it. So, I cannot make any assumption about the expression I’m getting and thus cannot modify it.

The difference between them is lines (17 and 29) versus (118 and 123). These two lines (17 and 29) in your node are wrong.

 17 .  .  .  .  .  NamePos: example.go:3:2
 29 .  .  .  .  .  ValuePos: example.go:5:4
118 .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  ValuePos: example.go:7:28
123 .  .  .  .  .  .  .  .  .  .  .  .  .  .  .  ValuePos: example.go:7:36

I do not think that I understand yet why those specific lines are the problem. However, I will also print those ASTs using ast.Print to debug the problem.

Thanks again!

[AndreasLinz]

@telo_tade I you pointed me in the right direction because I just found the source of the problem

What I changed is that I replaced parser.ParseExpr with parser.ParseExprFrom. The former function is just a wrapper around parser.ParseExprFrom and passes in an empty token.FileSet. This way the AST nodes of the replacement got assigned to the wrong positions, they got assigned to lines from example.go, after they were inserted the tree. The formatter seems to be aware of the position in the information, which makes total sense, and hence the output is formatted strangely. Using parser.ParseExprFrom and passing in the existing fset fixes the position information issue and the formatting one as well.

	replacement, err := parser.ParseExprFrom(fset, "replacement.go", string(rawReplacement), parser.AllErrors|parser.ParseComments)

https://go.dev/play/p/PzwikCXTqzB

[telo_tade]

This is great, I am glad you found the fix !