Fix file name reporting for Go 1.11 [v2] #31
Conversation
unconvert.go
Outdated
| @@ -43,6 +44,7 @@ func apply(file string, edits editSet) { | |||
| } | |||
|
|
|||
| fset := token.NewFileSet() | |||
| fmt.Printf("file: %q\n", file) | |||
There was a problem hiding this comment.
Oops, I've missed it, thanks for catching this. Removed.
| @@ -425,7 +427,13 @@ func (v *visitor) unconvert(call *ast.CallExpr) { | |||
| return | |||
| } | |||
|
|
|||
| v.edits[v.file.Position(call.Lparen)] = struct{}{} | |||
| pos := v.file.Position(call.Lparen) | |||
| if !filepath.IsAbs(pos.Filename) { // go 1.11+, see https://github.com/golang/go/issues/26671 | |||
There was a problem hiding this comment.
For go<1.11 filename will be a/b.go, but in go1.11 filename will be b.go. Absolute path checking doesn’t help.
IMO it should be fixed in go/scanner: it’s an incompatible change and we need to fix dozens of tools because of it
There was a problem hiding this comment.
For go<1.11 filename will be a/b.go
I don't think so. See golang/go#26671, the code (removed by golang/go@546bab8) was:
if filename != "" {
 filename = filepath.Clean(filename)
 if !filepath.IsAbs(filename) {
 // make filename relative to current directory
 filename = filepath.Join(s.dir, filename)Here s.dir is the path from the original file name supplied to scanner.Init(), and it could either be relative or absolute. Due to a way unconvert handles input (using golang.org/x/tools/go/loader) file names supplied are absolute.
The above it theory; my tests confirm it.
unconvert output is broken when using Go 1.11. A quick example: With Go 1.10: > go run unconvert.go github.com/docker/docker/daemon/graphdriver/copy > /home/kir/go/src/github.com/docker/docker/daemon/graphdriver/copy/copy.go:242:28: unnecessary conversion With Go 1.11 beta 2: > go run unconvert.go github.com/docker/docker/daemon/graphdriver/copy > copy.go:242:0: unnecessary conversion Note that there are two issues: - column is 0; - file names have path stripped. The issue with column is filed as golang/go#26692; there is no way to fix it in unconvert. The second issue is currently discussed in golang/go#26671, but it might not be fixed since the new documentation says: > The filenames in line directives now remain untouched by the scanner; > there is no cleanup or conversion of relative into absolute paths > anymore, in sync with what the compiler's scanner/parser are doing. > Any kind of filename transformation has to be done by a client. This > makes the scanner code simpler and also more predictable. So, here's the alternative: if the file name is not absolute, prepend the path as we know it. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
|
OK, since the golang/go#26671 is now closed, looks like this patch is the way to go forward. |
|
golang/go#26671 is fixed; closing this one |
|
Thanks all for engaging upstream on this issue. |
unconvert output for cgo-preprocessed files is broken when using Go 1.11beta. A quick example:
With Go 1.10:
With Go 1.11 beta2:
Note that there are two issues:
The issue with column is filed as golang/go#26692;
there is no way to fix it in unconvert.
The second issue is currently discussed in
golang/go#26671, but it might not be
fixed since the new documentation says:
So, here's the alternative: if the file name is not absolute, prepend
the path as we know it.
Fixes #29