Merge pull request #756 from yorukot/golangci_lint_fixes

fix: Add some of the remaining linter and fix errors

Author: lazysegtree

.golangci.yaml

@@ -278,12 +278,10 @@ linters:
     - errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
     # Todo enable
     #- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
-    # Todo enable - Need on small fix
-    #- exhaustive # checks exhaustiveness of enum switch statements
+    - exhaustive # checks exhaustiveness of enum switch statements
     - exptostd # detects functions from golang.org/x/exp/ that can be replaced by std functions
     - fatcontext # detects nested contexts in loops
-    # Todo enable - no . this blocks only fmt.Print family and is not useful right now
-    # - forbidigo # forbids identifiers
+    # - forbidigo # forbids identifiers # Not needed. this blocks only fmt.Print family and is not useful right now
     # Todo enable - need to make functions shorter
     #- funlen # tool for detection of long functions
     - gocheckcompilerdirectives # validates go compiler directive comments (//go:)
@@ -293,16 +291,14 @@ linters:
     - gochecksumtype # checks exhaustiveness on Go "sum types"
     # Todo enable - Need to refactor many functions
     # - gocognit # computes and checks the cognitive complexity of functions
-    # Todo enable - Need a few changes - easy
-    # - goconst # finds repeated strings that could be replaced by a constant
+    - goconst # finds repeated strings that could be replaced by a constant
     # Todo enable - Some few changes. There are good suggestions
     # - gocritic # provides diagnostics that check for bugs, performance and style issues
     # Todo enable
     #- gocyclo # computes and checks the cyclomatic complexity of functions
     # Todo enable - too much for us
     #- godot # checks if comments end in a period
-    # Todo enable - one small fix
-    #- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
+    - goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
     - gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
     - goprintffuncname # checks that printf-like functions are named with f at the end
     # Todo enable - ignore for a few int overflow, and fix file permissions
@@ -325,15 +321,12 @@ linters:
     - nilerr # finds the code that returns nil even if it checks that the error is not nil
     - nilnesserr # reports that it checks for err != nil, but it returns a different nil value error (powered by nilness and nilerr)
     - nilnil # checks that there is no simultaneous return of nil error and an invalid value
-    # Todo enable : Need to fix one client.Get usage
-    # - noctx # finds sending http request without context.Context
-    # Todo enable : Needs some fixes
-    # - nolintlint # reports ill-formed or insufficient nolint directives
+    - noctx # finds sending http request without context.Context
+    - nolintlint # reports ill-formed or insufficient nolint directives
     # Todo enable - maybe too strict for us
     # - nonamedreturns # reports all named returns
     - nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL
-    # Todo enable : One small fix
-    # - perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative
+    - perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative
     - predeclared # finds code that shadows one of Go's predeclared identifiers
     - promlinter # checks Prometheus metrics naming via promlint
     - protogetter # reports direct reads from proto message fields when getters should be used
@@ -351,13 +344,11 @@ linters:
     # Todo enable : Many changes, but good changes.
     # - stylecheck # is a replacement for golint
     - testableexamples # checks if examples are testable (have an expected output)
-    # Todo enable - some test bugs. To be fixed
-    # - testifylint # checks usage of github.com/stretchr/testify
+    - testifylint # checks usage of github.com/stretchr/testify
     # Todo enable - Might be too strict for us
     # - testpackage # makes you use a separate _test package
     - tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes
-    # Todo enable - One small change. Good find.
-    # - unconvert # removes unnecessary type conversions
+    - unconvert # removes unnecessary type conversions
     - unparam # reports unused function parameters
     - usestdlibvars # detects the possibility to use variables/constants from the Go standard library
     - usetesting # reports uses of functions with replacement inside the testing package
@@ -403,7 +394,7 @@ linters:
     # - err113 # [too strict] checks the errors handling expressions
     # - errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted
     # - forcetypeassert # [replaced by errcheck] finds forced type assertions
-    # Todo : rely on this, and skip seperate go fmt check in cicd
+    # Todo : rely on this, and skip separate go fmt check in cicd
     # - gofmt # [replaced by goimports] checks whether code was gofmt-ed
     # Todo : Check this
     #- gofumpt # [replaced by goimports, gofumports is not available yet] checks whether code was gofumpt-ed
@@ -412,8 +403,7 @@ linters:
     - grouper # analyzes expression groups
     - importas # enforces consistent import aliases
     - maintidx # measures the maintainability index of each function
-    # Todo - might enable this. This is not so bad
-    # - misspell # [useless] finds commonly misspelled English words in comments
+    - misspell # [useless] finds commonly misspelled English words in comments
     # - nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity
     # Todo : might make tests parallel
     #- paralleltest # [too many false positives] detects missing usage of t.Parallel() method in your Go test

main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"embed"
+
 	"github.com/yorukot/superfile/src/cmd"
 )
 

src/cmd/main.go

@@ -1,11 +1,10 @@
 package cmd
 
 import (
+	"context"
 	"embed"
 	"encoding/json"
 	"fmt"
-	"github.com/yorukot/superfile/src/internal/common"
-	"github.com/yorukot/superfile/src/internal/common/utils"
 	"io"
 	"log"
 	"log/slog"
@@ -14,6 +13,9 @@ import (
 	"runtime"
 	"time"
 
+	"github.com/yorukot/superfile/src/internal/common"
+	"github.com/yorukot/superfile/src/internal/common/utils"
+
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/lipgloss"
 	"github.com/urfave/cli/v2"
@@ -95,7 +97,7 @@ func Run(content embed.FS) {
 			// Validate the config file exists
 			if configFileArg != "" {
 				if _, err := os.Stat(configFileArg); err != nil {
-					log.Fatalf("Error: While reading config file '%s' from arguement : %v", configFileArg, err)
+					log.Fatalf("Error: While reading config file '%s' from argument : %v", configFileArg, err)
 				} else {
 					variable.ConfigFile = configFileArg
 				}
@@ -105,7 +107,7 @@ func Run(content embed.FS) {
 
 			if hotkeyFileArg != "" {
 				if _, err := os.Stat(hotkeyFileArg); err != nil {
-					log.Fatalf("Error: While reading hotkey file '%s' from arguement : %v", hotkeyFileArg, err)
+					log.Fatalf("Error: While reading hotkey file '%s' from argument : %v", hotkeyFileArg, err)
 				} else {
 					variable.HotkeysFile = hotkeyFileArg
 				}
@@ -131,7 +133,9 @@ func Run(content embed.FS) {
 
 			// This must be after calling internal.InitialModel()
 			// so that we know `common.Config` is loaded
-			go CheckForUpdates()
+			// Should not be a goroutine, Otherwise the main
+			// goroutine will exit first, and this will not be able to finish
+			CheckForUpdates()
 
 			if variable.PrintLastDir {
 				fmt.Println(variable.LastDir)
@@ -187,7 +191,7 @@ func InitConfigFile() {
 // We are initializing these, but not sure if we are ever using them
 func InitTrash() error {
 	// Create trash directories
-	if runtime.GOOS != "darwin" {
+	if runtime.GOOS != variable.OS_DARWIN {
 		err := createDirectories(
 			variable.CustomTrashDirectory,
 			variable.CustomTrashDirectoryFiles,
@@ -302,11 +306,22 @@ func CheckForUpdates() {
 		defer func() {
 			writeLastCheckTime(currentTime)
 		}()
-		client := &http.Client{
-			Timeout: 5 * time.Second,
+		// Create a context with a 5-second timeout
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel() // Cancel the context when done
+		client := &http.Client{}
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, variable.LatestVersionURL, nil)
+		if err != nil {
+			slog.Error("Error forming updates request:", "error", err)
+			return
 		}
-		resp, err := client.Get(variable.LatestVersionURL)
+
+		resp, err := client.Do(req)
 		if err != nil {
+			if ctx.Err() == context.DeadlineExceeded {
+				slog.Error("Update check request timed out")
+				return
+			}
 			slog.Error("Error checking for updates:", "error", err)
 			return
 		}

src/config/fixed_variable.go

@@ -17,6 +17,12 @@ const (
 	EmbedHotkeysFile         string = EmbedConfigDir + "/hotkeys.toml"
 	EmbedThemeDir            string = EmbedConfigDir + "/theme"
 	EmbedThemeCatppuccinFile string = EmbedThemeDir + "/catppuccin.toml"
+
+	// These are used while comparing with runtime.GOOS
+	// OS_WINDOWS represents the Windows operating system identifier
+	OS_WINDOWS = "windows"
+	// OS_DARWIN represents the macOS (Darwin) operating system identifier
+	OS_DARWIN = "darwin"
 )
 
 var (

src/internal/common/load_config.go

@@ -180,7 +180,6 @@ func WriteThemeFiles(content embed.FS) error {
 			continue
 		}
 		// This will not break in windows. This is a relative path for Embed FS. It uses "/" only
-		// nolint:govet // Suppress err shadowing
 		src, err := content.ReadFile(variable.EmbedThemeDir + "/" + file.Name())
 		if err != nil {
 			slog.Error("Error reading theme file from embed", "error", err)

src/internal/common/predefined_variable.go

@@ -1,10 +1,11 @@
 package common
 
 import (
+	"time"
+
 	"github.com/charmbracelet/lipgloss"
 	"github.com/charmbracelet/x/exp/term/ansi"
 	"github.com/yorukot/superfile/src/config/icon"
-	"time"
 )
 
 const WheelRunTime = 5

src/internal/common/utils/bool_file_store.go

@@ -0,0 +1,36 @@
+package utils
+
+import (
+	"log/slog"
+	"os"
+	"strconv"
+)
+
+// This file provides utilities for storing boolean values in a file
+
+// Read file with "true" / "false" as content. In case of issues, return defaultValue
+func ReadBoolFile(path string, defaultValue bool) bool {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		slog.Error("Error in readBoolFile", "path", path, "error", err)
+		return defaultValue
+	}
+
+	// Not using strconv.ParseBool() as it allows other values like : "TRUE"
+	// Using exact string comparison with predefined constants ensures
+	// consistent behavior and prevents issues with case-insensitivity or
+	// unexpected values like "yes", "on", etc. that ParseBool would accept
+	switch string(data) {
+	case TRUE_STRING:
+		return true
+	case FALSE_STRING:
+		return false
+	default:
+		return defaultValue
+	}
+}
+
+func WriteBoolFile(path string, value bool) error {
+	return os.WriteFile(path, []byte(strconv.FormatBool(value)), 0644)
+
+}