From c820f4aec3a280a9689d55b67fd8c2572a826865 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 30 May 2026 14:18:34 +0800 Subject: [PATCH 1/4] refactor: simplify and de-duplicate utility helpers - Delegate array Contains to the standard library slices package - Remove the unreachable pre-Go-1.20 bytesconv build-tag file - Add typed fast paths for string conversion to avoid reflection - Collapse redundant boolean return blocks in ToBool - Extract a shared helper for file close error logging --- array/array.go | 9 +++---- bytesconv/{bytesconv_1.20.go => bytesconv.go} | 2 -- bytesconv/bytesconv_1.19.go | 24 ------------------- convert/convert.go | 20 ++++++---------- file/file.go | 19 +++++++-------- 5 files changed, 19 insertions(+), 55 deletions(-) rename bytesconv/{bytesconv_1.20.go => bytesconv.go} (96%) delete mode 100644 bytesconv/bytesconv_1.19.go diff --git a/array/array.go b/array/array.go index 83a5468..b51c159 100644 --- a/array/array.go +++ b/array/array.go @@ -1,5 +1,7 @@ package array +import "slices" + // Contains checks if a given key of any comparable type exists within a slice. // It returns true if the key is found, otherwise it returns false. // @@ -14,10 +16,5 @@ package array // Returns: // - bool: True if the key is found in the slice, false otherwise. func Contains[T comparable](slice []T, key T) bool { - for _, item := range slice { - if item == key { - return true - } - } - return false + return slices.Contains(slice, key) } diff --git a/bytesconv/bytesconv_1.20.go b/bytesconv/bytesconv.go similarity index 96% rename from bytesconv/bytesconv_1.20.go rename to bytesconv/bytesconv.go index 1796d2c..c33d77f 100644 --- a/bytesconv/bytesconv_1.20.go +++ b/bytesconv/bytesconv.go @@ -1,5 +1,3 @@ -//go:build go1.20 - package bytesconv import "unsafe" diff --git a/bytesconv/bytesconv_1.19.go b/bytesconv/bytesconv_1.19.go deleted file mode 100644 index 70ca45c..0000000 --- a/bytesconv/bytesconv_1.19.go +++ /dev/null @@ -1,24 +0,0 @@ -//go:build !go1.20 - -package bytesconv - -import "unsafe" - -// BytesToStr converts byte slice to a string without memory allocation. -// See https://groups.google.com/forum/#!msg/Golang-Nuts/ENgbUzYvCuU/90yGx7GUAgAJ . -// -// Note it may break if string and/or slice header will change -// in the future go versions. -func BytesToStr(b []byte) string { - return *(*string)(unsafe.Pointer(&b)) -} - -// StrToBytes converts string to byte slice without a memory allocation. -func StrToBytes(s string) (b []byte) { - return *(*[]byte)(unsafe.Pointer( - &struct { - string - Cap int - }{s, len(s)}, - )) -} diff --git a/convert/convert.go b/convert/convert.go index dded399..34b5d0f 100644 --- a/convert/convert.go +++ b/convert/convert.go @@ -14,7 +14,10 @@ import ( // ToString convert any type to string func ToString(value any) string { - if v, ok := value.(*string); ok { + switch v := value.(type) { + case string: + return v + case *string: return *v } return fmt.Sprintf("%v", value) @@ -36,24 +39,15 @@ func ToBool(value any) bool { case *string: return ToBool(*value) case float64: - if value != 0 { - return true - } - return false + return value != 0 case *float64: return ToBool(*value) case float32: - if value != 0 { - return true - } - return false + return value != 0 case *float32: return ToBool(*value) case int: - if value != 0 { - return true - } - return false + return value != 0 case *int: return ToBool(*value) } diff --git a/file/file.go b/file/file.go index 617948c..c3212e0 100644 --- a/file/file.go +++ b/file/file.go @@ -37,6 +37,13 @@ func Remove(filePath string) error { return os.RemoveAll(filePath) } +// closeFile closes f, logging any close error with the given role label. +func closeFile(f *os.File, role string) { + if cerr := f.Close(); cerr != nil { + fmt.Printf("failed to close %s file: %v\n", role, cerr) + } +} + // Copy files // Copy copies a regular file from src to dst. If dst exists, returns error. // Uses io.Copy for efficient file transfer. @@ -53,11 +60,7 @@ func Copy(src, dst string) error { if err != nil { return err } - defer func() { - if cerr := source.Close(); cerr != nil { - fmt.Printf("failed to close source file: %v\n", cerr) - } - }() + defer closeFile(source, "source") if _, err := os.Stat(dst); err == nil { return fmt.Errorf("file %s already exists", dst) @@ -67,11 +70,7 @@ func Copy(src, dst string) error { if err != nil { return err } - defer func() { - if cerr := destination.Close(); cerr != nil { - fmt.Printf("failed to close destination file: %v\n", cerr) - } - }() + defer closeFile(destination, "destination") if _, err := io.Copy(destination, source); err != nil { return err From 277e4662ec78ba00ca28aded390325f6e3c4d3fb Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 30 May 2026 15:43:57 +0800 Subject: [PATCH 2/4] docs: address Copilot review feedback - Clarify closeFile doc comment to reflect stdout printing - Add ToString test cases for string and string pointer branches - Drop stale Go 1.19 build-tag references from bytesconv README --- bytesconv/README.md | 6 ++---- convert/convert_test.go | 14 ++++++++++++++ file/file.go | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/bytesconv/README.md b/bytesconv/README.md index e3b88f0..4286d79 100644 --- a/bytesconv/README.md +++ b/bytesconv/README.md @@ -5,7 +5,7 @@ High-performance zero-allocation conversion between strings and byte slices for ## Features - Zero memory allocation conversions -- Support for Go 1.19+ and Go 1.20+ with optimized implementations +- Optimized implementation using `unsafe.Slice()` and `unsafe.String()` - Significant performance improvement over standard conversions - Uses unsafe operations for maximum efficiency @@ -101,9 +101,7 @@ Converts a byte slice to a string without memory allocation. ## Implementation Details -- **Go 1.20+**: Uses `unsafe.Slice()` and `unsafe.String()` for optimal performance -- **Go 1.19**: Uses manual unsafe pointer manipulation for compatibility -- **Build Tags**: Automatically selects the appropriate implementation based on Go version +- Uses `unsafe.Slice()` and `unsafe.String()` for optimal, allocation-free performance - **Safety**: While using unsafe operations, the functions are safe when used correctly ## Performance diff --git a/convert/convert_test.go b/convert/convert_test.go index 0a1b06c..9f80838 100644 --- a/convert/convert_test.go +++ b/convert/convert_test.go @@ -14,6 +14,20 @@ func TestToString(t *testing.T) { args args want any }{ + { + name: "string", + args: args{ + value: "hello", + }, + want: "hello", + }, + { + name: "string pointer", + args: args{ + value: ToPtr("hello"), + }, + want: "hello", + }, { name: "int", args: args{ diff --git a/file/file.go b/file/file.go index c3212e0..146297c 100644 --- a/file/file.go +++ b/file/file.go @@ -37,7 +37,7 @@ func Remove(filePath string) error { return os.RemoveAll(filePath) } -// closeFile closes f, logging any close error with the given role label. +// closeFile closes f, printing any close error to stdout with the given role label. func closeFile(f *os.File, role string) { if cerr := f.Close(); cerr != nil { fmt.Printf("failed to close %s file: %v\n", role, cerr) From 21dbb726267dfca058d9d4aaad0b62ae1d2c30d5 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 30 May 2026 15:49:28 +0800 Subject: [PATCH 3/4] fix(file): write close errors to stderr instead of stdout - Redirect closeFile diagnostics to stderr to avoid corrupting piped stdout --- file/file.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/file/file.go b/file/file.go index 146297c..0eea798 100644 --- a/file/file.go +++ b/file/file.go @@ -37,10 +37,10 @@ func Remove(filePath string) error { return os.RemoveAll(filePath) } -// closeFile closes f, printing any close error to stdout with the given role label. +// closeFile closes f, printing any close error to stderr with the given role label. func closeFile(f *os.File, role string) { if cerr := f.Close(); cerr != nil { - fmt.Printf("failed to close %s file: %v\n", role, cerr) + fmt.Fprintf(os.Stderr, "failed to close %s file: %v\n", role, cerr) } } From dbc2e087f91727554550577995c3ef15828962b7 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Sat, 30 May 2026 15:57:35 +0800 Subject: [PATCH 4/4] test(file): add unit tests for Copy - Cover successful copy, existing-destination error, missing-source error - Cover non-regular-file source error --- file/file_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/file/file_test.go b/file/file_test.go index eeaffeb..9515dde 100644 --- a/file/file_test.go +++ b/file/file_test.go @@ -3,6 +3,7 @@ package file import ( "fmt" "os" + "path/filepath" "testing" ) @@ -87,6 +88,43 @@ func TestIsFile(t *testing.T) { } } +func TestCopy(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, "src.txt") + content := []byte("hello world") + if err := os.WriteFile(src, content, 0o644); err != nil { + t.Fatalf("WriteFile() = %v", err) + } + + // successful copy + dst := filepath.Join(dir, "dst.txt") + if err := Copy(src, dst); err != nil { + t.Fatalf("Copy() = %v, want nil", err) + } + got, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("ReadFile() = %v", err) + } + if string(got) != string(content) { + t.Errorf("Copy() content = %q, want %q", got, content) + } + + // error when destination already exists + if err := Copy(src, dst); err == nil { + t.Errorf("Copy() to existing destination = nil, want error") + } + + // error when source does not exist + if err := Copy(filepath.Join(dir, "missing.txt"), filepath.Join(dir, "out.txt")); err == nil { + t.Errorf("Copy() from missing source = nil, want error") + } + + // error when source is a directory (not a regular file) + if err := Copy(dir, filepath.Join(dir, "dir-copy.txt")); err == nil { + t.Errorf("Copy() of a directory = nil, want error") + } +} + // TestFormatSize tests the FormatSize function for various input cases. func TestFormatSize(t *testing.T) { tests := []struct {