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

Jump to bottom

x/tools/gopls: suggests invalid package names #44680

Closed
seankhliao opened this issue Feb 28, 2021 · 4 comments
Closed

x/tools/gopls: suggests invalid package names #44680

seankhliao opened this issue Feb 28, 2021 · 4 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Projects
gopls on-deck
Milestone
gopls/v0.6.11

Comments

@seankhliao
Copy link
Member

go: tip
gopls: v0.6.6

With module / directory names with - in them, gopls uses that directly as a suggested package name, which is invalid

2021-02-28-115235
@seankhliao seankhliao 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 Feb 28, 2021
@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2021
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Feb 28, 2021
@stamblerre stamblerre added this to To Do in gopls on-deck Feb 28, 2021
@ShoshinNikita
Copy link

We can replace - with _. But there are other cases:

  • directory name starts with a digit
  • directory name contains one of these characters: -, . or ~
  • directory name contains invalid characters (whitespace, comma, etc.)

I think we can ignore the last case and don't show suggestions at all because it's not possible to import such packages (https://golang.org/ref/mod#go-mod-file-ident). But what is the expected behavior in other situations?

@mattheusv
Copy link

mattheusv commented Apr 17, 2021 •
edited

What do you think about ignoring anything that is not a letter? So in case of foo-bar, 123foobar, foo bar will be always suggest foobar

ShoshinNikita added a commit to ShoshinNikita/tools that referenced this issue Apr 25, 2021
@ShoshinNikita
internal/lsp/source/completion: suggest only valid package names
bcb1498 
Before this change directory names were used "as is" for package completion.
It could lead to invalid suggestions (for example, 'package 1abc' or package 'ab-cd').

This change adds a check whether a directory name can be used in a package path.
If the directory name is invalid, only 'package main' will be suggested.
Otherwise, the directory name will be normalized and will be used as a package name.

Note: normalized directory names contain only lower case letters and digits.

Fixes golang/go#44680
@ShoshinNikita ShoshinNikita mentioned this issue Apr 25, 2021
Closed
@ShoshinNikita
Copy link

@msAlcantara Thanks for the idea! I was planning to convert all punctuation to _, but it would conflict with the package name convention. So, ignoring the punctuation is a better strategy.

@gopherbot
Copy link

Change https://golang.org/cl/313092 mentions this issue: internal/lsp/source/completion: suggest only valid package names

gopls on-deck automation moved this from To Do to Done Apr 27, 2021
@golang golang locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. help wanted Tools This label describes issues relating to any tools in the x/tools repository.
Projects
No open projects
gopls on-deck
Done
Milestone
gopls/v0.6.11
Development

Successfully merging a pull request may close this issue.

internal/lsp/source/completion: suggest only valid package names ShoshinNikita/tools
5 participants
@stamblerre @gopherbot @seankhliao @ShoshinNikita @mattheusv