Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

cc @muirmanders for fuzzy matching issues, but I'm confident there are unimported completion scoring issues in here too so I'll take it for now.

Change https://golang.org/cl/215023 mentions this issue: internal/lsp/source: score in-memory unimported candidates

It appears the fuzzy matcher gives the same (perfect) score to candidates "New" and "NewFoo" when searching on "New". We can either change the fuzzy matcher to add a penalty for non-exact matches, or we can change gopls to prefer shorter candidates when the scores are otherwise identical. I would guess the latter is a better approach, especially considering that there are different matchers beyond fuzzy.

Actually, the existing sort by candidate label should already order shorter candidates first when scores are equal. If "NewFoo" shows up before "New", it must be because "NewFoo" has a higher score than "New".

You're right, something is giving the worse candidates a much higher score than I set by a factor of 10. All my scores are .01 * 7 at most.

source.CompletionItem{Label:"AdErrorEnum_CANNOT_SET_FIELD_WITH_AD_ID_SET_FOR_SHARING", Detail:"(from \"google.golang.org/genproto/googleapis/ads/googleads/v1/errors\")", InsertText:"AdErrorEnum_CANNOT_SET_FIELD_WITH_AD_ID_SET_FOR_SHARING", Kind:var, AdditionalTextEdits:[]protocol.TextEdit{protocol.TextEdit{Range:1:0-1:0, NewText:"\nimport \"google.golang.org/genproto/googleapis/ads/googleads/v1/errors\"\n"}}, Depth:0, Score:0.4, snippet:(*snippet.Builder)(nil), Documentation:""}
vs
source.CompletionItem{Label:"As", Detail:"func(err error, target interface{}) bool (from \"errors\")", InsertText:"As", Kind:func, AdditionalTextEdits:[]protocol.TextEdit{protocol.TextEdit{Range:1:0-1:0, NewText:"\nimport \"errors\"\n"}}, Depth:0, Score:0.07, snippet:(*snippet.Builder)(0xc011946cc0), Documentation:"As finds the first error in err's chain that matches target, and if so, sets target to that error value and returns true."}

https://github.com/golang/tools/blob/6edc0a871e694a1c8728996c84668863c13d2b4f/internal/lsp/source/completion.go#L326

maybe? I think we should have a rule that scores only go one direction, probably down, from the ones passed in.

something is giving the worse candidates a much higher score

All non-typed candidates end up getting the highScore multiplier: https://github.com/golang/tools/blob/6edc0a871e694a1c8728996c84668863c13d2b4f/internal/lsp/source/completion.go#L1682

we should have a rule that scores only go one direction

The code that discovers a candidate doesn't normally need to be concerned with the quality of the candidate. If the candidate ends up being a good match, it gets the highScore multiplier. If it is a poor match, it gets the lowScore multiplier. If it is a maybe-annoying candidate it gets a more moderate score penalty. We could make it only go down, but then the caller of found() would need to call matchingCandidate() to set the starting score.

The untyped unimported candidates are tricky to rank properly relative to typed candidates. I would make typed unimported candidates use stdScore as a base (modified slightly by any relevance data), and make the untyped candidates use maybe 0.9 * stdScore as the base so they are downranked relative to typed candidates if all else is equal (but not too far so that it doesn't take much extra typing to get the untyped candidate as the top candidate). In general it doesn't seem like the unimported package members need to use such low scores. The unimported package name candidates should be downranked a bit, but they also probably don't need such a low score (e.g. 0.5 * stdScore might be sufficient).

We could make it only go down, but then the caller of found() would need to call matchingCandidate() to set the starting score.

I don't think that's true? Anywhere we multiply by 10 today, the caller could pass score*10 and we could divide by 10 everywhere we aren't multiplying now. Just so that it's more predictable what the score range of any given call could be.

I reviewed all the callers of matchingCandidate, and it looks like it will be harmless to return false instead of true -- it controls stuff like creating composite literals and taking references, none of which makes sense if you don't know the type. That resolves the immediate complaint in this bug. CL shortly.

Change https://golang.org/cl/215322 mentions this issue: internal/lsp/source: fix ranking of untyped completions

Hopefully this is fixed enough for v0.3. @muirdm, do you think we need to make the changes you described above, or just keep them in mind in case of future issues? I don't fully understand the implications.

We don't need to do anything now. We should keep it in mind if we get reports like "unimported candidate xyz not ranked first after typing xy" or similar unexpected ranking issues.

Victory. 😄

Cool, closing.