Merge pull request #4 from git-pkgs/fix-lint-issues

Fix lint issues from golangci-lint quality checks

Author: andrew

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/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/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")
 	}
 }

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)
 

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)
 

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 {