diff --git a/archives.go b/archives.go index 2278bbd..b7ac771 100644 --- a/archives.go +++ b/archives.go @@ -48,6 +48,7 @@ type Reader interface { // Open creates an archive reader for the given content. // The filename is used to detect the archive format. // The content reader will be read entirely into memory. +//nolint:ireturn // factory function returning interface by design func Open(filename string, content io.Reader) (Reader, error) { format := detectFormat(filename) if format == "" { @@ -79,6 +80,7 @@ func Open(filename string, content io.Reader) (Reader, error) { // OpenWithPrefix opens an archive and strips the given prefix from all paths. // This is useful for npm packages which wrap content in a "package/" directory. +//nolint:ireturn // factory function returning interface by design func OpenWithPrefix(filename string, content io.Reader, stripPrefix string) (Reader, error) { reader, err := Open(filename, content) if err != nil { diff --git a/diff/diff.go b/diff/diff.go index 83df0d7..aa07fb5 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -12,6 +12,17 @@ import ( "github.com/git-pkgs/archives" ) +const ( + // binaryCheckSize is the number of bytes to check for null bytes when + // determining if content is binary. + binaryCheckSize = 8192 + + // Diff type constants for FileDiff.Type. + TypeModified = "modified" + TypeAdded = "added" + TypeDeleted = "deleted" +) + // FileDiff represents the diff for a single file. type FileDiff struct { Path string `json:"path"` @@ -87,67 +98,71 @@ func Compare(oldReader, newReader archives.Reader) (*CompareResult, error) { oldExists := oldMap[path] newExists := newMap[path] - var fileDiff FileDiff + fileDiff, ok := compareFile(path, oldExists, newExists, oldReader, newReader) + if !ok { + continue + } - if oldExists.Path != "" && newExists.Path == "" { - // File was deleted - fileDiff = FileDiff{ - Path: path, - Type: "deleted", - } + switch fileDiff.Type { + case TypeDeleted: result.FilesDeleted++ - } else if oldExists.Path == "" && newExists.Path != "" { - // File was added - fileDiff = FileDiff{ - Path: path, - Type: "added", - } + case TypeAdded: result.FilesAdded++ + case TypeModified: + result.FilesChanged++ + result.TotalAdded += fileDiff.LinesAdded + result.TotalDeleted += fileDiff.LinesDeleted + } - // Try to get content for added files - if content, err := readFileContent(newReader, path); err == nil { - if isBinary(content) { - fileDiff.IsBinary = true - } else { - fileDiff.Diff = generateAddedDiff(path, content) - fileDiff.LinesAdded = countLines(content) - } - } - } else { - // File exists in both - check if modified - oldContent, err1 := readFileContent(oldReader, path) - newContent, err2 := readFileContent(newReader, path) - - if err1 != nil || err2 != nil { - continue // Skip files we can't read - } - - if bytes.Equal(oldContent, newContent) { - continue // No change - } + result.Files = append(result.Files, fileDiff) + } - fileDiff = FileDiff{ - Path: path, - Type: "modified", - } - result.FilesChanged++ + return result, nil +} - if isBinary(oldContent) || isBinary(newContent) { - fileDiff.IsBinary = true +// compareFile compares a single file between old and new archives. +// Returns the diff and false if the file should be skipped. +func compareFile(path string, oldInfo, newInfo archives.FileInfo, oldReader, newReader archives.Reader) (FileDiff, bool) { + inOld := oldInfo.Path != "" + inNew := newInfo.Path != "" + + switch { + case inOld && !inNew: + return FileDiff{Path: path, Type: TypeDeleted}, true + + case !inOld && inNew: + fd := FileDiff{Path: path, Type: TypeAdded} + if content, err := readFileContent(newReader, path); err == nil { + if isBinary(content) { + fd.IsBinary = true } else { - diffText, added, deleted := generateUnifiedDiff(path, oldContent, newContent) - fileDiff.Diff = diffText - fileDiff.LinesAdded = added - fileDiff.LinesDeleted = deleted - result.TotalAdded += added - result.TotalDeleted += deleted + fd.Diff = generateAddedDiff(path, content) + fd.LinesAdded = countLines(content) } } + return fd, true - result.Files = append(result.Files, fileDiff) - } + default: + oldContent, err1 := readFileContent(oldReader, path) + newContent, err2 := readFileContent(newReader, path) + if err1 != nil || err2 != nil { + return FileDiff{}, false + } + if bytes.Equal(oldContent, newContent) { + return FileDiff{}, false + } - return result, nil + fd := FileDiff{Path: path, Type: TypeModified} + if isBinary(oldContent) || isBinary(newContent) { + fd.IsBinary = true + } else { + diffText, added, deleted := generateUnifiedDiff(path, oldContent, newContent) + fd.Diff = diffText + fd.LinesAdded = added + fd.LinesDeleted = deleted + } + return fd, true + } } // readFileContent reads a file's content from an archive reader. @@ -169,8 +184,8 @@ func isBinary(content []byte) bool { // Check first 8KB for null bytes checkLen := len(content) - if checkLen > 8192 { - checkLen = 8192 + if checkLen > binaryCheckSize { + checkLen = binaryCheckSize } for i := 0; i < checkLen; i++ { diff --git a/diff/diff_test.go b/diff/diff_test.go index c25bea0..52c1479 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -82,21 +82,21 @@ func TestCompare(t *testing.T) { } // Check deleted file - if f, ok := fileMap["deleted.txt"]; !ok || f.Type != "deleted" { + if f, ok := fileMap["deleted.txt"]; !ok || f.Type != TypeDeleted { t.Error("deleted.txt should be marked as deleted") } // Check added file - if f, ok := fileMap["added.txt"]; !ok || f.Type != "added" { + if f, ok := fileMap["added.txt"]; !ok || f.Type != TypeAdded { t.Error("added.txt should be marked as added") } // Check modified files - if f, ok := fileMap["README.md"]; !ok || f.Type != "modified" { + if f, ok := fileMap["README.md"]; !ok || f.Type != TypeModified { t.Error("README.md should be marked as modified") } - if f, ok := fileMap["src/main.go"]; !ok || f.Type != "modified" { + if f, ok := fileMap["src/main.go"]; !ok || f.Type != TypeModified { t.Error("src/main.go should be marked as modified") } } diff --git a/gem.go b/gem.go index 297e525..b6c97fa 100644 --- a/gem.go +++ b/gem.go @@ -13,7 +13,7 @@ type gemReader struct { dataReader Reader // The inner data.tar.gz reader } -func openGem(content io.Reader) (Reader, error) { +func openGem(content io.Reader) (*gemReader, error) { // Read the gem file as a tar archive tr := tar.NewReader(content) diff --git a/tar.go b/tar.go index 307f46f..6c50884 100644 --- a/tar.go +++ b/tar.go @@ -21,7 +21,7 @@ type tarFileEntry struct { data []byte } -func openTar(content io.Reader, compression string) (Reader, error) { +func openTar(content io.Reader, compression string) (*tarReader, error) { // Wrap with decompressor if needed r := io.Reader(content) diff --git a/zip.go b/zip.go index 4e55671..09e1bd9 100644 --- a/zip.go +++ b/zip.go @@ -13,7 +13,7 @@ type zipReader struct { reader *zip.Reader } -func openZip(content io.Reader) (Reader, error) { +func openZip(content io.Reader) (*zipReader, error) { // Read entire content into memory data, err := io.ReadAll(content) if err != nil {