From 5d275d05eb28d46a3cd49246336b6c2619a9a7de Mon Sep 17 00:00:00 2001 From: x-stp Date: Fri, 2 May 2025 16:45:36 +0200 Subject: [PATCH 1/2] fix: address linter issues - Fix errcheck errors by properly handling return values of Close() calls - Fix staticcheck issues by replacing WriteString(fmt.Sprintf()) with fmt.Fprintf() - Improve error handling in tests --- pkg/crud_test.go | 12 +++++++++--- pkg/install.go | 24 ++++++++++++++++++------ pkg/utils/utils.go | 14 ++++++++++++-- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/pkg/crud_test.go b/pkg/crud_test.go index b1e414f..a48cf0f 100644 --- a/pkg/crud_test.go +++ b/pkg/crud_test.go @@ -35,7 +35,9 @@ func TestInstall(t *testing.T) { pathBin, err := os.MkdirTemp("", "test-dir") require.Nil(t, err) - defer os.RemoveAll(pathBin) + defer func() { + require.NoError(t, os.RemoveAll(pathBin)) + }() // install first time err = Install(pathBin, tool) @@ -51,7 +53,9 @@ func TestRemove(t *testing.T) { pathBin, err := os.MkdirTemp("", "test-dir") require.Nil(t, err) - defer os.RemoveAll(pathBin) + defer func() { + require.NoError(t, os.RemoveAll(pathBin)) + }() // install the tool err = Install(pathBin, tool) @@ -71,7 +75,9 @@ func TestUpdateSameVersion(t *testing.T) { pathBin, err := os.MkdirTemp("", "test-dir") require.Nil(t, err) - defer os.RemoveAll(pathBin) + defer func() { + require.NoError(t, os.RemoveAll(pathBin)) + }() // install the tool err = Install(pathBin, tool) diff --git a/pkg/install.go b/pkg/install.go index a006cc8..240a556 100644 --- a/pkg/install.go +++ b/pkg/install.go @@ -114,7 +114,11 @@ loop: return "", err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + gologger.Warning().Msgf("Error closing response body: %s", err) + } + }() if resp.StatusCode != 200 { return "", err } @@ -167,7 +171,11 @@ func downloadTar(reader io.Reader, toolName, path string) error { if err != nil { return err } - defer dstFile.Close() + defer func() { + if err := dstFile.Close(); err != nil { + gologger.Warning().Msgf("Error closing file: %s", err) + } + }() // copy the file data from the archive _, err = io.Copy(dstFile, tarReader) if err != nil { @@ -224,8 +232,12 @@ func downloadZip(reader io.Reader, toolName, path string) error { return err } - dstFile.Close() - fileInArchive.Close() + if err := dstFile.Close(); err != nil { + gologger.Warning().Msgf("Error closing file: %s", err) + } + if err := fileInArchive.Close(); err != nil { + gologger.Warning().Msgf("Error closing file in archive: %s", err) + } } return nil } @@ -240,12 +252,12 @@ func printRequirementInfo(tool types.Tool) { continue } if printTitle { - stringBuilder.WriteString(fmt.Sprintf("%s\n", au.Bold(tool.Name+" requirements:").String())) + fmt.Fprintf(stringBuilder, "%s\n", au.Bold(tool.Name+" requirements:").String()) printTitle = false } instruction := getFormattedInstruction(spec) isRequired := getRequirementStatus(spec) - stringBuilder.WriteString(fmt.Sprintf("%s %s\n", isRequired, instruction)) + fmt.Fprintf(stringBuilder, "%s %s\n", isRequired, instruction) } if stringBuilder.Len() > 0 { gologger.Info().Msgf("%s", stringBuilder.String()) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 87f45d0..89cc86a 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -38,7 +38,12 @@ func FetchToolList() ([]types.Tool, error) { if err != nil { return nil, err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + // Just log warning as we're already returning from function + fmt.Printf("Error closing response body: %s\n", err) + } + }() if resp.StatusCode == http.StatusOK { body, err := io.ReadAll(resp.Body) @@ -62,7 +67,12 @@ func fetchTool(toolName string) (types.Tool, error) { if err != nil { return tool, err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + // Just log warning as we're already returning from function + fmt.Printf("Error closing response body: %s\n", err) + } + }() if resp.StatusCode == http.StatusOK { body, err := io.ReadAll(resp.Body) From 140158211cc04b9171c1773065c8e5c04ee1d982 Mon Sep 17 00:00:00 2001 From: x-stp Date: Fri, 2 May 2025 17:02:16 +0200 Subject: [PATCH 2/2] fix: handle remaining os.RemoveAll error checks in tests --- pkg/crud_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/crud_test.go b/pkg/crud_test.go index a48cf0f..b103d57 100644 --- a/pkg/crud_test.go +++ b/pkg/crud_test.go @@ -93,7 +93,9 @@ func TestUpdateNonExistingTool(t *testing.T) { pathBin, err := os.MkdirTemp("", "test-dir") require.Nil(t, err) - defer os.RemoveAll(pathBin) + defer func() { + require.NoError(t, os.RemoveAll(pathBin)) + }() // updating non existing tool should error err = Update(pathBin, tool, true) @@ -105,7 +107,9 @@ func TestUpdateToolWithoutAssets(t *testing.T) { pathBin, err := os.MkdirTemp("", "test-dir") require.Nil(t, err) - defer os.RemoveAll(pathBin) + defer func() { + require.NoError(t, os.RemoveAll(pathBin)) + }() // install the tool err = Install(pathBin, tool)