gofmt is a code formatter, I don't think it should be required to handle invalid go code, much less change it to correct errors.

It also goes down a slippery slope. What syntax errors should be fixed? Where do you draw the line?

How about adding a tool like a goimports? It is a code fixer for import statements.

gofixtrailingcommas ? Mmh...

Honestly I'm not sure it would meet the bar for inclusion in x/tools/cmd; but you can always publish it as an open source project. I don't think it'll be used as widely as goimports though, this is a much smaller problem.

I agree that adding new tool like this is bike-shedding... But is there any chance to add more generic tool like go fix? For example, go fix imports does the same as goimports. go fix trailling-comma fixes commas, ...

but you can always publish it as an open source project.

Hmm... I can create a fixer which fixes something before passing a code to gofmt. I think I should try it (although parsing twice and spawning a process for gofmt may be not good for performance).

How about adding a new option to ignore parse error if no bad node is in AST? I tried modifying gofmt code as below.

https://github.com/rhysd/gofmtrlx

rhysd/gofmtrlx@76c93b2

I found that there is no bad node in AST even if trailing comma error occurs. So, as gofmt is formatter, it can ignore the parse error, just format the code even if parse error is contained when there is no bad node in AST. How do you think?

btw, gofmt already trims trailing , in function call.

$ gofmt -d test.go
diff test.go gofmt/test.go
--- /tmp/gofmt596094646 2017-02-04 23:43:27.384131425 +0900
+++ /tmp/gofmt073783965 2017-02-04 23:43:27.384131425 +0900
@@ -7,7 +7,7 @@
                3,
        }

-       f(1, 2, )
+       f(1, 2)
 }

I think it's reasonable to add missing trailing comma to some extent considering this behavior.
Especially if we can draw a reasonable line with detecting ast.Bad* node.
It's also really useful.

@haya14busa note that what you're describing there is transforming a valid program into another valid program.

Oh, that's true. I didn't know it's valid syntax because I'm always using gofmt before compile.

But my main point is, if go/parser can correctly created ast node, why not using it instead of discarding it with trivial error.

@rhysd I think that gofmt ignore the syntax error is not good. I'm not sure but, how about ignoring the error in your editor or something using gofmt? For example, vim-go may be possible to ignore the output of gofmt by filtering that error. missing ',' before newline in composite literal

@mattn If ignoring the error, I can't know why formatting failed and nothing was formatted. I don't want to stop formatting.

After all, what I want to say is what @haya14busa said (thanks @haya14busa). Even if there is a parse error, Go's parser can parse it. gofmt is not a syntax checker. IMO gofmt should be able to format every code which parser can parse without bad AST node.

Please note that I don't say this should be default behavior. But I feel there should be an option for this behavior.

Even though adding the new option to ignore bad AST node, gofmt should exit by non zero, I think. But it can output verbosy syntax warnings.

@mattn yeah, it should show warnings and return non-zero exit code if there was warning. Thank you for the point. I agree 👍

@rhysd BTW, what is your motivation for this?

@mattn

From description of this issue:

we tend to forget adding trailing comma when expose one line code to multi-line code.

I needed to fix trailing comma errors while writing a code. I thought gofmt might be able to fix them automatically. It was the original point I made this issue.

I thought gofmt might be able to fix it automatically. It was the original point I made this issue.

It will be more approval than this proposal, I guess. 👍

Whether there's a bad node in AST is not necessarily indicative of how close the file was to real Go. It sounds like the actual request is to insert missing end-of-line commas.

If the input said:

 []int{1, 2
     +3, 4}

then inserting a comma after 2 could possibly be breaking the program instead of helpfully fixing it. This seems undeniably convenient, but also dangerous. And it opens a slippery slope for other "help".

Also, gofmt is the "cat" of Go programs. There can be other programs that process them instead. Not everything has to go into gofmt.

I've been assuming that editor integration would make it easy to jump from the error message to the exact place noted in the error message, and the message itself says "put a comma here". That seems pretty direct and easy. If any editor integrations do not make it easy to jump from error messages to the named locations, please file bugs against them.

-rsc for @golang/proposal-review

@rsc Ah, you're correct. I could not come up with the edge case. The edge case can't be fixed only by filtering error message hence it looks hard to fix.

Thank you all for discussion.

blah.go:6:4: missing ',' before newline in composite literal

expectClosing seems to be called after finished to parse element list of composite literal. thus, I guess we can change the message for this like missing '}' in composite literal.