Skip to content

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

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

Closed
xzbdmw opened this issue Dec 6, 2023 · 9 comments
Assignees
Labels
Documentation Issues describing a change to documentation. FeatureRequest Issues asking for a new feature that does not need a proposal. gopls Issues related to the Go language server, gopls. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@xzbdmw
Copy link

xzbdmw commented Dec 6, 2023

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

@xzbdmw xzbdmw added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Dec 6, 2023
@gopherbot gopherbot added this to the Unreleased milestone Dec 6, 2023
@hyangah
Copy link
Contributor

hyangah commented Dec 7, 2023

@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?

@hyangah hyangah added FeatureRequest Issues asking for a new feature that does not need a proposal. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 7, 2023
@hyangah hyangah modified the milestones: Unreleased, gopls/unplanned Dec 7, 2023
@xzbdmw
Copy link
Author

xzbdmw commented Dec 7, 2023

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

pjweinb commented Dec 7, 2023

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

adonovan commented Dec 8, 2023

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

xzbdmw commented Dec 11, 2023

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

@jheroy
Copy link

jheroy commented Apr 18, 2024

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

@adonovan adonovan changed the title x/tools/gopls: variables don't distinguish write access and read access x/tools/gopls: syntax highlighting doesn't distinguish read from write access to variables Apr 18, 2024
@zhzhsx
Copy link

zhzhsx commented Jul 10, 2024

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

hyangah commented Jul 10, 2024

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

@hyangah hyangah changed the title x/tools/gopls: syntax highlighting doesn't distinguish read from write access to variables x/tools/gopls: enhance read/write access distinction in document highlighting for symbols Jul 10, 2024
@gopherbot gopherbot added the Documentation Issues describing a change to documentation. label Jul 10, 2024
zhzhsx added a commit to zhzhsx/tools that referenced this issue Jul 11, 2024
… for symbols

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.

Fixes golang/go#64579
zhzhsx added a commit to zhzhsx/tools that referenced this issue Jul 11, 2024
… for symbols

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.

Fixes golang/go#64579
zhzhsx added a commit to zhzhsx/tools that referenced this issue Jul 11, 2024
… for symbols

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.

Fixes golang/go#64579
@gopherbot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Issues describing a change to documentation. FeatureRequest Issues asking for a new feature that does not need a proposal. gopls Issues related to the Go language server, gopls. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants