ci: implement most changes requested by golangci-lint

Author: KyleKing

.golangci.toml

@@ -8,42 +8,62 @@
 
 [linters] # Find the whole list here https://golangci-lint.run/usage/linters/
 default = "standard"
+disabled = [
+  "forbidigo", # forbids identifiers	matched by reg exps # PLANNED: restore when fmt.Print is not necessary
+]
 enable = [
+  "asciicheck", # simple linter to check that your code does not contain non-ASCII identifiers
+  "bodyclose", # checks whether HTTP response body is closed successfully
+  "copyloopvar", # A linter detects places where loop variables are copied
+  "cyclop", # Checks function and package cyclomatic complexity
+  "durationcheck", # check for two durations multiplied together
   "errcheck", # checking for unchecked errors in go programs
   "errorlint", # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
-  "forbidigo", # forbids identifiers	matched by reg exps
+  "fatcontext", # Detects nested contexts in loops and function literals
+  "gocritic", # Provides diagnostics that check for bugs, performance and style issues
+  "goimports", # Goimports does everything that gofmt does. Additionally it checks unused imports
+  "gomoddirectives", # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod
+  "gomodguard", # check for blocked imports from go.mod
+  "gosec", # inspects source code for security problems
   "gosimple", # linter for Go source code that specializes in simplifying a code
+  "govet", # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
+  "iface", # Detect the incorrect use of interfaces, helping developers avoid interface pollution
+  "ineffassign", # detects when assignments to existing variables are not used
+  "intrange", # Intrange is a linter to find places where for loops could make use of an integer range
   "misspell", # finds commonly misspelled English words in comments
   "nakedret", # finds naked returns in functions greater than a specified function length
+  "nilerr", # finds the code that returns nil even if it checks that the error is not nil.
+  "noctx", # noctx finds sending http request without context.Context
   "nolintlint", # reports ill-formed or insufficient nolint directives
+  "perfsprint", # Checks that fmt.Sprintf can be replaced with a faster alternative
+  "prealloc", # Finds slice declarations that could potentially be pre-allocated
+  "predeclared", # Find code that shadows one of Go's predeclared identifiers
+  "revive", # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint
+  "sqlclosecheck", # Checks that sql.Rows, sql.Stmt, sqlx.NamedStmt, pgx.Query are closed
   "staticcheck", # Staticcheck is a go vet on steroids, applying a ton of static analysis checks
   "stylecheck", # a replacement for golint
-  "unused", # checks Go code for unused constants, variables, functions and types
-  "govet", # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
-  "ineffassign", # detects when assignments to existing variables are not used
+  "testifylint", # Checks usage of github.com/stretchr/testify
+  "testpackage", # makes you use a separate _test package
   "typecheck", # Like the front-end of a Go compiler, parses and type-checks Go code
-  "asciicheck", # simple linter to check that your code does not contain non-ASCII identifiers
-  "bodyclose", # checks whether HTTP response body is closed successfully
-  "durationcheck", # check for two durations multiplied together
-  "goimports", # Goimports does everything that gofmt does. Additionally it checks unused imports
-  "gosec", # inspects source code for security problems
-  "importas", # enforces consistent import aliases
-  "nilerr", # finds the code that returns nil even if it checks that the error is not nil.
-  "noctx", # noctx finds sending http request without context.Context
   "unconvert", # Remove unnecessary type conversions
+  "unused", # checks Go code for unused constants, variables, functions and types
   "wastedassign", # wastedassign finds wasted assignment statements.
-  "depguard", # check for blocked imports from Go files
-  "gomodguard", # check for blocked imports from go.mod
-  "gomoddirectives", # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod
+  "wrapcheck", # Checks that errors returned from external packages are wrapped.
 ]
 
 [linters.settings] # See all configuration here: https://golangci-lint.run/usage/linters
 
+[linters.settings.revive]
+enable-all-rules = true
+
 [linters.settings.rowserrcheck]
 packages = ["github.com/jmoiron/sqlx"]
 
 [linters.settings.staticcheck]
 checks = ["all"]
 
+[linters.settings.testifylint]
+enable-all = true
+
 [run]
 timeout = "1m" # Default is 1 min

cmd/subcommands/list.go

@@ -65,18 +65,18 @@ type FileSummary struct {
 type OutputFormat func([]FileSummary) string
 
 func summarize(summaries []FileSummary) string {
-	mod_time_col := "Modified"
+	modTimeCol := "Modified"
 
 	t := table.NewWriter()
-	t.AppendHeader(table.Row{"subDir", "File Name", mod_time_col, "Header"})
+	t.AppendHeader(table.Row{"subDir", "File Name", modTimeCol, "Header"})
 	for _, summary := range summaries {
 		stat := summary.stat
 		t.AppendRow([]interface{}{
 			stat.subDir, stat.file.Name(), stat.fileInfo.ModTime(), summary.header,
 		})
 	}
 	t.SetColumnConfigs([]table.ColumnConfig{{
-		Name:        mod_time_col,
+		Name:        modTimeCol,
 		Transformer: text.NewTimeTransformer(time.RFC822, nil), // "02 Jan 06 15:04 MST"
 	}})
 	return t.Render()
@@ -125,7 +125,7 @@ func calculateStats(syncDir, subDir string) (stats []FileStat, err error) {
 		if !file.IsDir() && strings.HasSuffix(file.Name(), ".dj") {
 			fi, err := file.Info()
 			if err != nil {
-				return stats, fmt.Errorf("Error with specified file (`%v`): %w", file, err)
+				return stats, fmt.Errorf("error with specified file (`%v`): %w", file, err)
 			}
 			stat := FileStat{file: file, fileInfo: fi, subDir: subDir, path: filepath.Join(dir, file.Name())}
 			stats = append(stats, stat)

cmd/subcommands/new.go

@@ -1,6 +1,7 @@
 package subcommands
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -21,11 +22,20 @@ func toTimeName(t time.Time) string {
 }
 
 func fromTimeName(name string) (time.Time, error) {
-	return time.Parse(time.RFC3339, strings.Replace(name, "_", ":", 2))
+	parsedName := strings.Replace(name, "_", ":", 2)
+	time, err := time.Parse(time.RFC3339, parsedName)
+	if err != nil {
+		return time, fmt.Errorf("failed to parse time %s (%s): %w", name, parsedName, err)
+	}
+	return time, nil
+
 }
 
 func createFile(path string) (err error) {
 	file, err := os.Create(path)
+	if err != nil {
+		return fmt.Errorf("failed to create file: %w", err)
+	}
 	defer file.Close()
 	return
 }
@@ -51,20 +61,20 @@ func AttachNew(cli *clir.Cli) {
 			return fmt.Errorf("'%s' is not one of %v subDirs in '%s'. Create the folder if intended", subDir, folderNames, syncDir)
 		}
 
-		name := fmt.Sprintf("%s.dj", toTimeName(time.Now()))
+		name := toTimeName(time.Now()) + ".dj"
 		path := filepath.Join(syncDir, subDir, name)
 		if err := createFile(path); err != nil {
 			return err
 		}
 		if open {
 			visual := os.Getenv("VISUAL")
 			if visual == "" {
-				return fmt.Errorf("No value is set for Visual")
+				return errors.New("no value is set for shell environment 'VISUAL'")
 			}
 			cmd := exec.Command("open", "-a", visual, path)
 			out, err := cmd.Output()
 			if err != nil {
-				return fmt.Errorf("Failed to run '%v' with error '%w' and output '%v'", cmd, err, out)
+				return fmt.Errorf("failed to run '%v' with error '%w' and output '%v'", cmd, err, out)
 			}
 		}
 		fmt.Println(path)

cmd/subcommands/rename.go

@@ -13,7 +13,7 @@ import (
 func readCreationTime(path string) (string, error) {
 	t, err := times.Stat(path)
 	if err != nil {
-		return "", fmt.Errorf("Error with specified file (`%s`): %w", path, err)
+		return "", fmt.Errorf("error with specified file (`%s`): %w", path, err)
 	}
 	return toTimeName(t.BirthTime()), nil
 }
@@ -33,7 +33,10 @@ func renameAction(flags *RenameFlags) (err error) {
 	if err != nil {
 		return
 	}
-	renameFile(flags.Path, cTime)
+	err = renameFile(flags.Path, cTime)
+	if err != nil {
+		return fmt.Errorf("failed to rename file: %w", err)
+	}
 	fmt.Printf("Renamed %s with time %v\n", flags.Path, cTime)
 	return
 }

cmd/subcommands/search.go

@@ -10,7 +10,7 @@ import (
 	"time"
 
 	"github.com/jmoiron/sqlx"
-	_ "github.com/marcboeker/go-duckdb"
+	_ "github.com/marcboeker/go-duckdb" // Configure DuckDB driver
 
 	"github.com/KyleKing/yak-shears/cmd/config"
 	"github.com/leaanthony/clir"
@@ -20,17 +20,16 @@ import (
 //  https://gobyexample.com/http-client
 //  https://www.digitalocean.com/community/tutorials/how-to-make-http-requests-in-go
 
-//go:embed sql/init.sql
-var SQL_INIT string
-
-//go:embed sql/insertNote.sql
-var INSERT_NOTE string
-
-//go:embed sql/insertEmbedding.sql
-var INSERT_EMBEDDING string
-
-//go:embed sql/searchQuery.sql
-var SEARCH_QUERY string
+var (
+	//go:embed sql/init.sql
+	sqlInitStmt string
+	//go:embed sql/insertNote.sql
+	insertNoteStmt string
+	//go:embed sql/insertEmbedding.sql
+	insertEmbeddingStmt string
+	//go:embed sql/searchQuery.sql
+	searchQueryStmt string
+)
 
 type Note struct {
 	SubDir     string    `db:"sub_dir"`
@@ -43,19 +42,19 @@ type Note struct {
 func storeNotes(db *sqlx.DB, notes []Note) (err error) {
 	// PLANNED: submit multiple notes in single statement
 	for _, note := range notes {
-		_, err := db.NamedExec(INSERT_NOTE, note)
+		_, err := db.NamedExec(insertNoteStmt, note)
 		if err != nil {
-			return fmt.Errorf("failed to execute INSERT_NOTE for note %s: %w", note.Filename, err)
+			return fmt.Errorf("failed to execute insertNote for note %s: %w", note.Filename, err)
 		}
 
 		// TODO: Consider alternative chunking techniques
 		for _, chunk := range strings.Split(note.Content, `\n`) {
-			_, err := db.NamedExec(INSERT_EMBEDDING, map[string]interface{}{
+			_, err := db.NamedExec(insertEmbeddingStmt, map[string]interface{}{
 				"filename":  note.Filename,
 				"embedding": chunk,
 			})
 			if err != nil {
-				return fmt.Errorf("failed to execute INSERT_EMBEDDING for chunk in note %s: %w", note.Filename, err)
+				return fmt.Errorf("failed to execute insertEmbeddingStmt for chunk in note %s: %w", note.Filename, err)
 			}
 		}
 	}
@@ -91,8 +90,21 @@ func ingestSubdir(db *sqlx.DB, syncDir, subDir string) (err error) {
 	return nil
 }
 
-// Ingest ALL Notes (HACK: Should be replaced by incremental ingestion)
+// Purge data
+func dropDataHack(db *sqlx.DB) (err error) {
+	_, err = db.Exec("DROP TABLE IF EXISTS note CASCADE")
+	if err != nil {
+		return fmt.Errorf("failed to remove tables: %w", err)
+	}
+	return nil
+}
+
+// Ingest ALL Notes
 func ingestAllNotes(db *sqlx.DB, syncDir string) (err error) {
+	if err = dropDataHack(db); err != nil {
+		return err
+	}
+
 	folderNames, err := ListsubDirs(syncDir)
 	if err != nil {
 		return fmt.Errorf("failed to list subdirectories in %s: %w", syncDir, err)
@@ -106,23 +118,15 @@ func ingestAllNotes(db *sqlx.DB, syncDir string) (err error) {
 	return nil
 }
 
-// HACK: Remove ALL notes (only for testing)
-func removeAllNotes(db *sqlx.DB) {
-	// PLANNED: CASCADE from table note didn't work
-	_, err := db.Exec("DELETE FROM embedding")
-	check(err)
-	_, err = db.Exec("DELETE FROM note")
-	check(err)
-}
-
-// Search for note in database
+// Search for note in database (TODO: currently only a PoC without WHERE/'query')
 func search(db *sqlx.DB, query string) (err error) {
+	fmt.Printf("Warning: does not yet use query='%s'", query)
 	notes := []Note{}
-	nstmt, err := db.PrepareNamed(SEARCH_QUERY)
-	// TODO: currently only a PoC without WHERE/'query'
+	nstmt, err := db.PrepareNamed(searchQueryStmt)
 	if err != nil {
 		return fmt.Errorf("failed to prepare search query: %w", err)
 	}
+	defer nstmt.Close()
 	err = nstmt.Select(&notes, map[string]interface{}{
 		"limit_":  2,
 		"offset_": 0,
@@ -140,7 +144,7 @@ func search(db *sqlx.DB, query string) (err error) {
 }
 
 // Connect to the database and non-destructively initialize the schema, if not already found
-func connectDb(dir string) (db *sqlx.DB, err error) {
+func connectDB(dir string) (db *sqlx.DB, err error) {
 	path := filepath.Join(dir, "yak-shears.db?access_mode=READ_WRITE")
 	db, err = sqlx.Open("duckdb", path)
 	if err != nil {
@@ -149,11 +153,10 @@ func connectDb(dir string) (db *sqlx.DB, err error) {
 
 	// PLANNED: use a connection for threading
 	// conn, err := db.Conn(context.Background())
-	// check(err)
 	// defer conn.Close()
 	// return conn, nil
 
-	_, err = db.Exec(SQL_INIT)
+	_, err = db.Exec(sqlInitStmt)
 	if err != nil {
 		return nil, fmt.Errorf("failed to initialize database schema: %w", err)
 	}
@@ -181,13 +184,13 @@ func AttachSearch(cli *clir.Cli) {
 			return err
 		}
 
-		db, err := connectDb(syncDir)
+		db, err := connectDB(syncDir)
 		if err != nil {
 			return err
 		}
 		defer db.Close()
-		// // HACK: re-ingestion on search is only for testing
-		// removeAllNotes(db)
+
+		// HACK: replace with conditional and incremental ingestion
 		if err := ingestAllNotes(db, syncDir); err != nil {
 			return err
 		}

cmd/subcommands/search_test.go

@@ -13,7 +13,7 @@ func TestAttachSearch(t *testing.T) {
 	tmpTestSubDir := resetTmpTestDir(t, "search")
 
 	// PLANNED: Consider testing against a seeded database
-	// db, err := connectDb(filepath.Dir(tmpTestSubDir))
+	// db, err := connectDB(filepath.Dir(tmpTestSubDir))
 	// require.NoError(t, err)
 	// defer db.Close()
 	//

cmd/subcommands/testUtils.go

@@ -34,7 +34,7 @@ func resetTmpTestDir(t *testing.T, subDir string) string {
 // Look for exactly one file that matches the prefix
 func matchCreatedFile(testDir string, prefix string, t *testing.T) {
 	matchedPaths := []string{}
-	validateFiles := func(path string, fileInfo os.FileInfo, inpErr error) error {
+	validateFiles := func(path string, _ os.FileInfo, _ error) error {
 		stat, err := os.Stat(path)
 		if err != nil {
 			return err
@@ -43,7 +43,7 @@ func matchCreatedFile(testDir string, prefix string, t *testing.T) {
 		if strings.HasPrefix(filepath.Base(path), prefix) {
 			matchedPaths = append(matchedPaths, path)
 		} else if stat.Mode().IsRegular() {
-			return fmt.Errorf("At least one unrecognized file when attempting to match (%s): %s (matchedPaths=%v)", prefix, path, matchedPaths)
+			return fmt.Errorf("at least one unrecognized file when attempting to match (%s): %s (matchedPaths=%v)", prefix, path, matchedPaths)
 		}
 
 		return nil

cmd/subcommands/utils.go

@@ -11,7 +11,7 @@ func main() {
 	cli := cmd.InitCli()
 
 	if err := cli.Run(); err != nil {
-		fmt.Println("Error encountered: ", err)
+		fmt.Fprintf(os.Stderr, "Error encountered: %v\n", err)
 		os.Exit(1)
 	}
 }

main.go

@@ -7,7 +7,7 @@ description = "Use in GitHub Actions"
 
 [tasks.format]
 description = "Format code"
-run = ["hk fix --quiet", "testifylint --enable-all --fix ./...", "golangci-lint run --fix ./..."]
+run = ["golangci-lint run --fix ./...", "hk fix --quiet"]
 sources = ['**/*.go']
 
 [tasks.install-system]

mise.toml

@@ -33,7 +33,7 @@ param_style = "colon"
 
 [tool.tomlsort]
 in_place = true
+sort_inline_arrays = true
 sort_inline_tables = true
 sort_table_keys = true
-# sort_inline_arrays = true
 trailing_comma_inline_array = true