Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Commit

Permalink
API docs: do not panic on missing type information from obscure main-…
Browse files Browse the repository at this point in the history
…exported types (#185)

* API docs: do not panic on missing type information from obscure main-exported types

Prior to this change, we would panic (nil pointer deref) in the event that we
encounter an exported type inside of a `package main` (which is non-sensical and
a linter error in Go):

```
package main

type Foo struct{}

func main() {}
```

This may be a bug in `go/packages`, since we are able to pick up type information for
the `main` symbol itself (ironic!) - but I'm not sure. For now, at least don't panic
/ fail to index everything else.

Signed-off-by: Stephen Gutekanst <[email protected]>
  • Loading branch information
Stephen Gutekanst authored Jul 29, 2021
1 parent 9aa5f44 commit c0312b8
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 8 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,26 @@ All notable changes to `lsif-go` are documented in this file.

## Unreleased changes

### Fixed

- An issue where indexing would fail if `package main` contained exported data types. [#185](https://github.com/sourcegraph/lsif-go/pull/185)

## v1.6.3

## Changed
### Changed

- Improved error messages.

## v1.6.2

## Fixed
### Fixed

- API docs no longer incorrectly tags Functions/Variables/etc sections as a package.
- API docs no longer emits null tag lists in violation of the spec.

## v1.6.1

## Fixed
### Fixed

- API docs no longer incorrectly tags some methods as functions and vice-versa.

Expand Down
22 changes: 17 additions & 5 deletions internal/indexer/documentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,11 @@ func (d *docsIndexer) indexGenDecl(p *packages.Package, f *ast.File, node *ast.G
// Not only is it not exported, it cannot be referenced outside this package at all.
continue
}
typeDocs := d.indexTypeSpec(p, t, isTestFile)
typeDocs.docsMarkdown = blockDocsMarkdown + typeDocs.docsMarkdown
result.types = append(result.types, typeDocs)
typeDocs, ok := d.indexTypeSpec(p, t, isTestFile)
if ok {
typeDocs.docsMarkdown = blockDocsMarkdown + typeDocs.docsMarkdown
result.types = append(result.types, typeDocs)
}
}
}
return result
Expand Down Expand Up @@ -689,7 +691,17 @@ func (t typeDocs) result() *documentationResult {
}
}

func (d *docsIndexer) indexTypeSpec(p *packages.Package, in *ast.TypeSpec, isTestFile bool) typeDocs {
func (d *docsIndexer) indexTypeSpec(p *packages.Package, in *ast.TypeSpec, isTestFile bool) (typeDocs, bool) {
if p.TypesInfo.TypeOf(in.Type) == nil || p.TypesInfo.ObjectOf(in.Name) == nil {
// TODO(slimsag): It's unclear why, but exported types declared in a `package main` - such
// as `type User struct` in minimal_main.go - do not have type information and would result
// in a nil pointer deref below.
//
// Will need to investigate/fix this at a later point - it may be a bug in go/packages.
// But, for now, at least don't panic - just discard it. This is a pretty rare occurrence
// anyway, as an exported type from package main is also a linter error / non-idiomatic.
return typeDocs{}, false
}
var result typeDocs
result.label = fmt.Sprintf("type %s %s", in.Name.String(), formatTypeLabel(p.TypesInfo.TypeOf(in.Type)))
result.name = in.Name.String()
Expand All @@ -713,7 +725,7 @@ func (d *docsIndexer) indexTypeSpec(p *packages.Package, in *ast.TypeSpec, isTes
result.signature = "type " + formatNode(p.Fset, &cpy)

result.docsMarkdown = godocToMarkdown(in.Doc.Text())
return result
return result, true
}

type funcDocs struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,29 @@
"children": null
}
},
{
"node": {
"pathID": "/#main",
"documentation": {
"identifier": "main",
"newPage": false,
"searchKey": "testdata.main",
"tags": [
"function",
"private"
]
},
"label": {
"kind": "plaintext",
"value": "func main()"
},
"detail": {
"kind": "markdown",
"value": "```Go\nfunc main()\n```\n\n"
},
"children": null
}
},
{
"node": {
"pathID": "/#useOfCompositeStructs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ testdata is a small package containing sample Go source code used for testing th
* [Functions](#func)
* [func Parallel(ctx context.Context, fns ...ParallelizableFunc) error](#Parallel)
* [func Switch(interfaceValue interface{}) bool](#Switch)
* [func main()](#main)
* [func useOfCompositeStructs()](#useOfCompositeStructs)


Expand Down Expand Up @@ -687,6 +688,17 @@ tags: [function]
func Switch(interfaceValue interface{}) bool
```

### <a id="main" href="#main">func main()</a>

```
searchKey: testdata.main
tags: [function private]
```

```Go
func main()
```

### <a id="useOfCompositeStructs" href="#useOfCompositeStructs">func useOfCompositeStructs()</a>

```
Expand Down
9 changes: 9 additions & 0 deletions internal/testdata/minimal_main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package main

type User struct {
Id, Name string
}

type UserResource struct{}

func main() {}

0 comments on commit c0312b8

Please sign in to comment.