Skip to content

Commit 3cad9ef

Browse files
authored
Emit definition for all package statements, not just one (#199)
- Mark all package statements as definitions, not just one - Use display_name and signature_documentation instead of stuffing everything into the documentation field - Attach each file's own doc comment to its package occurrence; no cross-file aggregation (SCIP consumers handle that) - Simplify lookup: drop PackageName struct, store symbol string directly - Remove levenshtein dependency and findBestPackageDefinitionPath - Add Document.SetSymbolInformation for pre-built SymbolInformation - Fix stdlib.go so generation comment isn't parsed as package doc - Add pr199 snapshot test covering multi-file package handling
1 parent 2fc8d0d commit 3cad9ef

70 files changed

Lines changed: 300 additions & 322 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

flake.nix

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
pname = "scip-go";
2727
inherit version;
2828
src = ./.;
29-
vendorHash = "sha256-JCzF4wT8un6Twpo/KLhWfYIfmjfOK6ygF1KSfed0PHY=";
29+
vendorHash = "sha256-KhmL7QzqPhGyqE8HLfk0YUywzIlvVsVEeNqilRpHFbo=";
3030
subPackages = [ "cmd/scip-go" ];
3131
env.CGO_ENABLED = 0;
3232
checkPhase = "go test ./...";

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ module github.com/sourcegraph/scip-go
33
go 1.25.0
44

55
require (
6-
github.com/agnivade/levenshtein v1.2.1
76
github.com/alecthomas/kong v1.14.0
87
github.com/scip-code/scip/bindings/go/scip v0.7.0
98
golang.org/x/mod v0.34.0

go.sum

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,11 @@
1-
github.com/agnivade/levenshtein v1.2.1 h1:EHBY3UOn1gwdy/VbFwgo4cxecRznFk7fKWN1KOX7eoM=
2-
github.com/agnivade/levenshtein v1.2.1/go.mod h1:QVVI16kDrtSuwcpd0p1+xMC6Z/VfhtCyDIjcwga4/DU=
31
github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0=
42
github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
53
github.com/alecthomas/kong v1.14.0 h1:gFgEUZWu2ZmZ+UhyZ1bDhuutbKN1nTtJTwh19Wsn21s=
64
github.com/alecthomas/kong v1.14.0/go.mod h1:wrlbXem1CWqUV5Vbmss5ISYhsVPkBb1Yo7YKJghju2I=
75
github.com/alecthomas/repr v0.5.2 h1:SU73FTI9D1P5UNtvseffFSGmdNci/O6RsqzeXJtP0Qs=
86
github.com/alecthomas/repr v0.5.2/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
9-
github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q=
10-
github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0/go.mod h1:t2tdKJDJF9BV14lnkjHmOQgcvEKgtqs5a1N3LNdJhGE=
117
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
128
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
13-
github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54 h1:SG7nF6SRlWhcT7cNTs5R6Hk4V2lcmLz2NsG2VnInyNo=
14-
github.com/dgryski/trifles v0.0.0-20230903005119-f50d829f2e54/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA=
159
github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
1610
github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
1711
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=

internal/document/document.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ func (d *Document) GetSymbol(pos token.Pos) (string, bool) {
6161
return d.pkgSymbols.GetSymbol(pos)
6262
}
6363

64+
// SetSymbolInformation registers a pre-built SymbolInformation at the given position.
65+
func (d *Document) SetSymbolInformation(
66+
pos token.Pos, info *scip.SymbolInformation,
67+
) {
68+
d.pkgSymbols.Set(pos, info)
69+
}
70+
6471
// SetNewSymbol declares a new symbol and tracks it within a Document.
6572
//
6673
// NOTE: Does NOT emit a new occurrence

internal/index/package_ident.go

Lines changed: 27 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -2,105 +2,48 @@ package index
22

33
import (
44
"go/ast"
5-
"math"
65
"path"
6+
"sort"
77
"strings"
88

9-
"github.com/agnivade/levenshtein"
109
"golang.org/x/tools/go/packages"
1110
)
1211

13-
func findBestPackageDefinitionPath(pkg *packages.Package) (*ast.File, error) {
14-
if pkg.PkgPath == "builtin" {
15-
return nil, nil
16-
}
17-
18-
// Unsafe is special case for builtin
19-
if pkg.PkgPath == "unsafe" {
20-
return nil, nil
21-
}
22-
23-
if len(pkg.Syntax) == 0 {
24-
// This case can be triggered when a package directory only contains `_test.go` files,
25-
// as those files will be compiled as part of a separate _test package.
26-
return nil, nil
27-
}
28-
29-
files := []*ast.File{}
30-
filesWithDocs := []*ast.File{}
12+
// findPackageDocs returns all doc comment texts for the package, sorted by
13+
// relevance: doc.go first, then the file matching the package name, then
14+
// other matching files, and finally test/non-test mismatched files last.
15+
// Returns nil if no file has a package doc comment.
16+
func findPackageDocs(pkg *packages.Package) []string {
17+
var filesWithDocs []*ast.File
3118
for _, f := range pkg.Syntax {
32-
// pos := pkg.Fset.Position(f.Pos())
33-
34-
files = append(files, f)
3519
if f.Doc != nil {
3620
filesWithDocs = append(filesWithDocs, f)
3721
}
3822
}
3923

40-
// The idiomatic way is to _only_ have one .go file per package that has a docstring
41-
// for the package. This should generally return here.
42-
if len(filesWithDocs) == 1 {
43-
return filesWithDocs[0], nil
44-
}
45-
46-
// If we for some reason have more than one .go file per package that has a docstring,
47-
// only consider returning paths that contain the docstring (instead of any of the possible
48-
// paths).
49-
if len(filesWithDocs) > 1 {
50-
files = filesWithDocs
51-
}
52-
53-
// Try to only pick non _test files for non _test packages and vice versa.
54-
files = filterBasedOnTestFiles(pkg, files)
55-
56-
// Find the best remaining path.
57-
// Chooses:
58-
// 1. doc.go
59-
// 2. exact match
60-
// 3. computes levenshtein and picks best score
61-
var bestFile *ast.File
62-
63-
minDistance := math.MaxInt32
64-
for _, f := range files {
65-
fPath := pkg.Fset.Position(f.Pos()).Filename
66-
fileName := fileNameWithoutExtension(fPath)
24+
sort.SliceStable(filesWithDocs, func(i, j int) bool {
25+
fi := pkg.Fset.Position(filesWithDocs[i].Pos()).Filename
26+
fj := pkg.Fset.Position(filesWithDocs[j].Pos()).Filename
27+
return fileRelevance(pkg.Name, fi) < fileRelevance(pkg.Name, fj)
28+
})
6729

68-
if "doc.go" == path.Base(fPath) {
69-
return f, nil
70-
}
71-
72-
if pkg.Name == fileName {
73-
return f, nil
74-
}
75-
76-
distance := levenshtein.ComputeDistance(pkg.Name, fileName)
77-
if distance < minDistance {
78-
minDistance = distance
79-
bestFile = f
80-
}
30+
var docs []string
31+
for _, f := range filesWithDocs {
32+
docs = append(docs, f.Doc.Text())
8133
}
82-
83-
return bestFile, nil
34+
return docs
8435
}
8536

86-
func fileNameWithoutExtension(fileName string) string {
87-
return strings.TrimSuffix(fileName, path.Ext(fileName))
88-
}
89-
90-
func filterBasedOnTestFiles(pkg *packages.Package, files []*ast.File) []*ast.File {
91-
packageNameEndsWithTest := strings.HasSuffix(pkg.Name, "_test")
92-
93-
preferredFiles := []*ast.File{}
94-
for _, f := range files {
95-
fPath := pkg.Fset.Position(f.Pos())
96-
if packageNameEndsWithTest == strings.HasSuffix(fPath.Filename, "_test.go") {
97-
preferredFiles = append(preferredFiles, f)
98-
}
37+
// fileRelevance returns a sort key: lower is more relevant.
38+
func fileRelevance(pkgName, filename string) int {
39+
switch {
40+
case path.Base(filename) == "doc.go":
41+
return 0
42+
case strings.TrimSuffix(path.Base(filename), path.Ext(filename)) == pkgName:
43+
return 1
44+
case !strings.HasSuffix(filename, "_test.go"):
45+
return 2
46+
default:
47+
return 3
9948
}
100-
101-
if len(preferredFiles) > 0 {
102-
return preferredFiles
103-
}
104-
105-
return files
10649
}

internal/index/package_ident_test.go

Lines changed: 86 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -3,118 +3,120 @@ package index
33
import (
44
"go/ast"
55
"go/token"
6+
"slices"
67
"testing"
78

89
"golang.org/x/tools/go/packages"
910
)
1011

11-
func TestIndexer_findBestPackageDefinitionPath(t *testing.T) {
12+
func TestFindPackageDocs(t *testing.T) {
1213
type FileInfo struct {
13-
Name string
14-
Docs bool
14+
Name string
15+
DocText string
1516
}
1617

17-
makePackage := func(pkgName string, fileInfo []FileInfo) (*packages.Package, map[*ast.File]string) {
18+
makePackage := func(pkgName string, fileInfo []FileInfo) *packages.Package {
1819
fset := token.NewFileSet()
20+
var syntax []*ast.File
1921

20-
files := map[string]*ast.File{}
21-
syntax := []*ast.File{}
22-
posMap := map[*ast.File]string{}
23-
24-
for idx, info := range fileInfo {
22+
for _, info := range fileInfo {
2523
var doc *ast.CommentGroup
26-
if info.Docs {
27-
doc = &ast.CommentGroup{}
28-
}
29-
30-
f := &ast.File{
31-
Doc: doc,
32-
Package: 0,
33-
Name: &ast.Ident{
34-
NamePos: token.Pos(idx),
35-
Name: pkgName,
36-
Obj: &ast.Object{},
37-
},
24+
if info.DocText != "" {
25+
doc = &ast.CommentGroup{
26+
List: []*ast.Comment{{Text: "// " + info.DocText}},
27+
}
3828
}
3929

40-
files[info.Name] = f
41-
syntax = append(syntax, f)
42-
posMap[f] = info.Name
30+
tf := fset.AddFile(info.Name, fset.Base(), 1)
4331

44-
fset.AddFile(info.Name, fset.Base(), 1)
32+
syntax = append(syntax, &ast.File{
33+
Doc: doc,
34+
Package: tf.Pos(0),
35+
Name: &ast.Ident{Name: pkgName},
36+
})
4537
}
4638

4739
return &packages.Package{
48-
ID: "test-package",
4940
Name: pkgName,
5041
PkgPath: "test-package",
51-
Imports: map[string]*packages.Package{},
5242
Fset: fset,
5343
Syntax: syntax,
54-
}, posMap
44+
}
5545
}
5646

57-
makeTest := func(name, pkgName, expected string, fileInfo []FileInfo) {
58-
t.Run(name, func(t *testing.T) {
59-
pkg, names := makePackage(pkgName, fileInfo)
60-
61-
pkgToken, _ := findBestPackageDefinitionPath(pkg)
62-
if name := names[pkgToken]; name != expected {
63-
t.Errorf("incorrect hover text documentation. want=%s have=%s", name, expected)
64-
}
65-
})
47+
// doc produces the text that ast.CommentGroup.Text() returns for a "// text" comment.
48+
doc := func(text string) string {
49+
return text + "\n"
6650
}
6751

68-
makeTest("Should find exact name match",
69-
"smol",
70-
"smol.go",
71-
[]FileInfo{
72-
{"smol.go", false},
73-
{"other.go", false},
74-
},
75-
)
52+
t.Run("returns nil when no file has docs", func(t *testing.T) {
53+
pkg := makePackage("smol", []FileInfo{
54+
{"smol.go", ""},
55+
{"other.go", ""},
56+
})
57+
if docs := findPackageDocs(pkg); docs != nil {
58+
t.Errorf("expected nil, got %v", docs)
59+
}
60+
})
7661

77-
makeTest("Should return something even if nothing matches",
78-
"smol",
79-
"random.go",
80-
[]FileInfo{
81-
{"random.go", false},
82-
},
83-
)
62+
t.Run("returns nil for empty syntax", func(t *testing.T) {
63+
pkg := makePackage("mylib", []FileInfo{})
64+
if docs := findPackageDocs(pkg); docs != nil {
65+
t.Errorf("expected nil, got %v", docs)
66+
}
67+
})
8468

85-
makeTest("Should not pick _test files if package is not a test package",
86-
"unreleated",
87-
"smol.go",
88-
[]FileInfo{
89-
{"smol.go", false},
90-
{"smol_test.go", false},
91-
},
92-
)
69+
t.Run("returns single doc", func(t *testing.T) {
70+
pkg := makePackage("mylib", []FileInfo{
71+
{"mylib.go", ""},
72+
{"has_docs.go", "Package docs"},
73+
})
74+
want := []string{doc("Package docs")}
75+
got := findPackageDocs(pkg)
76+
if !slices.Equal(got, want) {
77+
t.Errorf("want %v, got %v", want, got)
78+
}
79+
})
9380

94-
makeTest("Pick whatever has documentation",
95-
"mylib",
96-
"has_docs.go",
97-
[]FileInfo{
98-
{"mylib.go", false},
99-
{"has_docs.go", true},
100-
},
101-
)
81+
t.Run("returns all docs sorted with doc.go first", func(t *testing.T) {
82+
pkg := makePackage("mylib", []FileInfo{
83+
{"mylib.go", "from mylib"},
84+
{"doc.go", "from doc.go"},
85+
{"other.go", "from other"},
86+
})
87+
want := []string{doc("from doc.go"), doc("from mylib"), doc("from other")}
88+
got := findPackageDocs(pkg)
89+
if !slices.Equal(got, want) {
90+
t.Errorf("want %v, got %v", want, got)
91+
}
92+
})
10293

103-
makeTest("should pick a name that is a closer edit distance than one far away",
104-
"http_router",
105-
"httprouter.go",
106-
[]FileInfo{
107-
{"httprouter.go", false},
108-
{"httpother.go", false},
109-
},
110-
)
94+
t.Run("returns all docs sorted with package name match first when no doc.go", func(t *testing.T) {
95+
pkg := makePackage("mylib", []FileInfo{
96+
{"other.go", "from other"},
97+
{"mylib.go", "from mylib"},
98+
})
99+
want := []string{doc("from mylib"), doc("from other")}
100+
got := findPackageDocs(pkg)
101+
if !slices.Equal(got, want) {
102+
t.Errorf("want %v, got %v", want, got)
103+
}
104+
})
105+
}
111106

112-
makeTest("should prefer test packages over other packages if the package name has test suffix",
113-
"mylib_test",
114-
"mylib_test.go",
115-
[]FileInfo{
116-
{"mylib_test.go", false},
117-
{"mylib.go", false},
118-
},
119-
)
107+
func TestFileRelevance(t *testing.T) {
108+
tests := []struct {
109+
filename string
110+
want int
111+
}{
112+
{"doc.go", 0},
113+
{"mylib.go", 1},
114+
{"other.go", 2},
115+
{"other_test.go", 3},
116+
}
117+
for _, tt := range tests {
118+
if got := fileRelevance("mylib", tt.filename); got != tt.want {
119+
t.Errorf("fileRelevance(%q) = %d, want %d", tt.filename, got, tt.want)
120+
}
121+
}
120122
}

0 commit comments

Comments
 (0)