Snapshot: don't fail on unparseable symbols. (#118)

* Snapshot: don't fail on unparseable symbols.

Previously, the `scip snapshot` command failed if there was a symbol
that wasn't parseable. This was unhelpful when trying to debug why
navigation was not working as expected (which is the main situation
where I use `scip snapshot`). Unparseable symbols are not invalid,
they just don't work with cross-repo navigation. To fail fast
like before, there's a new `scip snapshot --strict` flag that
fails if there's a parser error.

* Enable strict by default and explain how to disable strict

Author: olafurpg

bindings/go/scip/symbol_formatter.go

@@ -8,6 +8,7 @@ import (
 // Excluding parts of the symbol can be helpful for testing purposes. For example, snapshot tests may hardcode
 // the package version number so it's easier to read the snapshot tests if the version is excluded.
 type SymbolFormatter struct {
+	OnError               func(err error) error
 	IncludeScheme         func(scheme string) bool
 	IncludePackageManager func(manager string) bool
 	IncludePackageName    func(name string) bool
@@ -17,6 +18,17 @@ type SymbolFormatter struct {
 
 // VerboseSymbolFormatter formats all parts of the symbol.
 var VerboseSymbolFormatter = SymbolFormatter{
+	OnError:               func(err error) error { return err },
+	IncludeScheme:         func(_ string) bool { return true },
+	IncludePackageManager: func(_ string) bool { return true },
+	IncludePackageName:    func(_ string) bool { return true },
+	IncludePackageVersion: func(_ string) bool { return true },
+	IncludeDescriptor:     func(_ string) bool { return true },
+}
+
+// Same as VerboseSymbolFormatter but silently ignores errors.
+var LenientVerboseSymbolFormatter = SymbolFormatter{
+	OnError:               func(_ error) error { return nil },
 	IncludeScheme:         func(_ string) bool { return true },
 	IncludePackageManager: func(_ string) bool { return true },
 	IncludePackageName:    func(_ string) bool { return true },
@@ -26,6 +38,7 @@ var VerboseSymbolFormatter = SymbolFormatter{
 
 // DescriptorOnlyFormatter formats only the descriptor part of the symbol.
 var DescriptorOnlyFormatter = SymbolFormatter{
+	OnError:               func(err error) error { return err },
 	IncludeScheme:         func(scheme string) bool { return scheme == "local" },
 	IncludePackageManager: func(_ string) bool { return false },
 	IncludePackageName:    func(_ string) bool { return false },

bindings/go/scip/testutil/format.go

@@ -24,10 +24,16 @@ func FormatSnapshots(
 	if err != nil {
 		return nil, err
 	}
+
+	var documentErrors error
 	for _, document := range index.Documents {
 		snapshot, err := FormatSnapshot(document, index, commentSyntax, symbolFormatter)
+		err = symbolFormatter.OnError(err)
 		if err != nil {
-			return nil, err
+			documentErrors = errors.CombineErrors(
+				documentErrors,
+				errors.Wrap(err, document.RelativePath),
+			)
 		}
 		sourceFile := scip.NewSourceFile(
 			filepath.Join(projectRoot.Path, document.RelativePath),
@@ -36,6 +42,9 @@ func FormatSnapshots(
 		)
 		result = append(result, sourceFile)
 	}
+	if documentErrors != nil {
+		return nil, documentErrors
+	}
 	return result, nil
 }
 
@@ -68,7 +77,7 @@ func FormatSnapshot(
 		formatted, err := formatter.Format(symbol)
 		if err != nil {
 			formattingError = errors.CombineErrors(formattingError, errors.Wrapf(err, symbol))
-			return err.Error()
+			return symbol
 		}
 		return formatted
 	}

cmd/snapshot.go

@@ -9,11 +9,13 @@ import (
 
 	"github.com/sourcegraph/scip/bindings/go/scip"
 	"github.com/sourcegraph/scip/bindings/go/scip/testutil"
+	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
 type snapshotFlags struct {
 	from   string
 	output string
+	strict bool
 }
 
 func snapshotCommand() cli.Command {
@@ -31,7 +33,13 @@ and symbol information.`,
 				Name:        "to",
 				Usage:       "Path to output directory for snapshot files",
 				Destination: &snapshotFlags.output,
-				Value: "scip-snapshot",
+				Value:       "scip-snapshot",
+			},
+			&cli.BoolFlag{
+				Name:        "strict",
+				Usage:       "If true, fail fast on errors",
+				Destination: &snapshotFlags.strict,
+				Value:       true,
 			},
 		},
 		Action: func(c *cli.Context) error {
@@ -52,7 +60,14 @@ func snapshotMain(flags snapshotFlags) error {
 	if err != nil {
 		return err
 	}
-	snapshots, err := testutil.FormatSnapshots(index, "//", scip.VerboseSymbolFormatter)
+	symbolFormatter := scip.LenientVerboseSymbolFormatter
+	if flags.strict {
+		symbolFormatter = scip.VerboseSymbolFormatter
+		symbolFormatter.OnError = func(err error) error {
+			return errors.Wrap(err, "use --strict=false to ignore this error")
+		}
+	}
+	snapshots, err := testutil.FormatSnapshots(index, "//", symbolFormatter)
 	if err != nil {
 		return err
 	}