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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: don't downrank symbol results outside of the workspace folder, but in a workspace module #63458

Open
myitcv opened this issue Oct 9, 2023 · 4 comments
Open

x/tools/gopls: don't downrank symbol results outside of the workspace folder, but in a workspace module #63458

myitcv opened this issue Oct 9, 2023 · 4 comments
Labels
gopls Issues related to the Go language server, gopls. 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
gopls/v0.17.0

Comments

@myitcv
Copy link
Member

myitcv commented Oct 9, 2023

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

$ go version
go version go1.21.1 linux/arm64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.14.1-0.20231008020826-a3b5082fb05e
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20231008020826-a3b5082fb05e

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
GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/home/myitcv/.cache/go-build'
GOENV='/home/myitcv/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/myitcv/gostuff/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/myitcv/gostuff'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/myitcv/gos'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/myitcv/gos/pkg/tool/linux_arm64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3782877243=/tmp/go-build -gno-record-gcc-switches'

What did you do?

cd $(mktemp -d)
git clone https://github.com/cue-lang/cuelang.org
cd cuelang.org
git checkout 8c3444a51e940c9673af9f96b2f6cb0f98c32529 # alpha branch

Open vim/editor, and do a symbol search for cue.Str

What did you expect to see?

The result for cuelang.org/go/cue.Str as the best match.

What did you see instead?

cuelang.org/go/cue.Str as the 13th best match with a number of worse matches scoring higher (highest score first)

github.com/cue-lang/cuelang.org/internal/parse.ContinueNode.String
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal/third_party/yaml.yaml_document_t.start_mark
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal/third_party/yaml.yaml_document_t.start_implicit
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal/third_party/yaml.yaml_document_t.tag_directives_start
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.ToStruct
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.EmbedStruct
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.Attr.String
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.ConstraintToken
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.SetConstraint
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.scanAttributeElem
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal.scanAttributeString
github.com/cue-lang/cuelang.org/playground/internal/cuelang_org_go_internal/third_party/yaml.labelStr
cuelang.org/go/cue.Str
...

Logs from my session locally:

cc @findleyr
@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Oct 9, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Oct 9, 2023
@gopherbot gopherbot added this to the Unreleased milestone Oct 9, 2023
@findleyr
Copy link
Contributor

findleyr commented Oct 9, 2023 •
edited

Thanks. That's definitely not right. The scoring algorithm is simple: (1) cue should score higher than Continue, (2) shorter results should score higher than longer.

Reproduces with: gopls workspace_symbol -matcher=fastfuzzy "cue.str"

I'll investigate and fix for v0.14.0.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.14.0 Oct 9, 2023
@findleyr
Copy link
Contributor

This isn't a bug in the scoring algorithm, per se: the fuzzy score of "cue.Str" is indeed higher than all others.
But "cue.Str" is down-ranked because it is outside of the workspace -- I didn't realize this initially because it is in a cue package, but of the main cue repo.

The following specific steps mitigate the problem (but see below -- these steps are too strict):

  • create a directory containing both the cue and cuelang.org repos
  • go work init && go work use -r .
  • open gopls from that directory

Then both cue and cuelang.org will be considered "in the workspace", and results will match expectation.

But this is confusing for LSP clients without a strong concept of 'workspace folder'. We typically expand the workspace to all main modules (including any specified in go.work). It should be sufficient simply to use a go.work file, and open gopls from any directory.

It may also seem tempting to remove this down-ranking of non-workspace packages, but we have heard multiple times that some users don't want any non-workspace results. I think properly respecting go.work files in symbol results is the way to resolve this problem.

@findleyr findleyr changed the title x/tools/gopls: symbol search gives poor results ahead of exact match x/tools/gopls: don't downrank symbol results outside of the workspace folder, but in a workspace module Oct 11, 2023
@findleyr findleyr modified the milestones: gopls/v0.14.0, gopls/v0.14.1 Oct 11, 2023
@myitcv
Copy link
Member Author

myitcv commented Oct 11, 2023

I think properly respecting go.work files in symbol results is the way to resolve this problem.

Just so that I'm clear, are you suggesting I should add the cuelang.org/go dependency to my go.work file just so that symbol results in that dependency are not downranked?

If so, I don't think this is a solution in this case or in general. Because cuelang.org/go is just one dependency of many, and I don't see myself creating go.work files for this purpose.

If this downranking is the reason for the "bad" results (from my perspective), then I believe #37237 is the better solution. Because with that approach, no such downranking would be required: I would explicitly be in a mode of searching the current package, the main module, workspace etc.

@findleyr
Copy link
Contributor

Just so that I'm clear, are you suggesting I should add the cuelang.org/go dependency to my go.work file just so that symbol results in that dependency are not downranked?

I'm not suggesting any action -- I'm simply explaining the current ranking :), and the heuristics behind it.

A go.work is how we ask users to specify the code they are "working" on, and symbol search down ranks results outside of the workspace. I think the current behavior is incorrect in that the user needs to also think about the "workspace folder" -- as much as possible we want to eliminate this from the configuration space (see also #57979).

Yes, #37237 would address this for clients that integrate the new syntax or custom command.

@findleyr findleyr modified the milestones: gopls/v0.14.2, gopls/v0.15.0 Nov 14, 2023
@findleyr findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Feb 6, 2024
@findleyr findleyr modified the milestones: gopls/v0.16.0, gopls/v0.17.0 Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
gopls Issues related to the Go language server, gopls. 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
Milestone
gopls/v0.17.0
Development

No branches or pull requests
3 participants
@myitcv @gopherbot @findleyr