Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apply all formatters even when there's errors #366

Merged
merged 1 commit into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ type Format struct {
globalExcludes []glob.Glob

filesCh chan *walk.File
formattedCh chan *walk.File
processedCh chan *walk.File
formattedCh chan *format.Task
processedCh chan *format.Task
}

func (f *Format) configureLogging() {
Expand Down
62 changes: 41 additions & 21 deletions cli/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ func (f *Format) Run() (err error) {
f.filesCh = make(chan *walk.File, BatchSize*runtime.NumCPU())

// create a channel for files that have been formatted
f.formattedCh = make(chan *walk.File, cap(f.filesCh))
f.formattedCh = make(chan *format.Task, cap(f.filesCh))

// create a channel for files that have been processed
f.processedCh = make(chan *walk.File, cap(f.filesCh))
f.processedCh = make(chan *format.Task, cap(f.filesCh))

// start concurrent processing tasks in reverse order
eg.Go(f.updateCache(ctx))
Expand Down Expand Up @@ -317,17 +317,21 @@ func (f *Format) applyFormatters(ctx context.Context) func() error {

// asynchronously apply the sequence formatters to the batch
fg.Go(func() error {
// iterate the formatters, applying them in sequence to the batch of tasks
// we get the formatters list from the first task since they have all the same formatters list
for _, f := range tasks[0].Formatters {
if err := f.Apply(ctx, tasks); err != nil {
return err
// Iterate the formatters, applying them in sequence to the batch of tasks.
// We get the formatter list from the first task since they have all the same formatters list.
formatters := tasks[0].Formatters

var formatErrors []error
for idx := range formatters {
if err := formatters[idx].Apply(ctx, tasks); err != nil {
formatErrors = append(formatErrors, err)
}
}

// pass each file to the formatted channel
for _, task := range tasks {
f.formattedCh <- task.File
task.Errors = formatErrors
f.formattedCh <- task
}

return nil
Expand Down Expand Up @@ -359,7 +363,9 @@ func (f *Format) applyFormatters(ctx context.Context) func() error {
if format.PathMatches(file.RelPath, f.globalExcludes) {
log.Debugf("path matched global excludes: %s", file.RelPath)
// mark it as processed and continue to the next
f.formattedCh <- file
f.formattedCh <- &format.Task{
File: file,
}
continue
}

Expand All @@ -378,7 +384,9 @@ func (f *Format) applyFormatters(ctx context.Context) func() error {
}
log.Logf(f.OnUnmatched, "no formatter for path: %s", file.RelPath)
// mark it as processed and continue to the next
f.formattedCh <- file
f.formattedCh <- &format.Task{
File: file,
}
} else {
// record the match
stats.Add(stats.Matched, 1)
Expand Down Expand Up @@ -414,14 +422,15 @@ func (f *Format) detectFormatted(ctx context.Context) func() error {
// detect ctx cancellation
case <-ctx.Done():
return ctx.Err()
// take the next file that has been processed
case file, ok := <-f.formattedCh:
// take the next task that has been processed
case task, ok := <-f.formattedCh:
if !ok {
// channel has been closed, no further files to process
return nil
}

// check if the file has changed
file := task.File
changed, newInfo, err := file.HasChanged()
if err != nil {
return err
Expand Down Expand Up @@ -451,7 +460,7 @@ func (f *Format) detectFormatted(ctx context.Context) func() error {
}

// mark as processed
f.processedCh <- file
f.processedCh <- task
}
}
}
Expand All @@ -460,12 +469,16 @@ func (f *Format) detectFormatted(ctx context.Context) func() error {
func (f *Format) updateCache(ctx context.Context) func() error {
return func() error {
// used to batch updates for more efficient txs
batch := make([]*walk.File, 0, BatchSize)
batch := make([]*format.Task, 0, BatchSize)

// apply a batch
processBatch := func() error {
// pass the batch to the cache for updating
if err := cache.Update(batch); err != nil {
files := make([]*walk.File, len(batch))
for idx := range batch {
files[idx] = batch[idx].File
}
if err := cache.Update(files); err != nil {
return err
}
batch = batch[:0]
Expand All @@ -486,12 +499,14 @@ func (f *Format) updateCache(ctx context.Context) func() error {
case <-ctx.Done():
return ctx.Err()
// respond to formatted files
case file, ok := <-f.processedCh:
case task, ok := <-f.processedCh:
if !ok {
// channel has been closed, no further files to process
break LOOP
}

file := task.File

if f.Stdin {
// dump file into stdout
f, err := os.Open(file.Path)
Expand All @@ -508,11 +523,16 @@ func (f *Format) updateCache(ctx context.Context) func() error {
continue
}

// append to batch and process if we have enough
batch = append(batch, file)
if len(batch) == BatchSize {
if err := processBatch(); err != nil {
return err
// Append to batch and process if we have enough.
// We do not cache any files that were part of a pipeline in which one or more formatters failed.
// This is to ensure those files are re-processed in later invocations after the user has potentially
// resolved the issue, e.g. fixed a config problem.
if len(task.Errors) == 0 {
batch = append(batch, task)
if len(batch) == BatchSize {
if err := processBatch(); err != nil {
return err
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ updated `nix/packages/treefmt/gomod2nix.toml`.

To sync it up, run `nix develop .#renovate -c gomod2nix:update`.



## Making changes

If you want to introduce changes to the project, please follow these steps:
Expand Down
7 changes: 3 additions & 4 deletions format/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,15 @@ func (f *Formatter) Apply(ctx context.Context, tasks []*Task) error {
f.log.Debugf("executing: %s", cmd.String())

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
}

Expand Down
1 change: 1 addition & 0 deletions format/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type Task struct {
File *walk.File
Formatters []*Formatter
BatchKey string
Errors []error
}

func NewTask(file *walk.File, formatters []*Formatter) Task {
Expand Down