From 027390dca5fe0f6dc477b0b327232afd3e665f8c Mon Sep 17 00:00:00 2001 From: Brian McGee Date: Tue, 15 Oct 2024 10:16:48 +0100 Subject: [PATCH] feat: more stringent golangci-lint rules Applies more stringent `golangci-lint` rules to the repository, borrowed from https://github.com/nix-community/go-nix. Also fixes a bug where we were not returning errors when calling `walk.Reader.Read()` in `cmd/format` Signed-off-by: Brian McGee Co-authored-by: Jeremy Fleischman Signed-off-by: Brian McGee --- .golangci.yml | 39 ++++++++++++ build/build.go | 4 +- cmd/format/format.go | 30 ++++----- cmd/init/init.go | 4 +- cmd/root.go | 17 +++-- cmd/root_test.go | 135 ++++++++++++++++++++-------------------- config/config.go | 28 +++++---- config/config_test.go | 41 ++++++++---- format/cache.go | 9 +-- format/formatter.go | 18 +++--- format/glob.go | 1 + format/glob_test.go | 3 +- format/task.go | 2 + stats/stats.go | 21 +++---- test/temp.go | 20 +++--- walk/cache/bucket.go | 18 +++++- walk/cache/cache.go | 13 ++-- walk/cached.go | 13 ++-- walk/cached_test.go | 22 +++---- walk/filesystem.go | 4 +- walk/filesystem_test.go | 13 ++-- walk/git.go | 6 +- walk/git_test.go | 8 +-- walk/stdin.go | 2 +- walk/walk.go | 5 ++ 25 files changed, 285 insertions(+), 191 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..c537725a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,39 @@ +# taken from https://github.com/nix-community/go-nix/blob/main/.golangci.yml +linters: + enable: + - errname + - exhaustive + - gci + - gochecknoglobals + - gochecknoinits + - goconst + - godot + - gofumpt + - goheader + - goimports + - gosec + - importas + - ireturn + - lll + - makezero + - misspell + - nakedret + - nestif + - nilerr + - nilnil + - nlreturn + - noctx + - nolintlint + - prealloc + - predeclared + - revive + - rowserrcheck + - stylecheck + - tagliatelle + - tenv + - testpackage + - unconvert + - unparam + - wastedassign + - whitespace + - wsl diff --git a/build/build.go b/build/build.go index bd1c65ed..96fe7281 100644 --- a/build/build.go +++ b/build/build.go @@ -1,6 +1,6 @@ package build var ( - Name = "treefmt" - Version = "v0.0.1+dev" + Name = "treefmt" //nolint:gochecknoglobals + Version = "v0.0.1+dev" //nolint:gochecknoglobals ) diff --git a/cmd/format/format.go b/cmd/format/format.go index 46eb6eed..4e877aee 100644 --- a/cmd/format/format.go +++ b/cmd/format/format.go @@ -14,19 +14,17 @@ import ( "syscall" "time" - "github.com/numtide/treefmt/walk/cache" - bolt "go.etcd.io/bbolt" - "github.com/charmbracelet/log" "github.com/gobwas/glob" "github.com/numtide/treefmt/config" "github.com/numtide/treefmt/format" "github.com/numtide/treefmt/stats" "github.com/numtide/treefmt/walk" + "github.com/numtide/treefmt/walk/cache" "github.com/spf13/cobra" "github.com/spf13/viper" + bolt "go.etcd.io/bbolt" "golang.org/x/sync/errgroup" - "mvdan.cc/sh/v3/expand" ) @@ -60,20 +58,23 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) // Wait until we tick over into the next second before processing to ensure our EPOCH level modtime comparisons // for change detection are accurate. // This can fail in CI between checkout and running treefmt if everything happens too quickly. - // For humans, the second level precision should not be a problem as they are unlikely to run treefmt in sub-second succession. + // For humans, the second level precision should not be a problem as they are unlikely to run treefmt in + // sub-second succession. <-time.After(time.Until(startAfter)) } // cpu profiling - if cfg.CpuProfile != "" { - cpuProfile, err := os.Create(cfg.CpuProfile) + if cfg.CPUProfile != "" { + cpuProfile, err := os.Create(cfg.CPUProfile) if err != nil { return fmt.Errorf("failed to open file for writing cpu profile: %w", err) } else if err = pprof.StartCPUProfile(cpuProfile); err != nil { return fmt.Errorf("failed to start cpu profile: %w", err) } + defer func() { pprof.StopCPUProfile() + if err := cpuProfile.Close(); err != nil { log.Errorf("failed to close cpu profile: %v", err) } @@ -99,6 +100,7 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) if errors.Is(err, format.ErrCommandNotFound) && cfg.AllowMissingFormatter { log.Debugf("formatter command not found: %v", name) + continue } else if err != nil { return fmt.Errorf("%w: failed to initialise formatter: %v", err, name) @@ -110,8 +112,8 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) var db *bolt.DB + // open the db unless --no-cache was specified if !cfg.NoCache { - // open the db db, err = cache.Open(cfg.TreeRoot) if err != nil { return fmt.Errorf("failed to open cache: %w", err) @@ -123,7 +125,9 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) log.Errorf("failed to close cache: %v", err) } }() + } + if db != nil { // clear the cache if desired if cfg.ClearCache { if err = cache.Clear(db); err != nil { @@ -226,9 +230,7 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) break } else if err != nil { // something went wrong - log.Errorf("failed to read files: %v", err) - cancel() - break + return fmt.Errorf("failed to read files: %w", err) } } @@ -243,7 +245,6 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) return reader.Close() } -// applyFormatters func applyFormatters( ctx context.Context, cfg *config.Config, @@ -275,7 +276,6 @@ func applyFormatters( // process the batch if it's full, or we've been asked to flush partial batches if flush || len(batch) == BatchSize { - // copy the batch as we re-use it for the next batch tasks := make([]*format.Task, len(batch)) copy(tasks, batch) @@ -287,6 +287,7 @@ func applyFormatters( formatters := tasks[0].Formatters var formatErrors []error + for idx := range formatters { if err := formatters[idx].Apply(ctx, tasks); err != nil { formatErrors = append(formatErrors, err) @@ -330,7 +331,6 @@ func applyFormatters( // iterate the file channel for file := range filesCh { - // a list of formatters that match this file var matches []*format.Formatter @@ -392,6 +392,7 @@ func applyFormatters( if err := fg.Wait(); err != nil { return fmt.Errorf("formatting failure: %w", err) } + return nil } } @@ -406,7 +407,6 @@ func postProcessing( LOOP: for { select { - // detect ctx cancellation case <-ctx.Done(): return ctx.Err() diff --git a/cmd/init/init.go b/cmd/init/init.go index 91c5dc11..4b780317 100644 --- a/cmd/init/init.go +++ b/cmd/init/init.go @@ -12,9 +12,11 @@ import ( var initBytes []byte func Run() error { - if err := os.WriteFile("treefmt.toml", initBytes, 0o644); err != nil { + if err := os.WriteFile("treefmt.toml", initBytes, 0o600); err != nil { return fmt.Errorf("failed to write treefmt.toml: %w", err) } + fmt.Printf("Generated treefmt.toml. Now it's your turn to edit it.\n") + return nil } diff --git a/cmd/root.go b/cmd/root.go index 89ae12eb..194c2701 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -5,13 +5,12 @@ import ( "os" "path/filepath" - "github.com/numtide/treefmt/stats" - "github.com/charmbracelet/log" "github.com/numtide/treefmt/build" "github.com/numtide/treefmt/cmd/format" _init "github.com/numtide/treefmt/cmd/init" "github.com/numtide/treefmt/config" + "github.com/numtide/treefmt/stats" "github.com/spf13/cobra" "github.com/spf13/viper" ) @@ -33,7 +32,7 @@ func NewRoot() (*cobra.Command, *stats.Stats) { // create out root command cmd := &cobra.Command{ - Use: "treefmt ", + Use: fmt.Sprintf("%s ", build.Name), Short: "One CLI to format your repo", Version: build.Version, RunE: func(cmd *cobra.Command, args []string) error { @@ -55,8 +54,15 @@ func NewRoot() (*cobra.Command, *stats.Stats) { cmd.HelpTemplate() // add a couple of special flags which don't have a corresponding entry in treefmt.toml - fs.StringVar(&configFile, "config-file", "", "Load the config file from the given path (defaults to searching upwards for treefmt.toml or .treefmt.toml).") - fs.BoolVarP(&treefmtInit, "init", "i", false, "Create a treefmt.toml file in the current directory.") + fs.StringVar( + &configFile, "config-file", "", + "Load the config file from the given path (defaults to searching upwards for treefmt.toml or "+ + ".treefmt.toml).", + ) + fs.BoolVarP( + &treefmtInit, "init", "i", false, + "Create a treefmt.toml file in the current directory.", + ) // bind our command's flags to viper if err := v.BindPFlags(fs); err != nil { @@ -110,6 +116,7 @@ func runE(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, args []string) // read in the config v.SetConfigFile(configFile) + if err := v.ReadInConfig(); err != nil { cobra.CheckErr(fmt.Errorf("failed to read config file '%s': %w", configFile, err)) } diff --git a/cmd/root_test.go b/cmd/root_test.go index 102511f1..c860cce2 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -12,21 +12,14 @@ import ( "testing" "time" - "github.com/numtide/treefmt/walk" - - "github.com/numtide/treefmt/cmd" - - "github.com/numtide/treefmt/config" - "github.com/charmbracelet/log" - "github.com/numtide/treefmt/stats" - + "github.com/numtide/treefmt/cmd" format2 "github.com/numtide/treefmt/cmd/format" - + "github.com/numtide/treefmt/config" "github.com/numtide/treefmt/format" - + "github.com/numtide/treefmt/stats" "github.com/numtide/treefmt/test" - + "github.com/numtide/treefmt/walk" "github.com/stretchr/testify/require" ) @@ -177,10 +170,11 @@ func TestSpecifyingFormatters(t *testing.T) { } setup() + _, statz, err := treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 3, @@ -192,7 +186,7 @@ func TestSpecifyingFormatters(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "elm,nix") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 2, stats.Formatted: 2, @@ -204,7 +198,7 @@ func TestSpecifyingFormatters(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "-f", "ruby,nix") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 2, stats.Formatted: 2, @@ -216,7 +210,7 @@ func TestSpecifyingFormatters(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--formatters", "nix") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 1, stats.Formatted: 1, @@ -254,7 +248,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err := treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -268,7 +262,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 31, stats.Formatted: 31, @@ -282,7 +276,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 25, stats.Formatted: 25, @@ -298,7 +292,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 23, stats.Formatted: 23, @@ -312,7 +306,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 22, stats.Formatted: 22, @@ -328,7 +322,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 1, stats.Formatted: 1, @@ -342,7 +336,7 @@ func TestIncludesAndExcludes(t *testing.T) { _, statz, err = treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 2, stats.Formatted: 2, @@ -371,7 +365,7 @@ func TestPrjRootEnvVariable(t *testing.T) { _, statz, err := treefmt(t, "--config-file", configPath) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -401,7 +395,7 @@ func TestCache(t *testing.T) { _, statz, err := treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -411,7 +405,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 0, @@ -422,7 +416,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "-c") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -432,7 +426,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 0, @@ -443,7 +437,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "-c") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -453,7 +447,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 0, @@ -464,7 +458,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "--no-cache") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -490,7 +484,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 6, stats.Formatted: 0, @@ -501,7 +495,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "-vv") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 6, stats.Formatted: 0, @@ -516,7 +510,7 @@ func TestCache(t *testing.T) { _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 6, stats.Formatted: 6, @@ -556,7 +550,7 @@ func TestChangeWorkingDirectory(t *testing.T) { _, statz, err := treefmt(t, "-C", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -568,7 +562,7 @@ func TestChangeWorkingDirectory(t *testing.T) { _, statz, err = treefmt(t, "-c") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -622,7 +616,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { } // prepend our test bin directory to PATH - as.NoError(os.Setenv("PATH", binPath+":"+os.Getenv("PATH"))) + t.Setenv("PATH", binPath+":"+os.Getenv("PATH")) // start with 2 formatters cfg := &config.Config{ @@ -644,7 +638,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err := treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 3, @@ -657,7 +651,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 3, @@ -668,7 +662,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 0, @@ -681,7 +675,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 3, @@ -692,7 +686,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 3, stats.Formatted: 0, @@ -710,7 +704,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 4, stats.Formatted: 4, @@ -721,7 +715,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 4, stats.Formatted: 0, @@ -735,7 +729,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 2, stats.Formatted: 2, @@ -746,7 +740,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 2, stats.Formatted: 0, @@ -760,7 +754,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 1, stats.Formatted: 1, @@ -771,7 +765,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) { _, statz, err = treefmt(t, args...) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 1, stats.Formatted: 0, @@ -797,11 +791,11 @@ func TestGit(t *testing.T) { test.WriteConfig(t, configPath, cfg) - run := func(traversed int32, matched int32, formatted int32, changed int32) { + run := func(traversed int, matched int, formatted int, changed int) { _, statz, err := treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: traversed, stats.Matched: matched, stats.Formatted: formatted, @@ -842,7 +836,7 @@ func TestGit(t *testing.T) { _, statz, err := treefmt(t, "-c", "--config-file", configPath, "--tree-root", tempDir, "--walk", "filesystem") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 80, stats.Matched: 80, stats.Formatted: 80, @@ -862,7 +856,7 @@ func TestGit(t *testing.T) { _, statz, err = treefmt(t, "-C", tempDir, "-c", "go", "-vv") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 2, stats.Matched: 2, stats.Changed: 0, @@ -871,7 +865,7 @@ func TestGit(t *testing.T) { _, statz, err = treefmt(t, "-C", tempDir, "-c", "go", "haskell") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 9, stats.Matched: 9, stats.Changed: 0, @@ -880,7 +874,7 @@ func TestGit(t *testing.T) { _, statz, err = treefmt(t, "-C", tempDir, "-c", "go", "haskell", "ruby") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 10, stats.Matched: 10, stats.Changed: 0, @@ -897,7 +891,7 @@ func TestGit(t *testing.T) { _, statz, err = treefmt(t, "-C", tempDir, "-c", "haskell", "foo.txt") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 8, stats.Matched: 8, stats.Changed: 0, @@ -906,7 +900,7 @@ func TestGit(t *testing.T) { _, statz, err = treefmt(t, "-C", tempDir, "-c", "foo.txt") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 1, stats.Matched: 1, stats.Changed: 0, @@ -956,7 +950,7 @@ func TestPathsArg(t *testing.T) { _, statz, err := treefmt(t) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -967,7 +961,7 @@ func TestPathsArg(t *testing.T) { _, statz, err = treefmt(t, "-c", "elm/elm.json", "haskell/Nested/Foo.hs") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 2, stats.Matched: 2, stats.Formatted: 2, @@ -981,7 +975,7 @@ func TestPathsArg(t *testing.T) { _, statz, err = treefmt(t, "-c", absoluteInternalPath) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 1, stats.Matched: 1, stats.Formatted: 1, @@ -1021,6 +1015,7 @@ func TestStdin(t *testing.T) { // capture current stdin and replace it on test cleanup prevStdIn := os.Stdin + t.Cleanup(func() { os.Stdin = prevStdIn }) @@ -1028,19 +1023,19 @@ func TestStdin(t *testing.T) { // omit the required filename parameter contents := `{ foo, ... }: "hello"` os.Stdin = test.TempFile(t, "", "stdin", &contents) + // we get an error about the missing filename parameter. out, _, err := treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin") as.EqualError(err, "exactly one path should be specified when using the --stdin flag") as.Equal("", string(out)) // now pass along the filename parameter - contents = `{ foo, ... }: "hello"` os.Stdin = test.TempFile(t, "", "stdin", &contents) out, statz, err := treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", "test.nix") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 1, stats.Matched: 1, stats.Formatted: 1, @@ -1052,7 +1047,6 @@ func TestStdin(t *testing.T) { `, string(out)) // try a file that's outside of the project root - contents = `{ foo, ... }: "hello"` os.Stdin = test.TempFile(t, "", "stdin", &contents) out, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", "../test.nix") @@ -1071,7 +1065,7 @@ func TestStdin(t *testing.T) { out, statz, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", "test.md") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 1, stats.Matched: 1, stats.Formatted: 1, @@ -1123,6 +1117,7 @@ func TestDeterministicOrderingInPipeline(t *testing.T) { }, }, }) + _, _, err = treefmt(t, "-C", tempDir) as.NoError(err) @@ -1135,18 +1130,21 @@ func TestDeterministicOrderingInPipeline(t *testing.T) { for _, p := range paths { file, err := os.Open(filepath.Join(tempDir, p)) as.NoError(err) - scanner := bufio.NewScanner(file) + scanner := bufio.NewScanner(file) idx := 0 for scanner.Scan() { line := scanner.Text() + matches := matcher.FindAllString(line, -1) if len(matches) != 1 { continue } + as.Equal(sequence[idx], matches[0]) - idx += 1 + + idx++ } } } @@ -1188,7 +1186,9 @@ func TestRunInSubdir(t *testing.T) { // test that formatters are resolved relative to the treefmt root echoPath, err := exec.LookPath("echo") as.NoError(err) + echoRel := path.Join(tempDir, "echo") + err = os.Symlink(echoPath, echoRel) as.NoError(err) @@ -1210,7 +1210,7 @@ func TestRunInSubdir(t *testing.T) { _, statz, err := treefmt(t) as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, stats.Matched: 32, stats.Formatted: 32, @@ -1226,7 +1226,7 @@ func TestRunInSubdir(t *testing.T) { _, statz, err = treefmt(t, "-c", "elm.json", "../haskell/Nested/Foo.hs") as.NoError(err) - assertStats(t, as, statz, map[stats.Type]int32{ + assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 2, stats.Matched: 2, stats.Formatted: 2, @@ -1266,6 +1266,7 @@ func treefmt(t *testing.T, args ...string) ([]byte, *stats.Stats, error) { // we must pass an empty array otherwise cobra with use os.Args[1:] args = []string{} } + root.SetArgs(args) root.SetOut(tempOut) root.SetErr(tempOut) @@ -1301,7 +1302,7 @@ func assertStats( t *testing.T, as *require.Assertions, statz *stats.Stats, - expected map[stats.Type]int32, + expected map[stats.Type]int, ) { t.Helper() diff --git a/config/config.go b/config/config.go index 7d723ca5..906e9030 100644 --- a/config/config.go +++ b/config/config.go @@ -7,26 +7,16 @@ import ( "strings" "github.com/numtide/treefmt/walk" - "github.com/spf13/pflag" "github.com/spf13/viper" ) -// configReset is used to null out attempts to set certain values in the config file -var configReset = map[string]any{ - "ci": false, - "clear-cache": false, - "no-cache": false, - "stdin": false, - "working-dir": ".", -} - // Config is used to represent the list of configured Formatters. type Config struct { AllowMissingFormatter bool `mapstructure:"allow-missing-formatter" toml:"allow-missing-formatter,omitempty"` CI bool `mapstructure:"ci" toml:"ci,omitempty"` ClearCache bool `mapstructure:"clear-cache" toml:"-"` // not allowed in config - CpuProfile string `mapstructure:"cpu-profile" toml:"cpu-profile,omitempty"` + CPUProfile string `mapstructure:"cpu-profile" toml:"cpu-profile,omitempty"` Excludes []string `mapstructure:"excludes" toml:"excludes,omitempty"` FailOnChange bool `mapstructure:"fail-on-change" toml:"fail-on-change,omitempty"` Formatters []string `mapstructure:"formatters" toml:"formatters,omitempty"` @@ -63,7 +53,7 @@ type Formatter struct { // SetFlags appends our flags to the provided flag set. // We have a flag matching most entries in Config, taking care to ensure the name matches the field name defined in the // mapstructure tag. -// We can rely on a flag's default value being provided in the event the same value was not specified in the config file. +// We rely on a flag's default value being provided in the event the same value was not specified in the config file. func SetFlags(fs *pflag.FlagSet) { fs.Bool( "allow-missing-formatter", false, @@ -136,7 +126,7 @@ func SetFlags(fs *pflag.FlagSet) { // * TOML config type // * automatic env enabled // * `TREEFMT_` env prefix for environment variables -// * replacement of `-` and `.` with `_` when mapping from flags to env e.g. `global.excludes` => `TREEFMT_GLOBAL_EXCLUDES` +// * replacement of `-` and `.` with `_` when mapping flags to env e.g. `global.excludes` => `TREEFMT_GLOBAL_EXCLUDES`. func NewViper() (*viper.Viper, error) { v := viper.New() @@ -158,6 +148,14 @@ func NewViper() (*viper.Viper, error) { // FromViper takes a viper instance and produces a Config instance. func FromViper(v *viper.Viper) (*Config, error) { + configReset := map[string]any{ + "ci": false, + "clear-cache": false, + "no-cache": false, + "stdin": false, + "working-dir": ".", + } + // reset certain values which are not allowed to be specified in the config file if err := v.MergeConfigMap(configReset); err != nil { return nil, fmt.Errorf("failed to overwrite config values: %w", err) @@ -165,6 +163,7 @@ func FromViper(v *viper.Viper) (*Config, error) { // read config from viper var err error + cfg := &Config{} if err = v.Unmarshal(cfg); err != nil { @@ -217,6 +216,7 @@ func FromViper(v *viper.Viper) (*Config, error) { if !ok { return nil, fmt.Errorf("formatter %v not found in config", name) } + filtered[name] = formatterCfg } @@ -247,6 +247,7 @@ func FindUp(searchDir string, fileNames ...string) (path string, dir string, err } } } + return "", "", fmt.Errorf("could not find %s in %s", fileNames, searchDir) } @@ -268,6 +269,7 @@ func eachDir(path string) (paths []string) { if path == "" { path = "/" } + paths = append(paths, path) } } diff --git a/config/config_test.go b/config/config_test.go index 78ee49eb..a87b2126 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -8,17 +8,16 @@ import ( "path/filepath" "testing" - "github.com/numtide/treefmt/config" - "github.com/BurntSushi/toml" + "github.com/numtide/treefmt/config" "github.com/spf13/pflag" "github.com/spf13/viper" - "github.com/stretchr/testify/require" ) func newViper(t *testing.T) (*viper.Viper, *pflag.FlagSet) { t.Helper() + v, err := config.NewViper() if err != nil { t.Fatal(err) @@ -33,6 +32,7 @@ func newViper(t *testing.T) (*viper.Viper, *pflag.FlagSet) { if err := v.BindPFlags(flags); err != nil { t.Fatal(err) } + return v, flags } @@ -41,6 +41,7 @@ func readValue(t *testing.T, v *viper.Viper, cfg *config.Config, test func(*conf // serialise the config and read it into viper buf := bytes.NewBuffer(nil) + encoder := toml.NewEncoder(buf) if err := encoder.Encode(cfg); err != nil { t.Fatal(fmt.Errorf("failed to marshal config: %w", err)) @@ -106,6 +107,7 @@ func TestCI(t *testing.T) { // set config value and check that it has no effect // you are not allowed to set ci in config cfg.CI = true + checkValues(false, false, false, 0) // env override @@ -118,6 +120,7 @@ func TestCI(t *testing.T) { // increase verbosity above 1 and check it isn't reset cfg.Verbose = 2 + checkValues(true, true, true, 2) } @@ -139,6 +142,7 @@ func TestClearCache(t *testing.T) { // set config value and check that it has no effect // you are not allowed to set clear-cache in config cfg.ClearCache = true + checkValue(false) // env override @@ -158,7 +162,7 @@ func TestCpuProfile(t *testing.T) { checkValue := func(expected string) { readValue(t, v, cfg, func(cfg *config.Config) { - as.Equal(expected, cfg.CpuProfile) + as.Equal(expected, cfg.CPUProfile) }) } @@ -166,7 +170,8 @@ func TestCpuProfile(t *testing.T) { checkValue("") // set config value - cfg.CpuProfile = "/foo/bar" + cfg.CPUProfile = "/foo/bar" + checkValue("/foo/bar") // env override @@ -195,11 +200,13 @@ func TestExcludes(t *testing.T) { // set config value cfg.Excludes = []string{"foo", "bar"} + checkValue([]string{"foo", "bar"}) // test global.excludes fallback cfg.Excludes = nil cfg.Global.Excludes = []string{"fizz", "buzz"} + checkValue([]string{"fizz", "buzz"}) // env override @@ -268,6 +275,7 @@ func TestFormatters(t *testing.T) { } cfg.Formatters = []string{"echo", "touch"} + checkValue([]string{"echo", "touch"}) // env override @@ -280,6 +288,7 @@ func TestFormatters(t *testing.T) { // bad formatter name as.NoError(flags.Set("formatters", "foo,echo,date")) + _, err := config.FromViper(v) as.ErrorContains(err, "formatter foo not found in config") } @@ -302,6 +311,7 @@ func TestNoCache(t *testing.T) { // set config value and check that it has no effect // you are not allowed to set no-cache in config cfg.NoCache = true + checkValue(false) // env override @@ -330,6 +340,7 @@ func TestOnUnmatched(t *testing.T) { // set config value cfg.OnUnmatched = "error" + checkValue("error") // env override @@ -359,6 +370,7 @@ func TestTreeRoot(t *testing.T) { // set config value cfg.TreeRoot = "/foo/bar" + checkValue("/foo/bar") // env override @@ -379,10 +391,10 @@ func TestTreeRootFile(t *testing.T) { // create a directory structure with config files at various levels tempDir := t.TempDir() as.NoError(os.MkdirAll(filepath.Join(tempDir, "foo", "bar"), 0o755)) - as.NoError(os.WriteFile(filepath.Join(tempDir, "foo", "bar", "a.txt"), []byte{}, 0o644)) - as.NoError(os.WriteFile(filepath.Join(tempDir, "foo", "go.mod"), []byte{}, 0o644)) + as.NoError(os.WriteFile(filepath.Join(tempDir, "foo", "bar", "a.txt"), []byte{}, 0o600)) + as.NoError(os.WriteFile(filepath.Join(tempDir, "foo", "go.mod"), []byte{}, 0o600)) as.NoError(os.MkdirAll(filepath.Join(tempDir, ".git"), 0o755)) - as.NoError(os.WriteFile(filepath.Join(tempDir, ".git", "config"), []byte{}, 0o644)) + as.NoError(os.WriteFile(filepath.Join(tempDir, ".git", "config"), []byte{}, 0o600)) checkValue := func(treeRoot string, treeRootFile string) { readValue(t, v, cfg, func(cfg *config.Config) { @@ -401,6 +413,7 @@ func TestTreeRootFile(t *testing.T) { // set config value // should match the lowest directory cfg.TreeRootFile = "a.txt" + checkValue(workDir, "a.txt") // env override @@ -431,16 +444,17 @@ func TestVerbosity(t *testing.T) { // set config value cfg.Verbose = 1 - checkValue(1) - // env override - t.Setenv("TREEFMT_VERBOSE", "2") - checkValue(2) + checkValue(1) // flag override // todo unsure how to set a count flag via the flags api // as.NoError(flags.Set("verbose", "v")) // checkValue(1) + + // env override + t.Setenv("TREEFMT_VERBOSE", "2") + checkValue(2) } func TestWalk(t *testing.T) { @@ -460,6 +474,7 @@ func TestWalk(t *testing.T) { // set config value cfg.Walk = "git" + checkValue("git") // env override @@ -495,6 +510,7 @@ func TestWorkingDirectory(t *testing.T) { // set config value and check that it has no effect // you are not allowed to set working-dir in config cfg.WorkingDirectory = "/foo/bar/baz/../fizz" + checkValue(cwd) // env override @@ -524,6 +540,7 @@ func TestStdin(t *testing.T) { // set config value and check that it has no effect // you are not allowed to set stdin in config cfg.Stdin = true + checkValues(false) // env override diff --git a/format/cache.go b/format/cache.go index 0bd59cf9..2d6e3e61 100644 --- a/format/cache.go +++ b/format/cache.go @@ -1,6 +1,7 @@ package format import ( + "errors" "fmt" "os" @@ -25,19 +26,18 @@ func CompareFormatters(db *bolt.DB, formatters map[string]*Formatter) error { // check for any newly configured or modified formatters for name, formatter := range formatters { - stat, err := os.Lstat(formatter.Executable()) if err != nil { return fmt.Errorf("failed to stat formatter executable %v: %w", formatter.Executable(), err) } entry, err := formattersBucket.Get(name) - if err != nil { + if !(err == nil || errors.Is(err, cache.ErrKeyNotFound)) { return fmt.Errorf("failed to retrieve cache entry for formatter %v: %w", name, err) } - isNew := entry == nil - hasChanged := entry != nil && !(entry.Size == stat.Size() && entry.Modified == stat.ModTime()) + isNew := errors.Is(err, cache.ErrKeyNotFound) + hasChanged := !(isNew || (entry.Size == stat.Size() && entry.Modified == stat.ModTime())) if isNew { log.Debugf("formatter '%s' is new", name) @@ -76,6 +76,7 @@ func CompareFormatters(db *bolt.DB, formatters map[string]*Formatter) error { // indicate a clean is required clearPaths = true } + return nil }); err != nil { return fmt.Errorf("failed to check cache for removed formatters: %w", err) diff --git a/format/formatter.go b/format/formatter.go index d7ebd8ec..340bc28c 100644 --- a/format/formatter.go +++ b/format/formatter.go @@ -8,12 +8,10 @@ import ( "os/exec" "time" - "github.com/numtide/treefmt/walk" - - "github.com/numtide/treefmt/config" - "github.com/charmbracelet/log" "github.com/gobwas/glob" + "github.com/numtide/treefmt/config" + "github.com/numtide/treefmt/walk" "mvdan.cc/sh/v3/expand" "mvdan.cc/sh/v3/interp" ) @@ -35,7 +33,7 @@ type Formatter struct { excludes []glob.Glob } -// Executable returns the path to the executable defined by Command +// Executable returns the path to the executable defined by Command. func (f *Formatter) Executable() string { return f.executable } @@ -65,7 +63,7 @@ func (f *Formatter) Apply(ctx context.Context, tasks []*Task) error { } // execute the command - cmd := exec.CommandContext(ctx, f.executable, args...) + cmd := exec.CommandContext(ctx, f.executable, args...) //nolint:gosec // replace the default Cancel handler installed by CommandContext because it sends SIGKILL (-9). cmd.Cancel = func() error { return cmd.Process.Signal(os.Interrupt) @@ -77,14 +75,16 @@ func (f *Formatter) Apply(ctx context.Context, tasks []*Task) error { if out, err := cmd.CombinedOutput(); err != nil { f.log.Errorf("failed to apply with options '%v': %s", f.config.Options, err) + if len(out) > 0 { _, _ = fmt.Fprintf(os.Stderr, "%s error:\n%s\n", f.name, out) } + return fmt.Errorf("formatter '%s' with options '%v' failed to apply: %w", f.config.Command, f.config.Options, err) - } else { - f.log.Infof("%v file(s) processed in %v", len(tasks), time.Since(start)) } + f.log.Infof("%v file(s) processed in %v", len(tasks), time.Since(start)) + return nil } @@ -95,6 +95,7 @@ func (f *Formatter) Wants(file *walk.File) bool { if match { f.log.Debugf("match: %v", file) } + return match } @@ -119,6 +120,7 @@ func NewFormatter( if err != nil { return nil, ErrCommandNotFound } + f.executable = executable // initialise internal state diff --git a/format/glob.go b/format/glob.go index e03aafb9..296bd23a 100644 --- a/format/glob.go +++ b/format/glob.go @@ -15,6 +15,7 @@ func CompileGlobs(patterns []string) ([]glob.Glob, error) { if err != nil { return nil, fmt.Errorf("failed to compile include pattern '%v': %w", pattern, err) } + globs[i] = g } diff --git a/format/glob_test.go b/format/glob_test.go index 137cdb25..6a85d3c4 100644 --- a/format/glob_test.go +++ b/format/glob_test.go @@ -3,9 +3,8 @@ package format_test import ( "testing" - "github.com/numtide/treefmt/format" - "github.com/gobwas/glob" + "github.com/numtide/treefmt/format" "github.com/stretchr/testify/require" ) diff --git a/format/task.go b/format/task.go index 9da38111..922dc6db 100644 --- a/format/task.go +++ b/format/task.go @@ -25,6 +25,7 @@ func NewTask(file *walk.File, formatters []*Formatter) Task { // formatters with the same priority are sorted lexicographically to ensure a deterministic outcome result = cmp.Compare(a.Name(), b.Name()) } + return result }) @@ -33,6 +34,7 @@ func NewTask(file *walk.File, formatters []*Formatter) Task { for _, f := range formatters { key += f.name + ":" } + key = key[:len(key)-1] return Task{ diff --git a/stats/stats.go b/stats/stats.go index 7c38d0bd..70c174f9 100644 --- a/stats/stats.go +++ b/stats/stats.go @@ -19,15 +19,15 @@ const ( type Stats struct { start time.Time - counters map[Type]*atomic.Int32 + counters map[Type]*atomic.Int64 } -func (s *Stats) Add(t Type, delta int32) int32 { - return s.counters[t].Add(delta) +func (s *Stats) Add(t Type, delta int) int { + return int(s.counters[t].Add(int64(delta))) } -func (s *Stats) Value(t Type) int32 { - return s.counters[t].Load() +func (s *Stats) Value(t Type) int { + return int(s.counters[t].Load()) } func (s *Stats) Elapsed() time.Duration { @@ -53,12 +53,11 @@ func (s *Stats) Print() { } func New() Stats { - // init counters - counters := make(map[Type]*atomic.Int32) - counters[Traversed] = &atomic.Int32{} - counters[Matched] = &atomic.Int32{} - counters[Formatted] = &atomic.Int32{} - counters[Changed] = &atomic.Int32{} + counters := make(map[Type]*atomic.Int64) + counters[Traversed] = &atomic.Int64{} + counters[Matched] = &atomic.Int64{} + counters[Formatted] = &atomic.Int64{} + counters[Changed] = &atomic.Int64{} return Stats{ start: time.Now(), diff --git a/test/temp.go b/test/temp.go index 928d0fcc..274cb1dc 100644 --- a/test/temp.go +++ b/test/temp.go @@ -1,24 +1,24 @@ package test import ( - "io" "os" "testing" "time" - "github.com/numtide/treefmt/config" - "github.com/BurntSushi/toml" + "github.com/numtide/treefmt/config" cp "github.com/otiai10/copy" "github.com/stretchr/testify/require" ) func WriteConfig(t *testing.T, path string, cfg *config.Config) { t.Helper() + f, err := os.Create(path) if err != nil { t.Fatalf("failed to create a new config file: %v", err) } + encoder := toml.NewEncoder(f) if err = encoder.Encode(cfg); err != nil { t.Fatalf("failed to write to config file: %v", err) @@ -28,6 +28,7 @@ func WriteConfig(t *testing.T, path string, cfg *config.Config) { func TempExamples(t *testing.T) string { tempDir := t.TempDir() TempExamplesInDir(t, tempDir) + return tempDir } @@ -35,7 +36,7 @@ func TempExamplesInDir(t *testing.T, dir string) { require.NoError(t, cp.Copy("../test/examples", dir), "failed to copy test data to dir") // we have second precision mod time tracking, so we wait a second before returning, so we don't trigger false - //positives for things like fail on change + // positives for things like fail on change time.Sleep(time.Second) } @@ -59,18 +60,13 @@ func TempFile(t *testing.T, dir string, pattern string, contents *string) *os.Fi return file } -func ReadStdout(t *testing.T) string { - _, err := os.Stdout.Seek(0, 0) - require.NoError(t, err, "failed to seek to 0") - bytes, err := io.ReadAll(os.Stdout) - require.NoError(t, err, "failed to read") - return string(bytes) -} - func RecreateSymlink(t *testing.T, path string) error { t.Helper() + src, err := os.Readlink(path) + require.NoError(t, err, "failed to read symlink") require.NoError(t, os.Remove(path), "failed to remove symlink") + return os.Symlink(src, path) } diff --git a/walk/cache/bucket.go b/walk/cache/bucket.go index 824514b4..dd2334e0 100644 --- a/walk/cache/bucket.go +++ b/walk/cache/bucket.go @@ -12,6 +12,8 @@ const ( bucketFormatters = "formatters" ) +var ErrKeyNotFound = fmt.Errorf("key not found") + type Bucket[V any] struct { bucket *bolt.Bucket } @@ -23,12 +25,14 @@ func (b *Bucket[V]) Size() int { func (b *Bucket[V]) Get(key string) (*V, error) { bytes := b.bucket.Get([]byte(key)) if bytes == nil { - return nil, nil + return nil, ErrKeyNotFound } + var value V if err := msgpack.Unmarshal(bytes, &value); err != nil { return nil, fmt.Errorf("failed to unmarshal cache entry for key '%v': %w", key, err) } + return &value, nil } @@ -38,6 +42,7 @@ func (b *Bucket[V]) Put(key string, value *V) error { } else if err = b.bucket.Put([]byte(key), bytes); err != nil { return fmt.Errorf("failed to put cache entry for key %v: %w", key, err) } + return nil } @@ -52,6 +57,7 @@ func (b *Bucket[V]) DeleteAll() error { return fmt.Errorf("failed to remove cache entry for key %s: %w", string(k), err) } } + return nil } @@ -61,6 +67,7 @@ func (b *Bucket[V]) ForEach(f func(string, *V) error) error { if err := msgpack.Unmarshal(bytes, &value); err != nil { return fmt.Errorf("failed to unmarshal cache entry for key '%v': %w", key, err) } + return f(string(key), &value) }) } @@ -74,15 +81,20 @@ func BucketFormatters(tx *bolt.Tx) (*Bucket[Entry], error) { } func cacheBucket(name string, tx *bolt.Tx) (*Bucket[Entry], error) { - var b *bolt.Bucket - var err error + var ( + err error + b *bolt.Bucket + ) + if tx.Writable() { b, err = tx.CreateBucketIfNotExists([]byte(name)) } else { b = tx.Bucket([]byte(name)) } + if err != nil { return nil, fmt.Errorf("failed to get/create bucket %s: %w", bucketPaths, err) } + return &Bucket[Entry]{b}, nil } diff --git a/walk/cache/cache.go b/walk/cache/cache.go index 47dca987..fba706f0 100644 --- a/walk/cache/cache.go +++ b/walk/cache/cache.go @@ -1,7 +1,7 @@ package cache import ( - "crypto/sha1" + "crypto/sha1" //nolint:gosec "encoding/hex" "fmt" "io/fs" @@ -21,13 +21,15 @@ func (e *Entry) HasChanged(info fs.FileInfo) bool { } func Open(root string) (*bolt.DB, error) { - var err error - var path string + var ( + err error + path string + ) // Otherwise, the database will be located in `XDG_CACHE_DIR/treefmt/eval-cache/.db`, where is // determined by hashing the treeRoot path. // This associates a given treeRoot with a given instance of the cache. - digest := sha1.Sum([]byte(root)) + digest := sha1.Sum([]byte(root)) //nolint:gosec name := hex.EncodeToString(digest[:]) if path, err = xdg.CacheFile(fmt.Sprintf("treefmt/eval-cache/%v.db", name)); err != nil { @@ -49,7 +51,9 @@ func EnsureBuckets(db *bolt.DB) error { if _, err := BucketPaths(tx); err != nil { return err } + _, err := BucketFormatters(tx) + return err }) } @@ -60,6 +64,7 @@ func Clear(db *bolt.DB) error { if err != nil { return fmt.Errorf("failed to get paths bucket: %w", err) } + return bucket.DeleteAll() }) } diff --git a/walk/cached.go b/walk/cached.go index aa396c87..ce451f20 100644 --- a/walk/cached.go +++ b/walk/cached.go @@ -31,7 +31,7 @@ type CachedReader struct { // process updates cached file entries by batching file updates and flushing them to the database periodically. func (c *CachedReader) process() error { - var batch []*File + batch := make([]*File, 0, c.batchSize) flush := func() error { // check for an empty batch @@ -56,6 +56,7 @@ func (c *CachedReader) process() error { return fmt.Errorf("failed to put entry for path %s: %w", file.RelPath, err) } } + return nil }) } @@ -66,6 +67,8 @@ func (c *CachedReader) process() error { if err := flush(); err != nil { return err } + + // reset the batch batch = batch[:0] } } @@ -90,9 +93,11 @@ func (c *CachedReader) Read(ctx context.Context, files []*File) (n int, err erro file := files[i] // lookup cache entry and append to the file - file.Cache, err = bucket.Get(file.RelPath) - if err != nil { - return err + var bucketErr error + + file.Cache, bucketErr = bucket.Get(file.RelPath) + if !(bucketErr == nil || errors.Is(bucketErr, cache.ErrKeyNotFound)) { + return bucketErr } // set a release function which inserts this file into the update channel diff --git a/walk/cached_test.go b/walk/cached_test.go index 37bf0e75..66f1eca2 100644 --- a/walk/cached_test.go +++ b/walk/cached_test.go @@ -22,8 +22,8 @@ func TestCachedReader(t *testing.T) { batchSize := 1024 tempDir := test.TempExamples(t) - readAll := func(path string) (totalCount, newCount, changeCount int, statz stats.Stats) { - statz = stats.New() + readAll := func(path string) (totalCount, newCount, changeCount int) { + statz := stats.New() db, err := cache.Open(tempDir) as.NoError(err) @@ -62,16 +62,16 @@ func TestCachedReader(t *testing.T) { as.NoError(reader.Close()) - return totalCount, newCount, changeCount, statz + return totalCount, newCount, changeCount } - totalCount, newCount, changeCount, _ := readAll("") + totalCount, newCount, changeCount := readAll("") as.Equal(32, totalCount) as.Equal(32, newCount) as.Equal(0, changeCount) // read again, should be no changes - totalCount, newCount, changeCount, _ = readAll("") + totalCount, newCount, changeCount = readAll("") as.Equal(32, totalCount) as.Equal(0, newCount) as.Equal(0, changeCount) @@ -84,7 +84,7 @@ func TestCachedReader(t *testing.T) { as.NoError(os.Chtimes(filepath.Join(tempDir, "shell/foo.sh"), time.Now(), modTime)) as.NoError(os.Chtimes(filepath.Join(tempDir, "haskell/Nested/Foo.hs"), time.Now(), modTime)) - totalCount, newCount, changeCount, _ = readAll("") + totalCount, newCount, changeCount = readAll("") as.Equal(32, totalCount) as.Equal(0, newCount) as.Equal(3, changeCount) @@ -96,7 +96,7 @@ func TestCachedReader(t *testing.T) { _, err = os.Create(filepath.Join(tempDir, "fizz.go")) as.NoError(err) - totalCount, newCount, changeCount, _ = readAll("") + totalCount, newCount, changeCount = readAll("") as.Equal(34, totalCount) as.Equal(2, newCount) as.Equal(0, changeCount) @@ -114,23 +114,23 @@ func TestCachedReader(t *testing.T) { as.NoError(err) as.NoError(f.Close()) - totalCount, newCount, changeCount, _ = readAll("") + totalCount, newCount, changeCount = readAll("") as.Equal(34, totalCount) as.Equal(0, newCount) as.Equal(2, changeCount) // read some paths within the root - totalCount, newCount, changeCount, _ = readAll("go") + totalCount, newCount, changeCount = readAll("go") as.Equal(2, totalCount) as.Equal(0, newCount) as.Equal(0, changeCount) - totalCount, newCount, changeCount, _ = readAll("elm/src") + totalCount, newCount, changeCount = readAll("elm/src") as.Equal(1, totalCount) as.Equal(0, newCount) as.Equal(0, changeCount) - totalCount, newCount, changeCount, _ = readAll("haskell") + totalCount, newCount, changeCount = readAll("haskell") as.Equal(7, totalCount) as.Equal(0, newCount) as.Equal(0, changeCount) diff --git a/walk/filesystem.go b/walk/filesystem.go index 0980b2cc..000aa9d8 100644 --- a/walk/filesystem.go +++ b/walk/filesystem.go @@ -82,14 +82,13 @@ func (f *FilesystemReader) process() error { func (f *FilesystemReader) Read(ctx context.Context, files []*File) (n int, err error) { // ensure we record how many files we traversed defer func() { - f.stats.Add(stats.Traversed, int32(n)) + f.stats.Add(stats.Traversed, n) }() LOOP: // keep filling files up to it's length for n < len(files) { select { - // exit early if the context was cancelled case <-ctx.Done(): return n, ctx.Err() @@ -99,6 +98,7 @@ LOOP: if !ok { // channel was closed, exit the loop err = io.EOF + break LOOP } diff --git a/walk/filesystem_test.go b/walk/filesystem_test.go index 60870fb4..8465480e 100644 --- a/walk/filesystem_test.go +++ b/walk/filesystem_test.go @@ -8,13 +8,12 @@ import ( "time" "github.com/numtide/treefmt/stats" - - "github.com/numtide/treefmt/walk" - "github.com/numtide/treefmt/test" + "github.com/numtide/treefmt/walk" "github.com/stretchr/testify/require" ) +//nolint:gochecknoglobals var examplesPaths = []string{ "elm/elm.json", "elm/src/Main.elm", @@ -80,8 +79,8 @@ func TestFilesystemReader(t *testing.T) { } as.Equal(32, count) - as.Equal(int32(32), statz.Value(stats.Traversed)) - as.Equal(int32(0), statz.Value(stats.Matched)) - as.Equal(int32(0), statz.Value(stats.Formatted)) - as.Equal(int32(0), statz.Value(stats.Changed)) + as.Equal(32, statz.Value(stats.Traversed)) + as.Equal(0, statz.Value(stats.Matched)) + as.Equal(0, statz.Value(stats.Formatted)) + as.Equal(0, statz.Value(stats.Changed)) } diff --git a/walk/git.go b/walk/git.go index edf88873..7681d150 100644 --- a/walk/git.go +++ b/walk/git.go @@ -29,7 +29,7 @@ type GitReader struct { func (g *GitReader) Read(ctx context.Context, files []*File) (n int, err error) { // ensure we record how many files we traversed defer func() { - g.stats.Add(stats.Traversed, int32(n)) + g.stats.Add(stats.Traversed, n) }() if g.scanner == nil { @@ -54,7 +54,6 @@ LOOP: for n < len(files) { select { - // exit early if the context was cancelled case <-ctx.Done(): return n, ctx.Err() @@ -72,6 +71,7 @@ LOOP: g.log.Warnf( "Path %s is in the worktree but appears to have been removed from the filesystem", path, ) + continue } else if err != nil { return n, fmt.Errorf("failed to stat %s: %w", path, err) @@ -83,10 +83,10 @@ LOOP: Info: info, } n++ - } else { // nothing more to read err = io.EOF + break LOOP } } diff --git a/walk/git_test.go b/walk/git_test.go index c7f8ff85..6e6177b5 100644 --- a/walk/git_test.go +++ b/walk/git_test.go @@ -63,8 +63,8 @@ func TestGitReader(t *testing.T) { } as.Equal(32, count) - as.Equal(int32(32), statz.Value(stats.Traversed)) - as.Equal(int32(0), statz.Value(stats.Matched)) - as.Equal(int32(0), statz.Value(stats.Formatted)) - as.Equal(int32(0), statz.Value(stats.Changed)) + as.Equal(32, statz.Value(stats.Traversed)) + as.Equal(0, statz.Value(stats.Matched)) + as.Equal(0, statz.Value(stats.Formatted)) + as.Equal(0, statz.Value(stats.Changed)) } diff --git a/walk/stdin.go b/walk/stdin.go index 1f86fc09..a74d82e0 100644 --- a/walk/stdin.go +++ b/walk/stdin.go @@ -91,7 +91,7 @@ func (s StdinReader) Close() error { return nil } -func NewStdinReader(root string, path string, statz *stats.Stats) Reader { +func NewStdinReader(root string, path string, statz *stats.Stats) StdinReader { return StdinReader{ root: root, path: path, diff --git a/walk/walk.go b/walk/walk.go index 47190749..4109b442 100644 --- a/walk/walk.go +++ b/walk/walk.go @@ -49,6 +49,7 @@ func (f *File) Release(formatErr error) error { return err } } + return nil } @@ -136,9 +137,11 @@ func (c *CompositeReader) Close() error { return fmt.Errorf("failed to close reader: %w", err) } } + return nil } +//nolint:ireturn func NewReader( walkType Type, root string, @@ -158,6 +161,7 @@ func NewReader( if err != nil { reader, err = NewReader(Filesystem, root, path, db, statz) } + return reader, err case Stdin: return nil, fmt.Errorf("stdin walk type is not supported") @@ -183,6 +187,7 @@ func NewReader( return reader, err } +//nolint:ireturn func NewCompositeReader( walkType Type, root string,