search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.27k stars 17.7k forks source link

x/tools/cmd/gopls: Formatting action results in file contents removal #31854

Closed nezorflame closed 5 years ago

nezorflame commented 5 years ago

What version of Go are you using (go version)?

$ go version
go version go1.12.4 darwin/amd64

gopls commit hash

3b6f9c0030f769fa3ded8a7f0d9420c71ac20caf

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOOS="darwin"
GOARCH="amd64"

What did you do?

While using recommended gopls settings with latest vscode-go extension for VSCode, I'm using editor.formatOnPaste setting to format the code on Paste action. Now when I paste the code, the whole file contents get removed (except for the inserted part).

Example: https://www.giphy.com/gifs/XHFt4o5jQOGIkOPKqo

What did you expect to see?

Code should be formatted, without removal.

What did you see instead?

Complete code removal (except for the inserted code).

Problem analysis

It seems that the fix for the issue https://github.com/golang/go/issues/31797 introduced this behavior. More particularly, this commit: https://github.com/golang/tools/commit/9cb3dcf692a103de0fd68c26f4f04183e0933f7c

Example previously shown in GIF shows the problem at the internal/span/span.go:252 where the word hasPosition gets copied and pasted.

Here's the trace after the Paste action:

gopls trace
[Trace - 1:49:29 PM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go","version":6},"contentChanges":[{"text":"// Copyright 2019 The Go Authors. All rights reserved.\n// Use of this source code is governed by a BSD-style\n// license that can be found in the LICENSE file.\n\npackage span\n\nimport (\n\t\"encoding/json\"\n\t\"fmt\"\n\t\"path\"\n)\n\n// Span represents a source code range in standardized form.\ntype Span struct {\n\tv span\n}\n\n// Point represents a single point within a file.\n// In general this should only be used as part of a Span, as on its own it\n// does not carry enough information.\ntype Point struct {\n\tv point\n}\n\ntype span struct {\n\tURI   URI   `json:\"uri\"`\n\tStart point `json:\"start\"`\n\tEnd   point `json:\"end\"`\n}\n\ntype point struct {\n\tLine   int `json:\"line\"`\n\tColumn int `json:\"column\"`\n\tOffset int `json:\"offset\"`\n}\n\nvar invalidPoint = Point{v: point{Line: 0, Column: 0, Offset: -1}}\n\n// Converter is the interface to an object that can convert between line:column\n// and offset forms for a single file.\ntype Converter interface {\n\t//ToPosition converts from an offset to a line:column pair.\n\tToPosition(offset int) (int, int, error)\n\t//ToOffset converts from a line:column pair to an offset.\n\tToOffset(line, col int) (int, error)\n}\n\nfunc New(uri URI, start Point, end Point) Span {\n\ts := Span{v: span{URI: uri, Start: start.v, End: end.v}}\n\ts.v.clean()\n\treturn s\n}\n\nfunc NewPoint(line, col, offset int) Point {\n\tp := Point{v: point{Line: line, Column: col, Offset: offset}}\n\tp.v.clean()\n\treturn p\n}\n\nfunc Compare(a, b Span) int {\n\tif r := CompareURI(a.URI(), b.URI()); r != 0 {\n\t\treturn r\n\t}\n\tif r := comparePoint(a.v.Start, b.v.Start); r != 0 {\n\t\treturn r\n\t}\n\treturn comparePoint(a.v.End, b.v.End)\n}\n\nfunc ComparePoint(a, b Point) int {\n\treturn comparePoint(a.v, b.v)\n}\n\nfunc comparePoint(a, b point) int {\n\tif !a.hasPosition() {\n\t\tif a.Offset < b.Offset {\n\t\t\treturn -1\n\t\t}\n\t\tif a.Offset > b.Offset {\n\t\t\treturn 1\n\t\t}\n\t\treturn 0\n\t}\n\tif a.Line < b.Line {\n\t\treturn -1\n\t}\n\tif a.Line > b.Line {\n\t\treturn 1\n\t}\n\tif a.Column < b.Column {\n\t\treturn -1\n\t}\n\tif a.Column > b.Column {\n\t\treturn 1\n\t}\n\treturn 0\n}\n\nfunc (s Span) HasPosition() bool             { return s.v.Start.hasPosition() }\nfunc (s Span) HasOffset() bool               { return s.v.Start.hasOffset() }\nfunc (s Span) IsValid() bool                 { return s.v.Start.isValid() }\nfunc (s Span) IsPoint() bool                 { return s.v.Start == s.v.End }\nfunc (s Span) URI() URI                      { return s.v.URI }\nfunc (s Span) Start() Point                  { return Point{s.v.Start} }\nfunc (s Span) End() Point                    { return Point{s.v.End} }\nfunc (s *Span) MarshalJSON() ([]byte, error) { return json.Marshal(&s.v) }\nfunc (s *Span) UnmarshalJSON(b []byte) error { return json.Unmarshal(b, &s.v) }\n\nfunc (p Point) HasPosition() bool             { return p.v.hasPosition() }\nfunc (p Point) HasOffset() bool               { return p.v.hasOffset() }\nfunc (p Point) IsValid() bool                 { return p.v.isValid() }\nfunc (p *Point) MarshalJSON() ([]byte, error) { return json.Marshal(&p.v) }\nfunc (p *Point) UnmarshalJSON(b []byte) error { return json.Unmarshal(b, &p.v) }\nfunc (p Point) Line() int {\n\tif !p.v.hasPosition() {\n\t\tpanic(fmt.Errorf(\"position not set in %v\", p.v))\n\t}\n\treturn p.v.Line\n}\nfunc (p Point) Column() int {\n\tif !p.v.hasPosition() {\n\t\tpanic(fmt.Errorf(\"position not set in %v\", p.v))\n\t}\n\treturn p.v.Column\n}\nfunc (p Point) Offset() int {\n\tif !p.v.hasOffset() {\n\t\tpanic(fmt.Errorf(\"offset not set in %v\", p.v))\n\t}\n\treturn p.v.Offset\n}\n\nfunc (p point) hasPosition() bool { return p.Line > 0 }\nfunc (p point) hasOffset() bool   { return p.Offset >= 0 }\nfunc (p point) isValid() bool     { return p.hasPosition() || p.hasOffset() }\nfunc (p point) isZero() bool {\n\treturn (p.Line == 1 && p.Column == 1) || (!p.hasPosition() && p.Offset == 0)\n}\n\nfunc (s *span) clean() {\n\t//this presumes the points are already clean\n\tif !s.End.isValid() || (s.End == point{}) {\n\t\ts.End = s.Start\n\t}\n}\n\nfunc (p *point) clean() {\n\tif p.Line < 0 {\n\t\tp.Line = 0\n\t}\n\tif p.Column <= 0 {\n\t\tif p.Line > 0 {\n\t\t\tp.Column = 1\n\t\t} else {\n\t\t\tp.Column = 0\n\t\t}\n\t}\n\tif p.Offset == 0 && (p.Line > 1 || p.Column > 1) {\n\t\tp.Offset = -1\n\t}\n}\n\n// Format implements fmt.Formatter to print the Location in a standard form.\n// The format produced is one that can be read back in using Parse.\nfunc (s Span) Format(f fmt.State, c rune) {\n\tfullForm := f.Flag('+')\n\tpreferOffset := f.Flag('#')\n\t// we should always have a uri, simplify if it is file format\n\t//TODO: make sure the end of the uri is unambiguous\n\turi := string(s.v.URI)\n\tif c == 'f' {\n\t\turi = path.Base(uri)\n\t} else if !fullForm {\n\t\tif filename, err := s.v.URI.Filename(); err == nil {\n\t\t\turi = filename\n\t\t}\n\t}\n\tfmt.Fprint(f, uri)\n\tif !s.IsValid() || (!fullForm && s.v.Start.isZero() && s.v.End.isZero()) {\n\t\treturn\n\t}\n\t// see which bits of start to write\n\tprintOffset := s.HasOffset() && (fullForm || preferOffset || !s.HasPosition())\n\tprintLine := s.HasPosition() && (fullForm || !printOffset)\n\tprintColumn := printLine && (fullForm || (s.v.Start.Column > 1 || s.v.End.Column > 1))\n\tfmt.Fprint(f, \":\")\n\tif printLine {\n\t\tfmt.Fprintf(f, \"%d\", s.v.Start.Line)\n\t}\n\tif printColumn {\n\t\tfmt.Fprintf(f, \":%d\", s.v.Start.Column)\n\t}\n\tif printOffset {\n\t\tfmt.Fprintf(f, \"#%d\", s.v.Start.Offset)\n\t}\n\t// start is written, do we need end?\n\tif s.IsPoint() {\n\t\treturn\n\t}\n\t// we don't print the line if it did not change\n\tprintLine = fullForm || (printLine && s.v.End.Line > s.v.Start.Line)\n\tfmt.Fprint(f, \"-\")\n\tif printLine {\n\t\tfmt.Fprintf(f, \"%d\", s.v.End.Line)\n\t}\n\tif printColumn {\n\t\tif printLine {\n\t\t\tfmt.Fprint(f, \":\")\n\t\t}\n\t\tfmt.Fprintf(f, \"%d\", s.v.End.Column)\n\t}\n\tif printOffset {\n\t\tfmt.Fprintf(f, \"#%d\", s.v.End.Offset)\n\t}\n}\n\nfunc (s Span) WithPosition(c Converter) (Span, error) {\n\tif err := s.update(c, true, false); err != nil {\n\t\treturn Span{}, err\n\t}\n\treturn s, nil\n}\n\nfunc (s Span) WithOffset(c Converter) (Span, error) {\n\tif err := s.update(c, false, true); err != nil {\n\t\treturn Span{}, err\n\t}\n\treturn s, nil\n}\n\nfunc (s Span) WithAll(c Converter) (Span, error) {\n\tif err := s.update(c, true, true); err != nil {\n\t\treturn Span{}, err\n\t}\n\treturn s, nil\n}\n\nfunc (s *Span) update(c Converter, withPos, withOffset bool) error {\n\tif !s.IsValid() {\n\t\treturn fmt.Errorf(\"cannot add information to an invalid span\")\n\t}\n\tif withPos && !s.HasPosition() {\n\t\tif err := s.v.Start.updatePosition(c); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tif s.v.End.Offset == s.v.Start.Offset {\n\t\t\ts.v.End = s.v.Start\n\t\t} else if err := s.v.End.updatePosition(c); err != nil {\n\t\t\treturn err\n\t\t}\n\t}\n\tif withOffset && (!s.HasOffset() || (s.v.End.hasPosition() && !s.v.End.hasOffset())) {\n\t\tif err := s.v.Start.updateOffset(c); err != nil {\n\t\t\treturn err\n\t\t}\n\t\tif s.v.End.Line == s.v.Start.Line && s.v.End.Column == s.v.Start.Column {\n\t\t\ts.v.End.Offset = s.v.Start.Offset\n\t\t} else if err := s.v.End.updateOffset(c); err != nil {\n\t\t\treturn err\n\t\t}\n\t}\n\treturn nil\n}\n\nfunc (p *point) updatePosition(c Converter) error {\n\tline, col, err := c.ToPosition(p.Offset)\n\tif err != nil {\n\t\treturn err\n\t}\n\tp.Line = line\n\tp.Column = col\n\treturn nil\n}\n\nfunc (p *point) updateOffset(c Converter) error {\n\toffset, err := c.ToOffset(p.Line, p.Column)\n\tif err != nil {\n\t\treturn err\n\t}\n\tp.Offset = offset\n\treturn nil\n}\n"}]}

[Trace - 1:49:29 PM] Sending request 'textDocument/rangeFormatting - (17)'.
Params: {"textDocument":{"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go"},"range":{"start":{"line":251,"character":46},"end":{"line":251,"character":57}},"options":{"tabSize":4,"insertSpaces":false}}

[Trace - 1:49:29 PM] Received notification 'window/logMessage'.
Params: {"type":4,"message":"didChange: file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go"}

didChange: file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go
[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/token112.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/uri.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/utf16.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/parse.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/token.go","diagnostics":[]}

[Trace - 1:49:29 PM] Received response 'textDocument/rangeFormatting - (17)' in 127ms.
Params: [{"range":{"start":{"line":0,"character":0},"end":{"line":282,"character":0}},"newText":""},{"range":{"start":{"line":282,"character":0},"end":{"line":282,"character":0}},"newText":"hasPosition"}]

[Trace - 1:49:30 PM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go","version":7},"contentChanges":[{"text":"hasPosition"}]}

[Trace - 1:49:30 PM] Received notification 'window/logMessage'.
Params: {"type":4,"message":"didChange: file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go"}

didChange: file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go
[Trace - 1:49:30 PM] Sending request 'textDocument/codeAction - (18)'.
Params: {"textDocument":{"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go"},"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},"context":{"diagnostics":[]}}

[Trace - 1:49:30 PM] Sending request 'textDocument/documentSymbol - (19)'.
Params: {"textDocument":{"uri":"file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go"}}

[Trace - 1:49:31 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"invalid position for file "}

[Error - 13:49:31] invalid position for file 
[Trace - 1:49:31 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"invalid position for file "}

[Error - 13:49:31] invalid position for file 
[Trace - 1:49:31 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"invalid position for file "}

[Error - 13:49:31] invalid position for file 
[Trace - 1:49:31 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"invalid position for file "}

[Error - 13:49:31] invalid position for file 
[Error - 1:49:31 PM] send textDocument/codeAction#18 no file information for file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go

[Error - 13:49:31] Request textDocument/codeAction failed.
  Message: no file information for file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go
  Code: 0 
[Trace - 1:49:32 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"invalid position for file "}

[Error - 13:49:32] invalid position for file 
[Error - 1:49:32 PM] send textDocument/documentSymbol#19 no file information for file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go

[Error - 13:49:32] Request textDocument/documentSymbol failed.
  Message: no file information for file:///Users/idanilkin/Go/src/golang.org/x/tools/internal/span/span.go
  Code: 0
ianthehat commented 5 years ago

I believe that range formatting is totally broken right now (the call textDocument/rangeFormatting) which is used by formatOnPaste. I don't think this has anything to do with that commit, it has just never worked. I know Rebecca was looking into it. All other formatting uses full file formatting.

mschneider82 commented 5 years ago

I also have this problem with the latest commit, just rolled back gopls and copy pasting is working.

nezorflame commented 5 years ago

Seems that the latest commits fixed this behaviour.