Repository: G-Rath/osv-detector
Pull request #85 (2022-04-23 23:36:26)

fix: validate `--parse-as` to ensure error output in case a directory is passed
Currently if you do `osv-detector --parse-as my-file path/to/directory`, we error with "You must provide at least one path to ..." because we've failed to find at least one file that we can parse.

This doesn't make sense because the user has attempted to provide a path to a directory containing at least one lockfile, and really the issue is that they've told us to parse the files as something we don't support parsing.

This resolves that by having us check if we do actually support parsing files in the format the user has specified, before attempting to identify which files we should actually be checking. It also felt a bit mean to not provide the user with an explicit list of valid inputs for `--parse-as` so I switched to using a map instead of a switch (which is a bit better all around anyway).

Finally, this is using a private global variable because the alternative (afaik) would be to use a function which while inexpensive still feels a bit dirty when a static "readonly" map is very much what we want.


Patch:
diff --git a/internal/lockfile/parse.go b/internal/lockfile/parse.go
index 482fe9d..6080d38 100644
--- a/internal/lockfile/parse.go
+++ b/internal/lockfile/parse.go
@@ -13,30 +13,33 @@ func FindParser(pathToLockfile string, parseAs string) (PackageDetailsParser, st
 		parseAs = path.Base(pathToLockfile)
 	}
 
-	return findParser(parseAs), parseAs
+	return parsers[parseAs], parseAs
 }
 
-func findParser(pathToLockfile string) PackageDetailsParser {
-	switch pathToLockfile {
-	case "cargo.lock":
-		return ParseCargoLock
-	case "composer.lock":
-		return ParseComposerLock
-	case "Gemfile.lock":
-		return ParseGemfileLock
-	case "package-lock.json":
-		return ParseNpmLock
-	case "yarn.lock":
-		return ParseYarnLock
-	case "go.mod":
-		return ParseGoLock
-	case "pnpm-lock.yaml":
-		return ParsePnpmLock
-	case "requirements.txt":
-		return ParseRequirementsTxt
-	default:
-		return nil
+// nolint:gochecknoglobals // this is an optimisation and read-only
+var parsers = map[string]PackageDetailsParser{
+	"cargo.lock":        ParseCargoLock,
+	"composer.lock":     ParseComposerLock,
+	"Gemfile.lock":      ParseGemfileLock,
+	"go.mod":            ParseGoLock,
+	"package-lock.json": ParseNpmLock,
+	"pnpm-lock.yaml":    ParsePnpmLock,
+	"requirements.txt":  ParseRequirementsTxt,
+	"yarn.lock":         ParseYarnLock,
+}
+
+func ListParsers() []string {
+	ps := make([]string, 0, len(parsers))
+
+	for s := range parsers {
+		ps = append(ps, s)
 	}
+
+	sort.Slice(ps, func(i, j int) bool {
+		return strings.ToLower(ps[i]) < strings.ToLower(ps[j])
+	})
+
+	return ps
 }
 
 var ErrParserNotFound = errors.New("could not determine parser")
diff --git a/main.go b/main.go
index a4a0375..2642c5c 100644
--- a/main.go
+++ b/main.go
@@ -20,7 +20,7 @@ var (
 	date    = "unknown"
 )
 
-func printDatabaseLoadErr(r *reporter.Reporter, err error) int {
+func printDatabaseLoadErr(r *reporter.Reporter, err error) {
 	msg := err.Error()
 
 	if errors.Is(err, database.ErrOfflineDatabaseNotFound) {
@@ -28,8 +28,6 @@ func printDatabaseLoadErr(r *reporter.Reporter, err error) int {
 	}
 
 	r.PrintError(fmt.Sprintf(" %s\n", color.RedString("failed: %s", msg)))
-
-	return 127
 }
 
 func printKnownEcosystems(r *reporter.Reporter) {
@@ -180,7 +178,9 @@ func run() int {
 		err := cacheAllEcosystemDatabases(r)
 
 		if err != nil {
-			return printDatabaseLoadErr(r, err)
+			printDatabaseLoadErr(r, err)
+
+			return 127
 		}
 
 		return 0
@@ -192,6 +192,18 @@ func run() int {
 		return 0
 	}
 
+	if *parseAs != "" {
+		if parser, parsedAs := lockfile.FindParser("", *parseAs); parser == nil {
+			r.PrintError(fmt.Sprintf("Don't know how to parse files as \"%s\" - supported values are:\n", parsedAs))
+
+			for _, s := range lockfile.ListParsers() {
+				r.PrintError(fmt.Sprintf("  %s\n", s))
+			}
+
+			return 127
+		}
+	}
+
 	pathsToLocks := findAllLockfiles(r, flag.Args(), *parseAs)
 
 	if len(pathsToLocks) == 0 {
@@ -199,7 +211,7 @@ func run() int {
 			"You must provide at least one path to either a lockfile or a directory containing a lockfile (see --help for usage and flags)\n",
 		)
 
-		return 1
+		return 127
 	}
 
 	exitCode := 0
@@ -233,7 +245,8 @@ func run() int {
 		dbs, err := loadEcosystemDatabases(r, lockf.Packages.Ecosystems(), *offline)
 
 		if err != nil {
-			exitCode = printDatabaseLoadErr(r, err)
+			printDatabaseLoadErr(r, err)
+			exitCode = 127
 
 			continue
 		}


Test patch:
diff --git a/internal/lockfile/parse_test.go b/internal/lockfile/parse_test.go
index e21350b..8187ed9 100644
--- a/internal/lockfile/parse_test.go
+++ b/internal/lockfile/parse_test.go
@@ -119,6 +119,20 @@ func TestParse_ParserNotFound(t *testing.T) {
 	}
 }
 
+func TestListParsers(t *testing.T) {
+	t.Parallel()
+
+	parsers := lockfile.ListParsers()
+
+	if first := parsers[0]; first != "cargo.lock" {
+		t.Errorf("Expected first element to be cargo.lock, but got %s", first)
+	}
+
+	if last := parsers[len(parsers)-1]; last != "yarn.lock" {
+		t.Errorf("Expected last element to be requirements.txt, but got %s", last)
+	}
+}
+
 func TestLockfile_ToString(t *testing.T) {
 	t.Parallel()