API docs: only index top-level symbols (#166)

* API docs: indexer: add a test proving that non-top-level symbols are being emitted incorrectly * fix the bug * CI: fix gofmt check, add comment Signed-off-by: Stephen Gutekanst <[email protected]>
sourcegraph · Jun 16, 2021 · 4e1a1f4 · 4e1a1f4
1 parent 05c7a45
commit 4e1a1f4
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 99 deletions.
diff --git a/buildkite.yml b/buildkite.yml
@@ -1,5 +1,6 @@
 steps:
   - label: ':go:'
-    command: find internal/testdata -type f -name '*.go' -exec go fmt {} \;
+    # Note: go fmt on its own ignores testdata/ directories.
+    command: test -z $(gfind internal/testdata -type f -name '*.go' -exec gofmt -l {} \;)
   - label: ':go:'
     command: go test ./...
diff --git a/internal/indexer/documentation.go b/internal/indexer/documentation.go
@@ -299,13 +299,9 @@ func (d *docsIndexer) indexFile(p *packages.Package, f *ast.File, isTestFile boo
 	var result fileDocs
 	result.pkgDocsMarkdown = godocToMarkdown(f.Doc.Text())
 
-	// Visit each AST node, collecting the bits we want to emit documentation for.
-	var (
-		firstErr error
-		visitor  ast.Visitor
-	)
-	visitor = visitorFunc(func(n ast.Node) (w ast.Visitor) {
-		switch node := n.(type) {
+	// Collect each top-level declaration.
+	for _, decl := range f.Decls {
+		switch node := decl.(type) {
 		case *ast.GenDecl:
 			genDeclDocs := d.indexGenDecl(p, f, node, isTestFile)
 			result.consts = append(result.consts, genDeclDocs.consts...)
@@ -315,11 +311,6 @@ func (d *docsIndexer) indexFile(p *packages.Package, f *ast.File, isTestFile boo
 			// Functions, methods
 			result.funcs = append(result.funcs, d.indexFuncDecl(p.Fset, p, node, isTestFile))
 		}
-		return visitor
-	})
-	ast.Walk(visitor, f)
-	if firstErr != nil {
-		return fileDocs{}, firstErr
 	}
 
 	// Emit documentation for all constants.
@@ -770,13 +761,6 @@ func godocToMarkdown(godoc string) string {
 	return buf.String()
 }
 
-// visitorFunc is a function which implements the ast.Visitor interface.
-type visitorFunc func(ast.Node) ast.Visitor
-
-func (v visitorFunc) Visit(n ast.Node) ast.Visitor {
-	return v(n)
-}
-
 // documentationResult is a simple emitter of complete documentationResults.
 //
 // Advanced usages should just emit the vertices/edges on their own instead

diff --git a/internal/indexer/testdata/TestIndexer_documentation/testdata.golden b/internal/indexer/testdata/TestIndexer_documentation/testdata.golden
@@ -34,7 +34,6 @@ testdata is a small package containing sample Go source code used for testing th
   - [var BigVar](#github.com-sourcegraph-lsif-go-internal-testdata---BigVar)
   - [var VarBlock1](#github.com-sourcegraph-lsif-go-internal-testdata---VarBlock1)
   - [var VarBlock2](#github.com-sourcegraph-lsif-go-internal-testdata---VarBlock2)
-  - [var wg](#github.com-sourcegraph-lsif-go-internal-testdata---wg)
 - [Types](#github.com-sourcegraph-lsif-go-internal-testdata---type)
   - [type Embedded struct](#github.com-sourcegraph-lsif-go-internal-testdata---Embedded)
   - [type Struct struct](#github.com-sourcegraph-lsif-go-internal-testdata---Struct)
@@ -1085,68 +1084,6 @@ ZZZzz /,`.-'`'    -.  ;-;;,_
 ```
 It's sleeping! Some people write that as `sleeping` but Markdown isn't allowed in Go docstrings, right? right?! 
 
-#### <a name="github.com-sourcegraph-lsif-go-internal-testdata---wg">var wg</a>
-
-<details><summary>hover</summary>
-
-> ```go
-> var wg WaitGroup
-> ```
-
-</details>
-
-<details><summary>definitions</summary>
-
-```json
-[
- {
-  "Document": "/parallel.go",
-  "Ranges": [
-   {
-    "start": {
-     "line": 14,
-     "character": 5
-    },
-    "end": {
-     "line": 14,
-     "character": 7
-    }
-   }
-  ]
- }
-]
-```
-
-</details>
-
-<details><summary>references</summary>
-
-```json
-[
- {
-  "Document": "/parallel.go",
-  "Ranges": [
-   {
-    "start": {
-     "line": 14,
-     "character": 5
-    },
-    "end": {
-     "line": 14,
-     "character": 7
-    }
-   }
-  ]
- }
-]
-```
-
-</details>
-
-```Go
-var wg sync.WaitGroup
-```
-
 ### <a name="github.com-sourcegraph-lsif-go-internal-testdata---type">Types</a>
 
 #### <a name="github.com-sourcegraph-lsif-go-internal-testdata---Embedded">type Embedded struct <small>(exported)</small></a>
@@ -1546,11 +1483,11 @@ func (s *Struct) MachineLearning(
   "Ranges": [
    {
     "start": {
-     "line": 97,
+     "line": 109,
      "character": 5
     },
     "end": {
-     "line": 97,
+     "line": 109,
      "character": 14
     }
    }
@@ -1570,11 +1507,11 @@ func (s *Struct) MachineLearning(
   "Ranges": [
    {
     "start": {
-     "line": 97,
+     "line": 109,
      "character": 5
     },
     "end": {
-     "line": 97,
+     "line": 109,
      "character": 14
     }
    }
@@ -1612,11 +1549,11 @@ Interface has docs too
   "Ranges": [
    {
     "start": {
-     "line": 101,
+     "line": 113,
      "character": 5
     },
     "end": {
-     "line": 101,
+     "line": 113,
      "character": 17
     }
    }
@@ -1636,11 +1573,11 @@ Interface has docs too
   "Ranges": [
    {
     "start": {
-     "line": 101,
+     "line": 113,
      "character": 5
     },
     "end": {
-     "line": 101,
+     "line": 113,
      "character": 17
     }
    }
@@ -1688,11 +1625,11 @@ func NewInterface() Interface
   "Ranges": [
    {
     "start": {
-     "line": 121,
+     "line": 133,
      "character": 1
     },
     "end": {
-     "line": 121,
+     "line": 133,
      "character": 2
     }
    }
@@ -1712,11 +1649,11 @@ func NewInterface() Interface
   "Ranges": [
    {
     "start": {
-     "line": 121,
+     "line": 133,
      "character": 1
     },
     "end": {
-     "line": 121,
+     "line": 133,
      "character": 2
     }
    }
@@ -1770,11 +1707,11 @@ And confusing
   "Ranges": [
    {
     "start": {
-     "line": 125,
+     "line": 137,
      "character": 1
     },
     "end": {
-     "line": 125,
+     "line": 137,
      "character": 2
     }
    }
@@ -1794,11 +1731,11 @@ And confusing
   "Ranges": [
    {
     "start": {
-     "line": 125,
+     "line": 137,
      "character": 1
     },
     "end": {
-     "line": 125,
+     "line": 137,
      "character": 2
     }
    }

diff --git a/internal/testdata/child_symbols.go b/internal/testdata/child_symbols.go
@@ -91,6 +91,18 @@ func (s *Struct) MachineLearning(
 	hyperparam2 float32,
 	hyperparam3 float32,
 ) float32 {
+	// varShouldNotHaveDocs is in a function, should not have docs emitted.
+	var varShouldNotHaveDocs int32
+
+	// constShouldNotHaveDocs is in a function, should not have docs emitted.
+	const constShouldNotHaveDocs = 5
+
+	// typeShouldNotHaveDocs is in a function, should not have docs emitted.
+	type typeShouldNotHaveDocs struct{ a string }
+
+	// funcShouldNotHaveDocs is in a function, should not have docs emitted.
+	funcShouldNotHaveDocs := func(a string) string { return "hello" }
+
 	return param1 + (hyperparam2 * *hyperparam3) // lol is this all ML is? I'm gonna be rich
 }