Skip to content

Commit 87060a6

Browse files
fix: Propagate errors when creating repro SCIP index (#296)
When trying to use repro with relationships, noticed that we would incorrectly set a bad symbol name instead of propagating the error in name resolution.
1 parent d1f063d commit 87060a6

File tree

9 files changed

+34
-15
lines changed

9 files changed

+34
-15
lines changed

cmd/scip/lint.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package main
22

33
import (
4-
"errors"
4+
stderrors "errors"
55
"fmt"
66
"sort"
77
"strings"
@@ -21,7 +21,7 @@ func lintCommand() cli.Command {
2121
Action: func(c *cli.Context) error {
2222
indexPath := c.Args().Get(0)
2323
if indexPath == "" {
24-
return errors.New("missing argument for path to SCIP index")
24+
return stderrors.New("missing argument for path to SCIP index")
2525
}
2626
return lintMain(indexPath)
2727
},
@@ -34,7 +34,7 @@ func lintMain(indexPath string) error {
3434
if err != nil {
3535
return err
3636
}
37-
return errors.Join(lintMainPure(scipIndex).data...)
37+
return stderrors.Join(lintMainPure(scipIndex).data...)
3838
}
3939

4040
func lintMainPure(scipIndex *scip.Index) errorSet {

cmd/scip/tests/reprolang/bindings/go/repro/ast.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"strings"
66

7+
"github.com/cockroachdb/errors"
78
sitter "github.com/smacker/go-tree-sitter"
89

910
"github.com/sourcegraph/scip/bindings/go/scip"
@@ -77,16 +78,17 @@ func NewRangePositionFromNode(node *sitter.Node) *scip.Range {
7778
}
7879
}
7980

80-
func (i *identifier) resolveSymbol(localScope *reproScope, context *reproContext) {
81+
func (i *identifier) resolveSymbol(localScope *reproScope, context *reproContext) error {
8182
scope := context.globalScope
8283
if i.isLocalSymbol() {
8384
scope = localScope
8485
}
8586
symbol, ok := scope.names[i.value]
8687
if !ok {
87-
symbol = "local ERROR_UNRESOLVED_SYMBOL"
88+
return errors.Newf("could not resolve local symbol %q", i.value)
8889
}
8990
i.symbol = symbol
91+
return nil
9092
}
9193

9294
func (i *identifier) isLocalSymbol() bool {

cmd/scip/tests/reprolang/bindings/go/repro/indexer.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package repro
33
import (
44
"context"
55

6+
"github.com/cockroachdb/errors"
67
"github.com/sourcegraph/scip/bindings/go/scip"
78
)
89

@@ -69,8 +70,14 @@ func Index(
6970
}
7071

7172
// Phase 3: resolve names for references
73+
var allErrs error
7274
for _, file := range reproSources {
73-
file.resolveReferences(ctx)
75+
if err := file.resolveReferences(ctx); err != nil {
76+
allErrs = errors.CombineErrors(allErrs, errors.Wrapf(err, "file %q", file.Source.AbsolutePath))
77+
}
78+
}
79+
if allErrs != nil {
80+
return nil, allErrs
7481
}
7582

7683
// Phase 4: emit SCIP

cmd/scip/tests/reprolang/bindings/go/repro/namer.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package repro
33
import (
44
"fmt"
55

6+
"github.com/cockroachdb/errors"
67
"github.com/sourcegraph/scip/bindings/go/scip"
78
)
89

@@ -58,13 +59,14 @@ func (s *reproSourceFile) enterDefinitions(context *reproContext) {
5859
}
5960

6061
// resolveReferences updates the .symbol field for all names of reference identifiers.
61-
func (s *reproSourceFile) resolveReferences(context *reproContext) {
62+
func (s *reproSourceFile) resolveReferences(context *reproContext) error {
63+
var err error
6264
resolveIdents := func(rel relationships) {
6365
for _, ident := range rel.identifiers() {
6466
if ident == nil {
6567
continue
6668
}
67-
ident.resolveSymbol(s.localScope, context)
69+
err = errors.CombineErrors(err, ident.resolveSymbol(s.localScope, context))
6870
}
6971
}
7072
for _, def := range s.definitions {
@@ -74,8 +76,9 @@ func (s *reproSourceFile) resolveReferences(context *reproContext) {
7476
resolveIdents(rel.relations)
7577
}
7678
for _, ref := range s.references {
77-
ref.name.resolveSymbol(s.localScope, context)
79+
err = errors.CombineErrors(err, ref.name.resolveSymbol(s.localScope, context))
7880
}
81+
return err
7982
}
8083

8184
// newGlobalSymbol returns an SCIP symbol for the given definition.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
definition hello().
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
reference localExample
2-
1+
definition localExample
2+
reference localExample
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Reference a global symbol from another workspace.
22
reference global global-workspace hello.repro/hello().
3-
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference local ERROR_UNRESOLVED_SYMBOL
3+
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference global-workspace hello.repro/hello().
44
reference global duplicates duplicate.repro/readFileSync.
55
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference duplicates duplicate.repro/readFileSync.
66

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
definition hello().
2+
# ^^^^^^^^ definition hello.repro/hello().
3+
# documentation
4+
# > signature of hello().
Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
definition localExample
2+
# ^^^^^^^^^^^^ definition local Example
3+
# documentation
4+
# > signature of localExample
15
reference localExample
2-
# ^^^^^^^^^^^^ reference local ERROR_UNRESOLVED_SYMBOL
3-
4-
6+
# ^^^^^^^^^^^^ reference local Example

0 commit comments

Comments
 (0)