internal/lsp: add a test for internal imports handling

I'm still not sure if we need to handle any other non-standard package
path apart from "command-line-arguments".

Also, a couple of staticcheck fixes.

Change-Id: I0bb3e60f6ffe104ff9027dbebb628020caaa1af4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220138
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

6 files changed, 24 insertions, 18 deletions

diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index e6f573dc8..be472fdcb 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -54,19 +54,6 @@ func (ph *packageHandle) packageKey() packageKey {
 	}
 }
 
-func (ph *packageHandle) isValidImportFor(parentPkgPath string) bool {
-	importPath := string(ph.m.pkgPath)
-
-	pkgRootIndex := strings.Index(importPath, "/internal/")
-	if pkgRootIndex != -1 && parentPkgPath != "command-line-arguments" {
-		if !strings.HasPrefix(parentPkgPath, importPath[0:pkgRootIndex]) {
-			return false
-		}
-	}
-
-	return true
-}
-
 // packageData contains the data produced by type-checking a package.
 type packageData struct {
 	memoize.NoCopy
@@ -381,7 +368,7 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc
 			if dep == nil {
 				return nil, errors.Errorf("no package for import %s", pkgPath)
 			}
-			if !dep.isValidImportFor(pkg.PkgPath()) {
+			if !isValidImport(m.pkgPath, dep.m.pkgPath) {
 				return nil, errors.Errorf("invalid use of internal package %s", pkgPath)
 			}
 			depPkg, err := dep.check(ctx)
@@ -416,6 +403,17 @@ func typeCheck(ctx context.Context, fset *token.FileSet, m *metadata, mode sourc
 	return pkg, nil
 }
 
+func isValidImport(pkgPath, importPkgPath packagePath) bool {
+	i := strings.LastIndex(string(importPkgPath), "/internal/")
+	if i == -1 {
+		return true
+	}
+	if pkgPath == "command-line-arguments" {
+		return true
+	}
+	return strings.HasPrefix(string(pkgPath), string(importPkgPath[:i]))
+}
+
 // An importFunc is an implementation of the single-method
 // types.Importer interface based on a function value.
 type importerFunc func(path string) (*types.Package, error)
diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go
index e248609e9..4ef7c8f23 100644
--- a/internal/lsp/fake/editor.go
+++ b/internal/lsp/fake/editor.go
@@ -119,14 +119,14 @@ func (e *Editor) initialize(ctx context.Context) error {
 	if e.server != nil {
 		resp, err := e.server.Initialize(ctx, params)
 		if err != nil {
-			return fmt.Errorf("Initialize: %v", err)
+			return fmt.Errorf("initialize: %v", err)
 		}
 		e.mu.Lock()
 		e.serverCapabilities = resp.Capabilities
 		e.mu.Unlock()
 
 		if err := e.server.Initialized(ctx, &protocol.InitializedParams{}); err != nil {
-			return fmt.Errorf("Initialized: %v", err)
+			return fmt.Errorf("initialized: %v", err)
 		}
 	}
 	return nil
@@ -341,7 +341,7 @@ func (e *Editor) GoToDefinition(ctx context.Context, path string, pos Pos) (stri
 
 	resp, err := e.server.Definition(ctx, params)
 	if err != nil {
-		return "", Pos{}, fmt.Errorf("Definition: %v", err)
+		return "", Pos{}, fmt.Errorf("definition: %v", err)
 	}
 	if len(resp) == 0 {
 		return "", Pos{}, nil
diff --git a/internal/lsp/testdata/lsp/primarymod/assign/assign.go.in b/internal/lsp/testdata/lsp/primarymod/assign/assign.go.in
index f07c461c7..3fe256e66 100644
--- a/internal/lsp/testdata/lsp/primarymod/assign/assign.go.in
+++ b/internal/lsp/testdata/lsp/primarymod/assign/assign.go.in
@@ -1,6 +1,9 @@
 package assign
 
+import "golang.org/x/tools/internal/lsp/assign/internal/secret"
+
 func _() {
+	secret.Hello()
 	var (
 		myInt int //@item(assignInt, "myInt", "int", "var")
 		myStr string //@item(assignStr, "myStr", "string", "var")
diff --git a/internal/lsp/testdata/lsp/primarymod/assign/internal/secret/secret.go b/internal/lsp/testdata/lsp/primarymod/assign/internal/secret/secret.go
new file mode 100644
index 000000000..5ee1554df
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/assign/internal/secret/secret.go
@@ -0,0 +1,3 @@
+package secret
+
+func Hello() {}
\ No newline at end of file
diff --git a/internal/lsp/testdata/lsp/primarymod/bad/bad0.go b/internal/lsp/testdata/lsp/primarymod/bad/bad0.go
index 3c3c24981..cfde87c87 100644
--- a/internal/lsp/testdata/lsp/primarymod/bad/bad0.go
+++ b/internal/lsp/testdata/lsp/primarymod/bad/bad0.go
@@ -2,6 +2,8 @@
 
 package bad
 
+import _ "golang.org/x/tools/internal/lsp/assign/internal/secret" //@diag("\"golang.org/x/tools/internal/lsp/assign/internal/secret\"", "compiler", "could not import golang.org/x/tools/internal/lsp/assign/internal/secret (invalid use of internal package golang.org/x/tools/internal/lsp/assign/internal/secret)", "error")
+
 func stuff() { //@item(stuff, "stuff", "func()", "func")
 	x := "heeeeyyyy"
 	random2(x) //@diag("x", "compiler", "cannot use x (variable of type string) as int value in argument to random2", "error")
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 57faa0c84..7b1a2eb1b 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -7,7 +7,7 @@ DeepCompletionsCount = 5
 FuzzyCompletionsCount = 8
 RankedCompletionsCount = 111
 CaseSensitiveCompletionsCount = 4
-DiagnosticsCount = 38
+DiagnosticsCount = 39
 FoldingRangesCount = 2
 FormatCount = 6
 ImportCount = 7