Not making fixes in stdout when using parallel: true

[paul]

I'm attempting to use standard in Neovim (through Mason -> null-ls.nvim, but that probably doesn't matter). When I try to have it format my file, it does nothing. In the logs, there's a message "buffer unchanged", and poking at it, it seems like standard is outputting the original version of the file, instead of the one it fixed. I've extracted the commands it runs, and here's the equivalent command and its output (truncated for simplicity):

$ cat /path/to/app/db/migrate/20230308220023_create_good_jobs.rb | standardrb --fix --format quiet --stderr --stdin /path/to/app/db/migrate/20230308220023_create_good_jobs.rb
== db/migrate/20230308220023_create_good_jobs.rb ==
C:  5: 22: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
# ...

1 file inspected, 16 offenses detected, 16 offenses corrected
====================
# frozen_string_literal: true

class CreateGoodJobs < ActiveRecord::Migration[7.0]
  def change
    enable_extension 'pgcrypto'

    create_table :good_jobs, id: :uuid do |t|
# ...

It detected the single-quoted string and says it corrected it, but the output on stdout is the original file, with single-quoted 'pgcrypto' unchanged.

If I do the equivalent thing in rubocop, it outputs the fixes, like I'd expect:

$ cat /path/to/app/db/migrate/20230308220023_create_good_jobs.rb | rubocop --autocorrect --format quiet --stderr --stdin /path/to/app/workshop/db/migrate/20230308220023_create_good_jobs.rb
== db/migrate/20230308220023_create_good_jobs.rb ==
C:  5: 22: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (https://rubystyle.guide#consistent-string-literals)
# ...

1 file inspected, 16 offenses detected, 16 offenses corrected
====================
# frozen_string_literal: true

class CreateGoodJobs < ActiveRecord::Migration[7.0]
  def change
    enable_extension "pgcrypto"

    create_table :good_jobs, id: :uuid do |t|
# ...

This project does have a .standard.yml file in it, with the following contents:

fix: true
parallel: true
format: progress

I tried removing each of them, and discovered if I removed parallel: true, it works. Leaving the others in there doesn't impact the output:

$ cat /path/to/app/db/migrate/20230308220023_create_good_jobs.rb | standardrb --fix --format quiet --stderr --stdin /path/to/app/db/migrate/20230308220023_create_good_jobs.rb
== db/migrate/20230308220023_create_good_jobs.rb ==
C:  5: 22: [Corrected] Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
# ...

1 file inspected, 16 offenses detected, 16 offenses corrected
====================
# frozen_string_literal: true

class CreateGoodJobs < ActiveRecord::Migration[7.0]
  def change
    enable_extension "pgcrypto"

    create_table :good_jobs, id: :uuid do |t|
# ...

This might be the same issue as #530?

Versions:

        Ruby version: 3.2.0
     RuboCop version: 1.42.0
    Standard version: 1.22.1

[searls]

I was able to quickly reproduce this issue. Weirdly, I'd already stumbled on this issue and even commented on it—but I mistakenly assumed that this only affected stdin.

Finding that comment was a great help, since it pointed me to the exact line that breaks things:

    def run(paths)
      target_files = find_target_files(paths)
      if @options[:list_target_files]
        list_files(target_files)
      else
        warm_cache(target_files) if @options[:parallel] #<--- This one!
        inspect_files(target_files)
      end
      #…
    end

That's as far as I've gotten so far, but wanted to document that much. Next step is to look into what the warm_cache method is doing and why it's behaving differently for standard than for rubocop

[searls]

One layer deeper, the error seems to be occurring because of this dup call on @options.

    def warm_cache(target_files)
      saved_options = @options.dup #<-- This right here
      if target_files.length <= 1
        puts 'Skipping parallel inspection: only a single file needs inspection' if @options[:debug]
        return
      end
      puts 'Running parallel inspection' if @options[:debug]
      %i[autocorrect safe_autocorrect].each { |opt| @options[opt] = false }
      Parallel.each(target_files) { |target_file| file_offenses(target_file) }
    ensure
      @options = saved_options
    end

My test passes when I simply replace saved_options = @options.dup with saved_options = @options.

[searls]

Woops, nevermind I was wrong when I said this:

I mistakenly assumed that this only affected stdin.

Past me was right. This only affects stdin.

Today I had forgotten that the test cases that call through to the CLI are doing so under stdin. Plain ol' parallel works fine; only stdin fails.

To be honest, does parallel even make sense over stdin? It only supports one path at a time. Seems more reasonable to just strip out the --parallel when invoking the runner in the presence of --stdin or -s (its alias) than to let Rubocop do the wrong thing.

[searls]

This should be fixed in 1.25.1.

[paul]

Thanks @searls!