internal/symbols: remove logging from Patched

Bubble up the errors instead of passing a logging function.

Change-Id: Id55c3588170ae822b09ede3ed0e0224089be147b
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/560777
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Maceo Thompson <[email protected]>
Reviewed-by: Zvonimir Pavlinovic <[email protected]>

Author: tatianab

cmd/vulnreport/symbols.go

@@ -56,7 +56,7 @@ func (s *symbolsCmd) run(ctx context.Context, filename string) (err error) {
 		for i, fixLink := range defaultFixes {
 			fixHash := filepath.Base(fixLink)
 			fixRepo := strings.TrimSuffix(fixLink, "/commit/"+fixHash)
-			pkgsToSymbols, err := symbols.Patched(mod.Module, fixRepo, fixHash, log.Errf)
+			pkgsToSymbols, err := symbols.Patched(mod.Module, fixRepo, fixHash)
 			if err != nil {
 				log.Err(err)
 				continue

internal/symbols/patched_functions.go

@@ -35,7 +35,7 @@ import (
 // patched in the package. Test packages and symbols are omitted.
 //
 // If the commit has more than one parent, an error is returned.
-func Patched(module, repoURL, commitHash string, errf logf) (_ map[string][]string, err error) {
+func Patched(module, repoURL, commitHash string) (_ map[string][]string, err error) {
 	defer derrors.Wrap(&err, "Patched(%s, %s, %s)", module, repoURL, commitHash)
 
 	repoRoot, err := os.MkdirTemp("", commitHash)
@@ -90,7 +90,10 @@ func Patched(module, repoURL, commitHash string, errf logf) (_ map[string][]stri
 		return nil, err
 	}
 
-	patched := patchedSymbols(oldSymbols, newSymbols, errf)
+	patched, err := patchedSymbols(oldSymbols, newSymbols)
+	if err != nil {
+		return nil, err
+	}
 	pkgSyms := make(map[string][]string)
 	for _, sym := range patched {
 		pkgSyms[sym.pkg] = append(pkgSyms[sym.pkg], sym.symbol)
@@ -101,7 +104,7 @@ func Patched(module, repoURL, commitHash string, errf logf) (_ map[string][]stri
 // patchedSymbols returns symbol indices in oldSymbols that either 1) cannot
 // be identified in newSymbols or 2) the corresponding functions have their
 // source code changed.
-func patchedSymbols(oldSymbols, newSymbols map[symKey]*ast.FuncDecl, errf logf) []symKey {
+func patchedSymbols(oldSymbols, newSymbols map[symKey]*ast.FuncDecl) ([]symKey, error) {
 	var syms []symKey
 	for key, of := range oldSymbols {
 		nf, ok := newSymbols[key]
@@ -112,23 +115,30 @@ func patchedSymbols(oldSymbols, newSymbols map[symKey]*ast.FuncDecl, errf logf)
 			continue
 		}
 
-		if source(of, errf) != source(nf, errf) {
+		osrc, err := source(of)
+		if err != nil {
+			return nil, err
+		}
+		nsrc, err := source(nf)
+		if err != nil {
+			return nil, err
+		}
+
+		if osrc != nsrc {
 			syms = append(syms, key)
 		}
 	}
-	return syms
+	return syms, nil
 }
 
 // source returns f's source code as text.
-func source(f *ast.FuncDecl, errf logf) string {
+func source(f *ast.FuncDecl) (string, error) {
 	var b bytes.Buffer
 	fs := token.NewFileSet()
 	if err := printer.Fprint(&b, fs, f); err != nil {
-		// should not happen, so just printing a warning
-		errf("getting source of %s failed with %v", astSymbolName(f), err)
-		return ""
+		return "", fmt.Errorf("getting source of %s failed: %w", astSymbolName(f), err)
 	}
-	return strings.TrimSpace(b.String())
+	return strings.TrimSpace(b.String()), nil
 }
 
 // moduleSymbols indexes all symbols of a module located

internal/symbols/patched_functions_test.go

@@ -45,9 +45,11 @@ func TestPatchedSymbols(t *testing.T) {
 		if err != nil {
 			t.Error(err)
 		}
-		// logs are not important for this test
-		discardLog := func(string, ...any) {}
-		got := toMap(patchedSymbols(oldSyms, newSyms, discardLog))
+		patched, err := patchedSymbols(oldSyms, newSyms)
+		if err != nil {
+			t.Fatal(err)
+		}
+		got := toMap(patched)
 		if diff := cmp.Diff(got, tc.want); diff != "" {
 			t.Errorf("(-got, want+):\n%s", diff)
 		}