Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: skip exported symbols with no definition #72

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions bindings/go/scip/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ import (
// such as asymmetric references/definitions.
//
// This conversion functionality is used by src-cli.
func ConvertSCIPToLSIF(index *Index) ([]reader.Element, error) {
func ConvertSCIPToLSIF(index *Index) ([]reader.Element, map[string]IndexDiagnostic, error) {
g := newGraph()

if index.Metadata == nil {
return nil, errors.New(".Metadata is nil")
return nil, nil, errors.New(".Metadata is nil")
}
if index.Metadata.ToolInfo == nil {
return nil, errors.New(".Metadata.ToolInfo is nil")
return nil, nil, errors.New(".Metadata.ToolInfo is nil")
}

positionEncoding := ""
Expand All @@ -49,7 +49,7 @@ func ConvertSCIPToLSIF(index *Index) ([]reader.Element, error) {
case TextEncoding_UTF16:
positionEncoding = "utf-16"
default:
return nil, errors.New(".Metadata.TextDocumentEncoding does not have value utf-8 or utf-16")
return nil, nil, errors.New(".Metadata.TextDocumentEncoding does not have value utf-8 or utf-16")
}

g.emitVertex(
Expand All @@ -65,12 +65,28 @@ func ConvertSCIPToLSIF(index *Index) ([]reader.Element, error) {
},
)

// Emit a warning with the symbols for a document that were exported
// but had no definition. Only display one message so that there are fewer warnings
// displayed to users.
indexDiagnostics := map[string]IndexDiagnostic{}

// Pass 1: create result sets for global symbols.
for _, importedSymbol := range index.ExternalSymbols {
g.symbolToResultSet[importedSymbol.Symbol] = g.emitResultSet(importedSymbol, "import")
}
for _, document := range index.Documents {
for _, exportedSymbol := range document.Symbols {
// If an exported symbol is not defined in a document,
// do not emit a definition for this symbol.
if !document.HasDefinition(exportedSymbol) {
diag := NoSymbolDefinitionDiagnostic{
symbol: exportedSymbol,
}
indexDiagnostics[diag.Key()] = diag

continue
}

g.registerInverseRelationships(exportedSymbol)
if IsGlobalSymbol(exportedSymbol.Symbol) {
// Local symbols are skipped here because we handle them in the
Expand All @@ -97,7 +113,7 @@ func ConvertSCIPToLSIF(index *Index) ([]reader.Element, error) {
g.emitDocument(index, document)
}

return g.Elements, nil
return g.Elements, indexDiagnostics, nil
}

// graph is a helper struct to emit LSIF.
Expand Down
11 changes: 11 additions & 0 deletions bindings/go/scip/document.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package scip

func (d *Document) HasDefinition(symbol *SymbolInformation) bool {
for _, occ := range d.Occurrences {
if occ.Symbol == symbol.Symbol && SymbolRole_Definition.Matches(occ) {
return true
}
}

return false
}
39 changes: 39 additions & 0 deletions bindings/go/scip/index_diagnostics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package scip

import "fmt"

type IndexDiagnosticType int

const (
ExportedSymbolsWithNoDefinition IndexDiagnosticType = iota
)

type IndexDiagnostic interface {
// Key returns a unique key for this diagnostic. It allows for easy deduplication with a Map
Key() string

// DiagnosticType returns the index type. Can be used to switch case over different types
// if we wanted to do anything special with them.
DiagnosticType() IndexDiagnosticType

// Message to display to user if calling code wants to display any information
Message() string
}

// NoSymbolDefinitionDiagnostic is used when a Symbol was exported by a document,
// but there was no corresponding definition occurrence in the document.
//
// We discard these symbols in the indexer, but with this we can warn the user as well.
type NoSymbolDefinitionDiagnostic struct {
symbol *SymbolInformation
}

func (NoSymbolDefinitionDiagnostic) DiagnosticType() IndexDiagnosticType {
return ExportedSymbolsWithNoDefinition
}
func (diag NoSymbolDefinitionDiagnostic) Message() string {
return fmt.Sprintf("No Definition For Exported Symbol: %s", diag.symbol.Symbol)
}
func (diag NoSymbolDefinitionDiagnostic) Key() string {
return fmt.Sprintf("%d:%s", diag.DiagnosticType(), diag.symbol.Symbol)
}
13 changes: 11 additions & 2 deletions cmd/convert.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"fmt"
"io"
"os"
"strings"
Expand Down Expand Up @@ -29,7 +30,7 @@ func convertCommand() cli.Command {
Name: "to",
Usage: "Output path for LSIF index",
Destination: &convertFlags.to,
Value: "dump.lsif",
Value: "dump.lsif",
},
},
Action: func(c *cli.Context) error {
Expand Down Expand Up @@ -60,11 +61,19 @@ func convertMain(flags convertFlags) error {
lsifWriter = lsifFile
}

lsifIndex, err := scip.ConvertSCIPToLSIF(scipIndex)
lsifIndex, diagnostics, err := scip.ConvertSCIPToLSIF(scipIndex)
if err != nil {
return errors.Wrap(err, "failed to convert SCIP index to LSIF index")
}

if len(diagnostics) > 0 {
fmt.Println("The following warnings occurred while processing SCIP index:")
fmt.Println("====================")
for _, diag := range diagnostics {
fmt.Println(diag.Message())
}
}

err = reader.WriteNDJSON(reader.ElementsToJsonElements(lsifIndex), lsifWriter)
if err != nil {
return errors.Wrapf(err, "failed to write LSIF index to path %s", toPath)
Expand Down
2 changes: 1 addition & 1 deletion cmd/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestSCIPSnapshots(t *testing.T) {
snapshots, err := testutil.FormatSnapshots(index, "#", symbolFormatter)
require.Nil(t, err)
index.Metadata.ProjectRoot = "file:/root"
lsif, err := scip.ConvertSCIPToLSIF(index)
lsif, _, err := scip.ConvertSCIPToLSIF(index)
require.Nil(t, err)
var obtained bytes.Buffer
err = reader.WriteNDJSON(reader.ElementsToJsonElements(lsif), &obtained)
Expand Down