mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp/source/completion: suggest only valid package names 

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

Change-Id: I4b710f90d1723c512e29dc3c248a1e681f1cd37f
GitHub-Last-Rev: 8ae69f1c6fdf80831e5773bdb3507a8d51a4a0cf
GitHub-Pull-Request: golang/tools#310
Reviewed-on: https://go-review.googlesource.com/c/tools/+/313092
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Trust: Rebecca Stambler <rstambler@golang.org>
Trust: Peter Weinberger <pjw@google.com>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This commit is contained in:
Shoshin Nikita 2021-04-27 18:57:43 +00:00 committed by Rebecca Stambler
parent 6397a11608
commit d0768c9130
3 changed files with 187 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
gopls/internal/regtest/completion/completion_test.go
View File

@ -70,6 +70,11 @@ package
-- fruits/testfile3.go --
pac
-- 123f_r.u~its-123/testfile.go --
package
-- .invalid-dir@-name/testfile.go --
package
`
var (
testfile4 = ""
@ -147,6 +152,21 @@ pac
want: []string{"package apple", "package apple_test", "package fruits", "package fruits_test", "package main"},
editRegexp: `\*\/\n()`,
},
// Issue golang/go#44680
{
name: "package completion for dir name with punctuation",
filename: "123f_r.u~its-123/testfile.go",
triggerRegexp: "package()",
want: []string{"package fruits123", "package fruits123_test", "package main"},
editRegexp: "package\n",
},
{
name: "package completion for invalid dir name",
filename: ".invalid-dir@-name/testfile.go",
triggerRegexp: "package()",
want: []string{"package main"},
editRegexp: "package\n",
},
} {
t.Run(tc.name, func(t *testing.T) {
Run(t, files, func(t *testing.T, env *Env) {

108
internal/lsp/source/completion/package.go
View File

@ -5,6 +5,7 @@
package completion
import (
"bytes"
"context"
"fmt"
"go/ast"
@ -14,6 +15,7 @@ import (
"go/types"
"path/filepath"
"strings"
"unicode"
"golang.org/x/tools/internal/lsp/fuzzy"
"golang.org/x/tools/internal/lsp/protocol"
@ -207,17 +209,12 @@ func (c *completer) packageNameCompletions(ctx context.Context, fileURI span.URI
// have the given prefix and are used in the same directory as the given
// file. This also includes test packages for these packages (<pkg>_test) and
// the directory name itself.
func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI span.URI, prefix string) ([]candidate, error) {
func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI span.URI, prefix string) (packages []candidate, err error) {
workspacePackages, err := snapshot.WorkspacePackages(ctx)
if err != nil {
return nil, err
}
dirPath := filepath.Dir(string(fileURI))
dirName := filepath.Base(dirPath)
seenPkgs := make(map[string]struct{})
toCandidate := func(name string, score float64) candidate {
obj := types.NewPkgName(0, nil, name, types.NewPackage("", name))
return candidate{obj: obj, name: name, detail: name, score: score}
@ -225,9 +222,24 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s
matcher := fuzzy.NewMatcher(prefix)
// Always try to suggest a main package
defer func() {
if score := float64(matcher.Score("main")); score > 0 {
packages = append(packages, toCandidate("main", score*lowScore))
}
}()
dirPath := filepath.Dir(fileURI.Filename())
dirName := filepath.Base(dirPath)
if !isValidDirName(dirName) {
return packages, nil
}
pkgName := convertDirNameToPkgName(dirName)
seenPkgs := make(map[string]struct{})
// The `go` command by default only allows one package per directory but we
// support multiple package suggestions since gopls is build system agnostic.
var packages []candidate
for _, pkg := range workspacePackages {
if pkg.Name() == "main" || pkg.Name() == "" {
continue
@ -239,7 +251,7 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s
// Only add packages that are previously used in the current directory.
var relevantPkg bool
for _, pgf := range pkg.CompiledGoFiles() {
if filepath.Dir(string(pgf.URI)) == dirPath {
if filepath.Dir(pgf.URI.Filename()) == dirPath {
relevantPkg = true
break
}
@ -267,20 +279,80 @@ func packageSuggestions(ctx context.Context, snapshot source.Snapshot, fileURI s
}
// Add current directory name as a low relevance suggestion.
if _, ok := seenPkgs[dirName]; !ok {
if score := float64(matcher.Score(dirName)); score > 0 {
packages = append(packages, toCandidate(dirName, score*lowScore))
if _, ok := seenPkgs[pkgName]; !ok {
if score := float64(matcher.Score(pkgName)); score > 0 {
packages = append(packages, toCandidate(pkgName, score*lowScore))
}
testDirName := dirName + "_test"
if score := float64(matcher.Score(testDirName)); score > 0 {
packages = append(packages, toCandidate(testDirName, score*lowScore))
testPkgName := pkgName + "_test"
if score := float64(matcher.Score(testPkgName)); score > 0 {
packages = append(packages, toCandidate(testPkgName, score*lowScore))
}
}
if score := float64(matcher.Score("main")); score > 0 {
packages = append(packages, toCandidate("main", score*lowScore))
}
return packages, nil
}
// isValidDirName checks whether the passed directory name can be used in
// a package path. Requirements for a package path can be found here:
// https://golang.org/ref/mod#go-mod-file-ident.
func isValidDirName(dirName string) bool {
if dirName == "" {
return false
}
for i, ch := range dirName {
if isLetter(ch) || isDigit(ch) {
continue
}
if i == 0 {
// Directory name can start only with '_'. '.' is not allowed in module paths.
// '-' and '~' are not allowed because elements of package paths must be
// safe command-line arguments.
if ch == '_' {
continue
}
} else {
// Modules path elements can't end with '.'
if isAllowedPunctuation(ch) && (i != len(dirName)-1 || ch != '.') {
continue
}
}
return false
}
return true
}
// convertDirNameToPkgName converts a valid directory name to a valid package name.
// It leaves only letters and digits. All letters are mapped to lower case.
func convertDirNameToPkgName(dirName string) string {
var buf bytes.Buffer
for _, ch := range dirName {
switch {
case isLetter(ch):
buf.WriteRune(unicode.ToLower(ch))
case buf.Len() != 0 && isDigit(ch):
buf.WriteRune(ch)
}
}
return buf.String()
}
// isLetter and isDigit allow only ASCII characters because
// "Each path element is a non-empty string made of up ASCII letters,
// ASCII digits, and limited ASCII punctuation"
// (see https://golang.org/ref/mod#go-mod-file-ident).
func isLetter(ch rune) bool {
return 'a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z'
}
func isDigit(ch rune) bool {
return '0' <= ch && ch <= '9'
}
func isAllowedPunctuation(ch rune) bool {
return ch == '_' || ch == '-' || ch == '~' || ch == '.'
}

77
internal/lsp/source/completion/package_test.go Normal file
View File

@ -0,0 +1,77 @@
// Copyright 2021 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
package completion
import "testing"
func TestIsValidDirName(t *testing.T) {
tests := []struct {
dirName string
valid bool
}{
{dirName: "", valid: false},
//
{dirName: "a", valid: true},
{dirName: "abcdef", valid: true},
{dirName: "AbCdEf", valid: true},
//
{dirName: "1a35", valid: true},
{dirName: "a16", valid: true},
//
{dirName: "_a", valid: true},
{dirName: "a_", valid: true},
//
{dirName: "~a", valid: false},
{dirName: "a~", valid: true},
//
{dirName: "-a", valid: false},
{dirName: "a-", valid: true},
//
{dirName: ".a", valid: false},
{dirName: "a.", valid: false},
//
{dirName: "a~_b--c.-e", valid: true},
{dirName: "~a~_b--c.-e", valid: false},
{dirName: "a~_b--c.-e--~", valid: true},
{dirName: "a~_b--2134dc42.-e6--~", valid: true},
{dirName: "abc`def", valid: false},
{dirName: "тест", valid: false},
{dirName: "你好", valid: false},
}
for _, tt := range tests {
valid := isValidDirName(tt.dirName)
if tt.valid != valid {
t.Errorf("%s: expected %v, got %v", tt.dirName, tt.valid, valid)
}
}
}
func TestConvertDirNameToPkgName(t *testing.T) {
tests := []struct {
dirName string
pkgName string
}{
{dirName: "a", pkgName: "a"},
{dirName: "abcdef", pkgName: "abcdef"},
{dirName: "AbCdEf", pkgName: "abcdef"},
{dirName: "1a35", pkgName: "a35"},
{dirName: "14a35", pkgName: "a35"},
{dirName: "a16", pkgName: "a16"},
{dirName: "_a", pkgName: "a"},
{dirName: "a_", pkgName: "a"},
{dirName: "a~", pkgName: "a"},
{dirName: "a-", pkgName: "a"},
{dirName: "a~_b--c.-e", pkgName: "abce"},
{dirName: "a~_b--c.-e--~", pkgName: "abce"},
{dirName: "a~_b--2134dc42.-e6--~", pkgName: "ab2134dc42e6"},
}
for _, tt := range tests {
pkgName := convertDirNameToPkgName(tt.dirName)
if tt.pkgName != pkgName {
t.Errorf("%s: expected %v, got %v", tt.dirName, tt.pkgName, pkgName)
continue
}
}
}