Need advice on AST formatting

[Denis Cheremisov]

Hi!

I have a utility that parses Go files into AST , makes some changes on the tree (import paths) and format it back into the Go source code, in the original file.

The problem is I am using format.Node function from this package. The formatting this function applies to the AST is very similar to what gofmt does but it is not identical. 

Example.

The original file may look like this

// +build !windows

package main

import (
    "<import-path>"
)

func main() {}

And the result will look like

// +build !windows
package main

import (
    "<new-import-path>"
)

func main() {}

See the 2nd line of the original: it is blank. And it disappears in the result. And I don't touch neither the Package token of the AST, neither the leading comment.

What AST into source printer can I use instead to preserve that blank line?

[Michel Levieux]

Hi Denis,

In a project of my own I encountered the same issue. This program moves a go project to another location, then it remembers imports (that are necessary), exported types, functions and package-scope values (var / const) and maps them in the original go project to the newly created one. The original goal of this program is to move packages from other packages, while leaving everything compatible as is. Everything almost works now but in practice, all my code has a strange look, like function/type definitions are all stuck with one another, with no lines in between.

If anyone has suggestions or ideas, please let us know!

Thanks to all of you for your time.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/50b2e8fe-e017-403e-a2d8-f44cfd86f20a%40googlegroups.com.

[Denis Cheremisov]

The answer was simple: 

var fset token.FileSet – wrong

fset := token.NewFileSet() – right

вторник, 22 октября 2019 г., 11:20:38 UTC+3 пользователь Michel Levieux написал:

Hi Denis,

In a project of my own I encountered the same issue. This program moves a go project to another location, then it remembers imports (that are necessary), exported types, functions and package-scope values (var / const) and maps them in the original go project to the newly created one. The original goal of this program is to move packages from other packages, while leaving everything compatible as is. Everything almost works now but in practice, all my code has a strange look, like function/type definitions are all stuck with one another, with no lines in between.

If anyone has suggestions or ideas, please let us know!

Thanks to all of you for your time.

Le ven. 18 oct. 2019 à 00:11, Denis Cheremisov <denis.c...@gmail.com> a écrit :

Hi!

I have a utility that parses Go files into AST , makes some changes on the tree (import paths) and format it back into the Go source code, in the original file.

The problem is I am using format.Node function from this package. The formatting this function applies to the AST is very similar to what gofmt does but it is not identical. 

Example.

The original file may look like this

// +build !windows

package main

import (
    "<import-path>"
)

func main() {}

And the result will look like

// +build !windows
package main

import (
    "<new-import-path>"
)

func main() {}

See the 2nd line of the original: it is blank. And it disappears in the result. And I don't touch neither the Package token of the AST, neither the leading comment.

What AST into source printer can I use instead to preserve that blank line?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.

[Marvin Renich]

* Denis Cheremisov <denis.ch...@gmail.com> [191026 06:09]:

> The answer was simple:
>
> var fset token.FileSet – wrong
>
> fset := token.NewFileSet() – right

I believe I am in the minority here, but I am not a singleton minority.
There are at least several of us who agree that «var» should be used
over «:=» whenever possible, and when you do use «:=», you should be
very careful, because it is frequently the cause of hidden shadowing
bugs. This is especially true when declaring multiple variables.

I strongly encourage you to use

var fset = token.NewFileSet()

rather than

fset := token.NewFileSet()

...Marvin

[Michel Levieux]

Oh, well I'm glad it solved the problem for you Denis! Unfortunately, it seems mine's somewhere else since I was already using token.NewFileSet()...

I think I might have to "re-handle" the spacing logic within my AST since I'm changing many many things and it's possible that I just break the formatting by copying things around. I'll tell you guys if ever I find something that fixes my case.

@Marvin, I'd say I don't agree with you. Your point is interesting, but in my opinion using one construct over another is mainly subjective. If the "var" construct is safer in terms of hard-to-find bugs, the ":=" construct tends to improve readability a lot, especially when there are lots and lots of variables involved. Moreover, when juggling with constructs such as "something, err := func()", it is really convenient to not have to think too much about whether "err" wad already declared higher in the block. Again, I'm not arguing because I can understand that people might prefer the way around, just stating an opinion here :)

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/20191027025214.uqkd4rtu6gbrbt5b%40basil.wdw.

[roger peppe]

Those two alternatives do exactly the same thing AFAIK. How is the latter vulnerable to shadowing bugs where the former isn't?

...Marvin

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/20191027025214.uqkd4rtu6gbrbt5b%40basil.wdw.

[Marvin Renich]

* roger peppe <rogp...@gmail.com> [191028 04:49]:

> On Sun, 27 Oct 2019, 02:52 Marvin Renich, <mr...@renich.org> wrote:
> > I strongly encourage you to use
> >
> > var fset = token.NewFileSet()
> >
> > rather than
> >
> > fset := token.NewFileSet()
> >
>
> Those two alternatives do exactly the same thing AFAIK. How is the latter
> vulnerable to shadowing bugs where the former isn't?

In this case they are identical. It is the multi-variable "perhaps"
redefinition that is the problem. Making a habit of using "var"
whenever possible allows the compiler to call out where you have
unintentionally shadowed one of several variables.

With the multi-variable short declaration, you cannot tell which
variables are being declared and which variables are "along for the
ride" without _careful_ examination of the code all the way back to the
beginning of the block. If you use "var", it is explicit at that
statement.

blk, msg, err := widget.Frabulate(arg)

Which of blk, msg, and err are being newly declared here? You cannot
tell.

var blk Thingle
var msg string
blk, msg, err = widget.Frabulate(arg)

Absolutely no question here; no need to examine earlier code. And if
msg had been used earlier in this block, you would be told. This also
has the distinct advantage that now the types of blk and msg are
explicit. In the first case, someone reading the code would first have
to determine what widget is (package or var) and then find the
declaration of Frabulate to determine the types.

I assert that in a large codebase, the second form is significantly
easier to maintain than the first.

Furthermore, := only saves three characters over using var if you are
not mixing new declarations with simple assignment. This is
insignificant. And var has significantly less cognitive load for those
of us who have used many languages over the years, where := does not
have a universally accepted meaning across languages.

And, yes, I am aware that there are cases where you must use ":=". I
wish that the design of the multi-variable short declaration had not
only provided for but required that the variables that the programmer
intended to declare were explicitly identified. Perhaps Go2 will
address this.

...Marvin