Skip to content

Commit 52c78de

Browse files
committed
fix: address MaineK00n review feedback
- Rename golden files: .golden.json → .json (redundant under testdata/golden/) - Use cmp.Or for sort comparators in normalizeResult - Switch on langType in parseBinary to isolate sentinel errors per language - Wrap parseExecutableBinary errors with language name, symmetrize structure - Simplify dispatch.go: inline filepath.Base in switch, use slices.Contains - Extract base-runner.go from compare-lockfile.go via //go:embed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ce2bcdf commit 52c78de

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+245
-325
lines changed

scanner/analyze_golden_test.go

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ var lockfiles = []lockfileEntry{
9595
// e.g. "npm-v3/package-lock.json" -> "npm-v3_package-lock.json"
9696
// Uses filepath.ToSlash to normalize path separators across platforms.
9797
func goldenFileName(lockfilePath string) string {
98-
return strings.ReplaceAll(filepath.ToSlash(lockfilePath), "/", "_") + ".golden.json"
98+
return strings.ReplaceAll(filepath.ToSlash(lockfilePath), "/", "_") + ".json"
9999
}
100100

101101
func TestAnalyzeLibrary_Golden(t *testing.T) {
@@ -199,7 +199,7 @@ func TestAnalyzeLibrary_PomOnline(t *testing.T) {
199199
t.Fatalf("Failed to marshal result: %v", err)
200200
}
201201

202-
goldenPath := filepath.Join(goldenDir, "pom.xml.online.golden.json")
202+
goldenPath := filepath.Join(goldenDir, "pom.xml.online.json")
203203

204204
if *update {
205205
if err := os.MkdirAll(goldenDir, 0755); err != nil {
@@ -223,7 +223,7 @@ func TestAnalyzeLibrary_PomOnline(t *testing.T) {
223223
}
224224

225225
// Online mode should resolve transitive dependencies, producing more results than offline.
226-
offlineGoldenPath := filepath.Join(goldenDir, "pom.xml.golden.json")
226+
offlineGoldenPath := filepath.Join(goldenDir, "pom.xml.json")
227227
offlineJSON, err := os.ReadFile(offlineGoldenPath)
228228
if err != nil {
229229
t.Logf("Offline golden file not found, skipping comparison: %s", offlineGoldenPath)
@@ -268,13 +268,6 @@ type goldenLibrary struct {
268268
Dev bool `json:"dev,omitempty"`
269269
}
270270

271-
func boolToInt(b bool) int {
272-
if b {
273-
return 1
274-
}
275-
return 0
276-
}
277-
278271
func normalizeResult(scanners []models.LibraryScanner) []goldenLibraryScanner {
279272
result := make([]goldenLibraryScanner, 0, len(scanners))
280273
for _, s := range scanners {
@@ -294,30 +287,31 @@ func normalizeResult(scanners []models.LibraryScanner) []goldenLibraryScanner {
294287
})
295288
}
296289
slices.SortFunc(gs.Libs, func(a, b goldenLibrary) int {
297-
if c := cmp.Compare(a.Name, b.Name); c != 0 {
298-
return c
299-
}
300-
if c := cmp.Compare(a.Version, b.Version); c != 0 {
301-
return c
302-
}
303-
if c := cmp.Compare(a.PURL, b.PURL); c != 0 {
304-
return c
305-
}
306-
if c := cmp.Compare(a.FilePath, b.FilePath); c != 0 {
307-
return c
308-
}
309-
if c := cmp.Compare(a.Digest, b.Digest); c != 0 {
310-
return c
311-
}
312-
return cmp.Compare(boolToInt(a.Dev), boolToInt(b.Dev))
290+
return cmp.Or(
291+
cmp.Compare(a.Name, b.Name),
292+
cmp.Compare(a.Version, b.Version),
293+
cmp.Compare(a.PURL, b.PURL),
294+
cmp.Compare(a.FilePath, b.FilePath),
295+
cmp.Compare(a.Digest, b.Digest),
296+
func() int {
297+
switch {
298+
case !a.Dev && b.Dev:
299+
return -1
300+
case a.Dev && !b.Dev:
301+
return +1
302+
default:
303+
return 0
304+
}
305+
}(),
306+
)
313307
})
314308
result = append(result, gs)
315309
}
316310
slices.SortFunc(result, func(a, b goldenLibraryScanner) int {
317-
if c := cmp.Compare(a.Type, b.Type); c != 0 {
318-
return c
319-
}
320-
return cmp.Compare(a.LockfilePath, b.LockfilePath)
311+
return cmp.Or(
312+
cmp.Compare(a.Type, b.Type),
313+
cmp.Compare(a.LockfilePath, b.LockfilePath),
314+
)
321315
})
322316
return result
323317
}

scanner/base.go

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -871,13 +871,22 @@ func parseLockfile(ctx context.Context, langType ftypes.LangType, filePath strin
871871
func parseBinary(ctx context.Context, langType ftypes.LangType, filePath string, r xio.ReadSeekerAt, parser lockfileParser) (*ftypes.Application, error) {
872872
pkgs, _, err := parser.Parse(ctx, r)
873873
if err != nil {
874-
// Go binary parser: ErrUnrecognizedExe, ErrNonGoBinary
875-
// Rust binary parser: ErrUnrecognizedExe, ErrNonRustBinary
876-
if errors.Is(err, gobinary.ErrUnrecognizedExe) || errors.Is(err, gobinary.ErrNonGoBinary) ||
877-
errors.Is(err, rustbinary.ErrUnrecognizedExe) || errors.Is(err, rustbinary.ErrNonRustBinary) {
878-
return nil, nil
874+
switch langType {
875+
case ftypes.GoBinary:
876+
// Go binary parser: ErrUnrecognizedExe, ErrNonGoBinary
877+
if errors.Is(err, gobinary.ErrUnrecognizedExe) || errors.Is(err, gobinary.ErrNonGoBinary) {
878+
return nil, nil
879+
}
880+
return nil, xerrors.Errorf("parse error: %w", err)
881+
case ftypes.RustBinary:
882+
// Rust binary parser: ErrUnrecognizedExe, ErrNonRustBinary
883+
if errors.Is(err, rustbinary.ErrUnrecognizedExe) || errors.Is(err, rustbinary.ErrNonRustBinary) {
884+
return nil, nil
885+
}
886+
return nil, xerrors.Errorf("parse error: %w", err)
887+
default:
888+
return nil, xerrors.Errorf("unexpected language type for binary parser. expected: %q, actual: %q", []ftypes.LangType{ftypes.GoBinary, ftypes.RustBinary}, langType)
879889
}
880-
return nil, xerrors.Errorf("parse error: %w", err)
881890
}
882891
if len(pkgs) == 0 {
883892
return nil, nil
@@ -895,7 +904,7 @@ func parseExecutableBinary(ctx context.Context, filePath string, r xio.ReadSeeke
895904
// Try Go binary first
896905
app, err := parseBinary(ctx, ftypes.GoBinary, filePath, r, gobinary.NewParser())
897906
if err != nil {
898-
return nil, err
907+
return nil, xerrors.Errorf("Failed to parse Go binary: %w", err)
899908
}
900909
if app != nil {
901910
return app, nil
@@ -905,7 +914,16 @@ func parseExecutableBinary(ctx context.Context, filePath string, r xio.ReadSeeke
905914
if _, err := r.Seek(0, io.SeekStart); err != nil {
906915
return nil, xerrors.Errorf("seek error: %w", err)
907916
}
908-
return parseBinary(ctx, ftypes.RustBinary, filePath, r, rustbinary.NewParser())
917+
918+
app, err = parseBinary(ctx, ftypes.RustBinary, filePath, r, rustbinary.NewParser())
919+
if err != nil {
920+
return nil, xerrors.Errorf("Failed to parse Rust binary: %w", err)
921+
}
922+
if app != nil {
923+
return app, nil
924+
}
925+
926+
return nil, nil
909927
}
910928

911929
// parseYarn handles yarn.lock which has a different parser signature (4 return values including licenses).

scanner/dispatch.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package scanner
33
import (
44
"os"
55
"path/filepath"
6+
"slices"
67
"strings"
78

89
ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
@@ -45,10 +46,8 @@ const (
4546
// detectParserType determines which parser should handle a given file,
4647
// replicating the logic of each Trivy fanal analyzer's Required() method.
4748
func detectParserType(filePath string, filemode os.FileMode) parserType {
48-
fileName := filepath.Base(filePath)
49-
5049
// Exact filename matches (most common case)
51-
switch fileName {
50+
switch filepath.Base(filePath) {
5251
// Node.js
5352
case ftypes.NpmPkgLock: // package-lock.json
5453
if containsDir(filePath, "node_modules") {
@@ -159,12 +158,7 @@ func detectParserType(filePath string, filemode os.FileMode) parserType {
159158

160159
// containsDir checks if a filepath contains a specific directory component.
161160
func containsDir(filePath, dir string) bool {
162-
for _, part := range strings.Split(filepath.ToSlash(filePath), "/") {
163-
if part == dir {
164-
return true
165-
}
166-
}
167-
return false
161+
return slices.Contains(strings.Split(filepath.ToSlash(filePath), "/"), dir)
168162
}
169163

170164
// isExecutable checks if the file is an executable binary.
File renamed without changes.

scanner/testdata/golden/Directory.Packages.props.golden.json renamed to scanner/testdata/golden/Directory.Packages.props.json

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.

0 commit comments

Comments
 (0)