golang / go

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

x/tools/gopls: enhance read/write access distinction in document highlighting for symbols #64579

Closed xzbdmw closed 1 week ago

xzbdmw commented 8 months ago

gopls version

0.14.2

go env

GO111MODULE="auto"
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/xzb/Library/Caches/go-build"
GOENV="/Users/xzb/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/xzb/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/xzb/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go@1.18/1.18.10/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go@1.18/1.18.10/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18.10"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gv/r110hgbx1gbgzp95kf_q71x40000gn/T/go-build3631566834=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

image

first line is write access and second line is read access

What did you expect to see?

vscode distinguishs write access and read access, most lsp implementations have it such as clangd and rust-analyzer an rust example:

image

see the different color of scoped_interpreter

What did you see instead?

no color difference

Editor and settings

No response

Logs

No response

hyangah commented 8 months ago

@pjweinb Is the modification modifier in the semantic token for this distinction? On the other hand, I wonder what write-access vs read-access means in Go, unlike rust. In io.Copy(w, r), is w read access or write access?

xzbdmw commented 8 months ago

@pjweinb Is the modification modifier in the semantic token for this distinction? On the other hand, I wonder what write-access vs read-access means in Go, unlike rust. In io.Copy(w, r), is w read access or write access?

no,only variable reassign is considered write in pylance,clangd and rust-analyzer.

IMG_7645

pjweinb commented 8 months ago

The documentation says nothing about the meaning of the 'modification' modifier, and gopls doesn't use it. I suspect the uses would vary widely by language, but I don't know what would be useful (and practical) in Go.

adonovan commented 8 months ago

The easiest definition to implement is the variable/value distinction (which C calls lvalue/rvalue). For example:

var v struct { x int }
ptr := &v // ptr and v are both lvalues

ptr.x = ptr.x + 1   // the first x is an lvalue; the second x, and both ptrs, are rvalues
ptr.x++  // x is an lvalue, ptr is an rvalue
v.x = v.x + 1 // the first v and x are lvalues; the second v and x are rvalues

But I have no idea if that's actually what people want. As @hyangah points out with the io.Copy example, what we mean by a read and a write is highly context-dependent.

xzbdmw commented 8 months ago

I think the easiest way is what most people won't feel uncomfortable, and io.Copy example can wait for future improvement

jheroy commented 4 months ago

The simplest way is to reference the highlighting style of GoLand.

zhzhsx commented 1 month ago

I have a plan to implement this feature, and want to know if it is acceptable.

The definition of 'write access' is same as it in GoLand. Some examples are access to variables in declaration, assignment(left value), self increasing, channel sending and composite literal.

The algorithm to find write access is same as it in jdt (Java LSP), by visiting every write statement in ast traversal and collecting the positions of access to variables.

A darft version of implementation is here and the preview of the feature in VSCode is: highlight2

hyangah commented 1 month ago

Thanks @zhzhsx! TIL that this read/write distinction can be marked using DocumentHighlight. I think this is a less invasive and more reliable approach than using custom modifier-based syntax highlighting. Please send a cl following the contribution guide https://go.dev/doc/contribute

gopherbot commented 1 month ago

Change https://go.dev/cl/597675 mentions this issue: gopls: enhance read/write access distinction in document highlighting for symbols