Repository: G-Rath/osv-detector
Pull request #9 (2022-03-03 23:45:54)

fix: improve "file not found" handling
By which I mean, actually take it into account 😂 


Patch:
diff --git a/detector/parsers/parse-composer-lock.go b/detector/parsers/parse-composer-lock.go
index a61fbd2..c3f2042 100644
--- a/detector/parsers/parse-composer-lock.go
+++ b/detector/parsers/parse-composer-lock.go
@@ -22,12 +22,16 @@ func ParseComposerLock(pathToLockfile string) ([]PackageDetails, error) {
 	var packages []PackageDetails
 	var parsedLockfile *ComposerLock
 
-	if lockfileContents, err := ioutil.ReadFile(pathToLockfile); err == nil {
-		err := json.Unmarshal(lockfileContents, &parsedLockfile)
+	lockfileContents, err := ioutil.ReadFile(pathToLockfile)
 
-		if err != nil {
-			return packages, fmt.Errorf("could not parse %s: %w", pathToLockfile, err)
-		}
+	if err != nil {
+		return packages, fmt.Errorf("could not read %s: %w", pathToLockfile, err)
+	}
+
+	err = json.Unmarshal(lockfileContents, &parsedLockfile)
+
+	if err != nil {
+		return packages, fmt.Errorf("could not parse %s: %w", pathToLockfile, err)
 	}
 
 	for _, composerPackage := range parsedLockfile.Packages {
diff --git a/detector/parsers/parse-npm-lock.go b/detector/parsers/parse-npm-lock.go
index 35037e0..536c31c 100644
--- a/detector/parsers/parse-npm-lock.go
+++ b/detector/parsers/parse-npm-lock.go
@@ -111,12 +111,16 @@ func parseNpmLock(lockfile NpmLockfile) map[string]PackageDetails {
 func ParseNpmLock(pathToLockfile string) ([]PackageDetails, error) {
 	var parsedLockfile *NpmLockfile
 
-	if lockfileContents, err := ioutil.ReadFile(pathToLockfile); err == nil {
-		err := json.Unmarshal(lockfileContents, &parsedLockfile)
+	lockfileContents, err := ioutil.ReadFile(pathToLockfile)
 
-		if err != nil {
-			return []PackageDetails{}, fmt.Errorf("could not parse %s: %w", pathToLockfile, err)
-		}
+	if err != nil {
+		return []PackageDetails{}, fmt.Errorf("could not read %s: %w", pathToLockfile, err)
+	}
+
+	err = json.Unmarshal(lockfileContents, &parsedLockfile)
+
+	if err != nil {
+		return []PackageDetails{}, fmt.Errorf("could not parse %s: %w", pathToLockfile, err)
 	}
 
 	return pkgDetailsMapToSlice(parseNpmLock(*parsedLockfile)), nil
diff --git a/detector/parsers/parse-requirements-txt.go b/detector/parsers/parse-requirements-txt.go
index 47b12ac..5231147 100644
--- a/detector/parsers/parse-requirements-txt.go
+++ b/detector/parsers/parse-requirements-txt.go
@@ -3,7 +3,6 @@ package parsers
 import (
 	"bufio"
 	"fmt"
-	"log"
 	"os"
 	"regexp"
 	"strings"
@@ -92,7 +91,7 @@ func ParseRequirementsTxt(pathToLockfile string) ([]PackageDetails, error) {
 	}
 
 	if err := scanner.Err(); err != nil {
-		log.Fatal(err)
+		return packages, fmt.Errorf("error while scanning %s: %w", pathToLockfile, err)
 	}
 
 	return packages, nil


Test patch:
diff --git a/detector/parsers/helpers_test.go b/detector/parsers/helpers_test.go
index 0568549..cd258b1 100644
--- a/detector/parsers/helpers_test.go
+++ b/detector/parsers/helpers_test.go
@@ -3,9 +3,22 @@ package parsers_test
 import (
 	"fmt"
 	"osv-detector/detector/parsers"
+	"strings"
 	"testing"
 )
 
+func expectErrContaining(t *testing.T, err error, str string) {
+	t.Helper()
+
+	if err == nil {
+		t.Errorf("Expected to get error, but did not")
+	}
+
+	if !strings.Contains(err.Error(), str) {
+		t.Errorf("Expected to get \"%s\" error, but got \"%v\"", str, err)
+	}
+}
+
 func packageToString(pkg parsers.PackageDetails) string {
 	return fmt.Sprintf("%s@%s (%s)", pkg.Name, pkg.Version, pkg.Ecosystem)
 }
diff --git a/detector/parsers/parse-composer-lock_test.go b/detector/parsers/parse-composer-lock_test.go
index adc5777..2f3cce2 100644
--- a/detector/parsers/parse-composer-lock_test.go
+++ b/detector/parsers/parse-composer-lock_test.go
@@ -5,15 +5,21 @@ import (
 	"testing"
 )
 
+func TestParseComposerLock_FileDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	packages, err := parsers.ParseComposerLock("fixtures/composer/does-not-exist")
+
+	expectErrContaining(t, err, "could not read")
+	expectPackages(t, packages, []parsers.PackageDetails{})
+}
+
 func TestParseComposerLock_InvalidJson(t *testing.T) {
 	t.Parallel()
 
 	packages, err := parsers.ParseComposerLock("fixtures/composer/not-json.txt")
 
-	if err == nil {
-		t.Errorf("Expected to get error, but did not")
-	}
-
+	expectErrContaining(t, err, "could not parse")
 	expectPackages(t, packages, []parsers.PackageDetails{})
 }
 
diff --git a/detector/parsers/parse-gemfile-lock_test.go b/detector/parsers/parse-gemfile-lock_test.go
index 47f6c0c..8f3ad01 100644
--- a/detector/parsers/parse-gemfile-lock_test.go
+++ b/detector/parsers/parse-gemfile-lock_test.go
@@ -5,6 +5,15 @@ import (
 	"testing"
 )
 
+func TestParseGemfileLock_FileDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	packages, err := parsers.ParseGemfileLock("fixtures/bundler/does-not-exist")
+
+	expectErrContaining(t, err, "could not read")
+	expectPackages(t, packages, []parsers.PackageDetails{})
+}
+
 func TestParseGemfileLock_InvalidJson(t *testing.T) {
 	t.Parallel()
 
diff --git a/detector/parsers/parse-npm-lock-v1_test.go b/detector/parsers/parse-npm-lock-v1_test.go
index d8d2171..5bcb1ee 100644
--- a/detector/parsers/parse-npm-lock-v1_test.go
+++ b/detector/parsers/parse-npm-lock-v1_test.go
@@ -5,15 +5,21 @@ import (
 	"testing"
 )
 
+func TestParseNpmLock_v1_FileDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	packages, err := parsers.ParseNpmLock("fixtures/npm/does-not-exist")
+
+	expectErrContaining(t, err, "could not read")
+	expectPackages(t, packages, []parsers.PackageDetails{})
+}
+
 func TestNpmLock_v1_InvalidJson(t *testing.T) {
 	t.Parallel()
 
 	packages, err := parsers.ParseNpmLock("fixtures/npm/not-json.txt")
 
-	if err == nil {
-		t.Errorf("Expected to get error, but did not")
-	}
-
+	expectErrContaining(t, err, "could not parse")
 	expectPackages(t, packages, []parsers.PackageDetails{})
 }
 
diff --git a/detector/parsers/parse-npm-lock-v2_test.go b/detector/parsers/parse-npm-lock-v2_test.go
index c691e18..3940048 100644
--- a/detector/parsers/parse-npm-lock-v2_test.go
+++ b/detector/parsers/parse-npm-lock-v2_test.go
@@ -5,15 +5,21 @@ import (
 	"testing"
 )
 
+func TestParseNpmLock_v2_FileDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	packages, err := parsers.ParseNpmLock("fixtures/npm/does-not-exist")
+
+	expectErrContaining(t, err, "could not read")
+	expectPackages(t, packages, []parsers.PackageDetails{})
+}
+
 func TestNpmLock_v2_InvalidJson(t *testing.T) {
 	t.Parallel()
 
 	packages, err := parsers.ParseNpmLock("fixtures/npm/not-json.txt")
 
-	if err == nil {
-		t.Errorf("Expected to get error, but did not")
-	}
-
+	expectErrContaining(t, err, "could not parse")
 	expectPackages(t, packages, []parsers.PackageDetails{})
 }
 
diff --git a/detector/parsers/parse-requirements-txt_test.go b/detector/parsers/parse-requirements-txt_test.go
index f0fe526..28a7e2f 100644
--- a/detector/parsers/parse-requirements-txt_test.go
+++ b/detector/parsers/parse-requirements-txt_test.go
@@ -5,6 +5,15 @@ import (
 	"testing"
 )
 
+func TestParseRequirementsTxt_FileDoesNotExist(t *testing.T) {
+	t.Parallel()
+
+	packages, err := parsers.ParseRequirementsTxt("fixtures/pip/does-not-exist")
+
+	expectErrContaining(t, err, "could not open")
+	expectPackages(t, packages, []parsers.PackageDetails{})
+}
+
 func TestParseRequirementsTxt_Empty(t *testing.T) {
 	t.Parallel()