Emit definition for all package statements, not just one

flake.nix

@@ -26,7 +26,7 @@
             pname = "scip-go";
             inherit version;
             src = ./.;
-            vendorHash = "sha256-JCzF4wT8un6Twpo/KLhWfYIfmjfOK6ygF1KSfed0PHY=";
+            vendorHash = "sha256-KhmL7QzqPhGyqE8HLfk0YUywzIlvVsVEeNqilRpHFbo=";
             subPackages = [ "cmd/scip-go" ];
             env.CGO_ENABLED = 0;
             checkPhase = "go test ./...";

go.mod

@@ -3,7 +3,6 @@ module github.com/sourcegraph/scip-go
 go 1.25.0
 
 require (
-	github.com/agnivade/levenshtein v1.2.1
 	github.com/alecthomas/kong v1.14.0
 	github.com/scip-code/scip/bindings/go/scip v0.7.0
 	golang.org/x/mod v0.34.0

go.sum

@@ -1,17 +1,11 @@
-github.com/agnivade/levenshtein v1.2.1 h1:EHBY3UOn1gwdy/VbFwgo4cxecRznFk7fKWN1KOX7eoM=
-github.com/agnivade/levenshtein v1.2.1/go.mod h1:QVVI16kDrtSuwcpd0p1+xMC6Z/VfhtCyDIjcwga4/DU=
 github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0=
 github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
 github.com/alecthomas/kong v1.14.0 h1:gFgEUZWu2ZmZ+UhyZ1bDhuutbKN1nTtJTwh19Wsn21s=
 github.com/alecthomas/kong v1.14.0/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I=
 github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs=
 github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
-github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q=
-github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0/go.mod h1:t2tdKJDJF9BV14lnkjHmOQgcvEKgtqs5a1N3LNdJhGE=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54 h1:SG7nF6SRlWhcT7cNTs5R6Hk4V2lcmLz2NsG2VnInyNo=
-github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA=
 github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
 github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=

internal/document/document.go

@@ -61,6 +61,13 @@ func (d *Document) GetSymbol(pos token.Pos) (string, bool) {
 	return d.pkgSymbols.GetSymbol(pos)
 }
 
+// SetSymbolInformation registers a pre-built SymbolInformation at the given position.
+func (d *Document) SetSymbolInformation(
+	pos token.Pos, info *scip.SymbolInformation,
+) {
+	d.pkgSymbols.Set(pos, info)
+}
+
 // SetNewSymbol declares a new symbol and tracks it within a Document.
 //
 // NOTE: Does NOT emit a new occurrence

internal/index/package_ident.go

@@ -2,105 +2,48 @@ package index
 
 import (
 	"go/ast"
-	"math"
 	"path"
+	"sort"
 	"strings"
 
-	"github.com/agnivade/levenshtein"
 	"golang.org/x/tools/go/packages"
 )
 
-func findBestPackageDefinitionPath(pkg *packages.Package) (*ast.File, error) {
-	if pkg.PkgPath == "builtin" {
-		return nil, nil
-	}
-
-	// Unsafe is special case for builtin
-	if pkg.PkgPath == "unsafe" {
-		return nil, nil
-	}
-
-	if len(pkg.Syntax) == 0 {
-		// This case can be triggered when a package directory only contains `_test.go` files,
-		// as those files will be compiled as part of a separate _test package.
-		return nil, nil
-	}
-
-	files := []*ast.File{}
-	filesWithDocs := []*ast.File{}
+// findPackageDocs returns all doc comment texts for the package, sorted by
+// relevance: doc.go first, then the file matching the package name, then
+// other matching files, and finally test/non-test mismatched files last.
+// Returns nil if no file has a package doc comment.
+func findPackageDocs(pkg *packages.Package) []string {
+	var filesWithDocs []*ast.File
 	for _, f := range pkg.Syntax {
-		// pos := pkg.Fset.Position(f.Pos())
-
-		files = append(files, f)
 		if f.Doc != nil {
 			filesWithDocs = append(filesWithDocs, f)
 		}
 	}
 
-	// The idiomatic way is to _only_ have one .go file per package that has a docstring
-	// for the package. This should generally return here.
-	if len(filesWithDocs) == 1 {
-		return filesWithDocs[0], nil
-	}
-
-	// If we for some reason have more than one .go file per package that has a docstring,
-	// only consider returning paths that contain the docstring (instead of any of the possible
-	// paths).
-	if len(filesWithDocs) > 1 {
-		files = filesWithDocs
-	}
-
-	// Try to only pick non _test files for non _test packages and vice versa.
-	files = filterBasedOnTestFiles(pkg, files)
-
-	// Find the best remaining path.
-	// Chooses:
-	//     1. doc.go
-	//     2. exact match
-	//     3. computes levenshtein and picks best score
-	var bestFile *ast.File
-
-	minDistance := math.MaxInt32
-	for _, f := range files {
-		fPath := pkg.Fset.Position(f.Pos()).Filename
-		fileName := fileNameWithoutExtension(fPath)
+	sort.SliceStable(filesWithDocs, func(i, j int) bool {
+		fi := pkg.Fset.Position(filesWithDocs[i].Pos()).Filename
+		fj := pkg.Fset.Position(filesWithDocs[j].Pos()).Filename
+		return fileRelevance(pkg.Name, fi) < fileRelevance(pkg.Name, fj)
+	})
 
-		if "doc.go" == path.Base(fPath) {
-			return f, nil
-		}
-
-		if pkg.Name == fileName {
-			return f, nil
-		}
-
-		distance := levenshtein.ComputeDistance(pkg.Name, fileName)
-		if distance < minDistance {
-			minDistance = distance
-			bestFile = f
-		}
+	var docs []string
+	for _, f := range filesWithDocs {
+		docs = append(docs, f.Doc.Text())
 	}
-
-	return bestFile, nil
+	return docs
 }
 
-func fileNameWithoutExtension(fileName string) string {
-	return strings.TrimSuffix(fileName, path.Ext(fileName))
-}
-
-func filterBasedOnTestFiles(pkg *packages.Package, files []*ast.File) []*ast.File {
-	packageNameEndsWithTest := strings.HasSuffix(pkg.Name, "_test")
-
-	preferredFiles := []*ast.File{}
-	for _, f := range files {
-		fPath := pkg.Fset.Position(f.Pos())
-		if packageNameEndsWithTest == strings.HasSuffix(fPath.Filename, "_test.go") {
-			preferredFiles = append(preferredFiles, f)
-		}
+// fileRelevance returns a sort key: lower is more relevant.
+func fileRelevance(pkgName, filename string) int {
+	switch {
+	case path.Base(filename) == "doc.go":
+		return 0
+	case strings.TrimSuffix(path.Base(filename), path.Ext(filename)) == pkgName:
+		return 1
+	case !strings.HasSuffix(filename, "_test.go"):
+		return 2
+	default:
+		return 3
 	}
-
-	if len(preferredFiles) > 0 {
-		return preferredFiles
-	}
-
-	return files
 }

internal/index/package_ident_test.go

@@ -3,118 +3,120 @@ package index
 import (
 	"go/ast"
 	"go/token"
+	"slices"
 	"testing"
 
 	"golang.org/x/tools/go/packages"
 )
 
-func TestIndexer_findBestPackageDefinitionPath(t *testing.T) {
+func TestFindPackageDocs(t *testing.T) {
 	type FileInfo struct {
-		Name string
-		Docs bool
+		Name    string
+		DocText string
 	}
 
-	makePackage := func(pkgName string, fileInfo []FileInfo) (*packages.Package, map[*ast.File]string) {
+	makePackage := func(pkgName string, fileInfo []FileInfo) *packages.Package {
 		fset := token.NewFileSet()
+		var syntax []*ast.File
 
-		files := map[string]*ast.File{}
-		syntax := []*ast.File{}
-		posMap := map[*ast.File]string{}
-
-		for idx, info := range fileInfo {
+		for _, info := range fileInfo {
 			var doc *ast.CommentGroup
-			if info.Docs {
-				doc = &ast.CommentGroup{}
-			}
-
-			f := &ast.File{
-				Doc:     doc,
-				Package: 0,
-				Name: &ast.Ident{
-					NamePos: token.Pos(idx),
-					Name:    pkgName,
-					Obj:     &ast.Object{},
-				},
+			if info.DocText != "" {
+				doc = &ast.CommentGroup{
+					List: []*ast.Comment{{Text: "// " + info.DocText}},
+				}
 			}
 
-			files[info.Name] = f
-			syntax = append(syntax, f)
-			posMap[f] = info.Name
+			tf := fset.AddFile(info.Name, fset.Base(), 1)
 
-			fset.AddFile(info.Name, fset.Base(), 1)
+			syntax = append(syntax, &ast.File{
+				Doc:     doc,
+				Package: tf.Pos(0),
+				Name:    &ast.Ident{Name: pkgName},
+			})
 		}
 
 		return &packages.Package{
-			ID:      "test-package",
 			Name:    pkgName,
 			PkgPath: "test-package",
-			Imports: map[string]*packages.Package{},
 			Fset:    fset,
 			Syntax:  syntax,
-		}, posMap
+		}
 	}
 
-	makeTest := func(name, pkgName, expected string, fileInfo []FileInfo) {
-		t.Run(name, func(t *testing.T) {
-			pkg, names := makePackage(pkgName, fileInfo)
-
-			pkgToken, _ := findBestPackageDefinitionPath(pkg)
-			if name := names[pkgToken]; name != expected {
-				t.Errorf("incorrect hover text documentation. want=%s have=%s", name, expected)
-			}
-		})
+	// doc produces the text that ast.CommentGroup.Text() returns for a "// text" comment.
+	doc := func(text string) string {
+		return text + "\n"
 	}
 
-	makeTest("Should find exact name match",
-		"smol",
-		"smol.go",
-		[]FileInfo{
-			{"smol.go", false},
-			{"other.go", false},
-		},
-	)
+	t.Run("returns nil when no file has docs", func(t *testing.T) {
+		pkg := makePackage("smol", []FileInfo{
+			{"smol.go", ""},
+			{"other.go", ""},
+		})
+		if docs := findPackageDocs(pkg); docs != nil {
+			t.Errorf("expected nil, got %v", docs)
+		}
+	})
 
-	makeTest("Should return something even if nothing matches",
-		"smol",
-		"random.go",
-		[]FileInfo{
-			{"random.go", false},
-		},
-	)
+	t.Run("returns nil for empty syntax", func(t *testing.T) {
+		pkg := makePackage("mylib", []FileInfo{})
+		if docs := findPackageDocs(pkg); docs != nil {
+			t.Errorf("expected nil, got %v", docs)
+		}
+	})
 
-	makeTest("Should not pick _test files if package is not a test package",
-		"unreleated",
-		"smol.go",
-		[]FileInfo{
-			{"smol.go", false},
-			{"smol_test.go", false},
-		},
-	)
+	t.Run("returns single doc", func(t *testing.T) {
+		pkg := makePackage("mylib", []FileInfo{
+			{"mylib.go", ""},
+			{"has_docs.go", "Package docs"},
+		})
+		want := []string{doc("Package docs")}
+		got := findPackageDocs(pkg)
+		if !slices.Equal(got, want) {
+			t.Errorf("want %v, got %v", want, got)
+		}
+	})
 
-	makeTest("Pick whatever has documentation",
-		"mylib",
-		"has_docs.go",
-		[]FileInfo{
-			{"mylib.go", false},
-			{"has_docs.go", true},
-		},
-	)
+	t.Run("returns all docs sorted with doc.go first", func(t *testing.T) {
+		pkg := makePackage("mylib", []FileInfo{
+			{"mylib.go", "from mylib"},
+			{"doc.go", "from doc.go"},
+			{"other.go", "from other"},
+		})
+		want := []string{doc("from doc.go"), doc("from mylib"), doc("from other")}
+		got := findPackageDocs(pkg)
+		if !slices.Equal(got, want) {
+			t.Errorf("want %v, got %v", want, got)
+		}
+	})
 
-	makeTest("should pick a name that is a closer edit distance than one far away",
-		"http_router",
-		"httprouter.go",
-		[]FileInfo{
-			{"httprouter.go", false},
-			{"httpother.go", false},
-		},
-	)
+	t.Run("returns all docs sorted with package name match first when no doc.go", func(t *testing.T) {
+		pkg := makePackage("mylib", []FileInfo{
+			{"other.go", "from other"},
+			{"mylib.go", "from mylib"},
+		})
+		want := []string{doc("from mylib"), doc("from other")}
+		got := findPackageDocs(pkg)
+		if !slices.Equal(got, want) {
+			t.Errorf("want %v, got %v", want, got)
+		}
+	})
+}
 
-	makeTest("should prefer test packages over other packages if the package name has test suffix",
-		"mylib_test",
-		"mylib_test.go",
-		[]FileInfo{
-			{"mylib_test.go", false},
-			{"mylib.go", false},
-		},
-	)
+func TestFileRelevance(t *testing.T) {
+	tests := []struct {
+		filename string
+		want     int
+	}{
+		{"doc.go", 0},
+		{"mylib.go", 1},
+		{"other.go", 2},
+		{"other_test.go", 3},
+	}
+	for _, tt := range tests {
+		if got := fileRelevance("mylib", tt.filename); got != tt.want {
+			t.Errorf("fileRelevance(%q) = %d, want %d", tt.filename, got, tt.want)
+		}
+	}
 }