Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nezorflame opened this issue May 6, 2019 · 3 comments
Closed

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

nezorflame opened this issue May 6, 2019 · 3 comments
Assignees
Milestone

Comments

@nezorflame
Copy link

@nezorflame nezorflame commented May 6, 2019

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 #31797 introduced this behavior.
More particularly, this commit: golang/tools@9cb3dcf

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

@gopherbot gopherbot added this to the Unreleased milestone May 6, 2019
@gopherbot gopherbot added the gopls label May 6, 2019
@ianthehat
Copy link

@ianthehat ianthehat commented May 6, 2019

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
Copy link

@mschneider82 mschneider82 commented May 7, 2019

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

@nezorflame
Copy link
Author

@nezorflame nezorflame commented May 22, 2019

Seems that the latest commits fixed this behaviour.

@nezorflame nezorflame closed this May 22, 2019
@golang golang locked and limited conversation to collaborators May 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.