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

Update auto-gen-config's comment re auto-correct #10384

Merged

Conversation

maxjacobson
Copy link
Contributor

The auto-gen-config output will say "Cop supports --auto-correct" for
cops which do not support --auto-correct, but which do support
--auto-correct-all. Let's update the output to reflect that. When
someone is working on the todo list, this can be helpful information to
have at hand.

I'll often run something like:

bin/rubocop --only Style/StringConcatenation -a and then realize
actually that won't work, and this would save me a few seconds.


Hi there. This is my first contribution in a few years. Happy to be back. WDYT about this idea?

Unfortunately bundle exec rake default is failing for me locally with this test failure, which is unrelated to my change:

Failure
==> Failures

  1) RuboCop::TargetFinder#find_files prevents infinite loops when traversing symlinks
     Failure/Error: expect(found_basenames).to include('ruby1.rb').once

     NoMethodError:
       undefined method `once' for #<RSpec::Matchers::BuiltIn::Include:0x00007fc6f360f678>
     # ./spec/rubocop/target_finder_spec.rb:454:in `block (3 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (4 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `chdir'
     # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (3 levels) in <top (required)>'
     # ./lib/rubocop/rspec/shared_contexts.rb:7:in `block (2 levels) in <top (required)>'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner/rspec3.rb:37:in `block (3 levels) in run_specs'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/iterator.rb:57:in `block in each'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:406:in `around_filter'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/iterator.rb:57:in `call'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/iterator.rb:57:in `each'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner/rspec3.rb:26:in `map'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner/rspec3.rb:26:in `block (2 levels) in run_specs'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner/rspec3.rb:25:in `block in run_specs'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner/rspec3.rb:24:in `run_specs'
     # tasks/spec_runner.rake:83:in `run_worker'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:301:in `block (2 levels) in spawn_workers'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:296:in `fork'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:296:in `block in spawn_workers'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:293:in `times'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:293:in `spawn_workers'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:238:in `execute_internal'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/test-queue-0.4.2/lib/test_queue/runner.rb:129:in `execute'
     # tasks/spec_runner.rake:38:in `block in run_specs'
     # tasks/spec_runner.rake:52:in `with_encoding'
     # tasks/spec_runner.rake:36:in `run_specs'
     # tasks/spec_runner.rake:165:in `block in <top (required)>'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:281:in `block in execute'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:281:in `each'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:281:in `execute'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:219:in `block in invoke_with_call_chain'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:199:in `invoke_with_call_chain'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:243:in `block in invoke_prerequisites'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:241:in `each'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:241:in `invoke_prerequisites'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:218:in `block in invoke_with_call_chain'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:199:in `invoke_with_call_chain'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/task.rb:188:in `invoke'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:160:in `invoke_task'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:116:in `block (2 levels) in top_level'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:116:in `each'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:116:in `block in top_level'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:125:in `run_with_threads'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:110:in `top_level'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:83:in `block in run'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:186:in `standard_exception_handling'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/lib/rake/application.rb:80:in `run'
     # /Users/max.jacobson/.gem/ruby/2.6.6/gems/rake-13.0.6/exe/rake:27:in `<top (required)>'


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

The auto-gen-config output will say "Cop supports --auto-correct" for
cops which do not support --auto-correct, but which do support
--auto-correct-all. Let's update the output to reflect that. When
someone is working on the todo list, this can be helpful information to
have at hand.

I'll often run something like:

`bin/rubocop --only Style/StringConcatenation -a` and then realize
actually that won't work, and this would save me a few seconds.
todo_contents = File.read('.rubocop_todo.yml').lines[8..-1].join
expect(todo_contents).to eq(<<~YAML)
# Offense count: 1
# Cop supports --auto-correct-all.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is the relevant assertion. Other examples would say --auto-correct instead of --auto-correct-all

@bbatsov bbatsov merged commit ab1e5b7 into rubocop:master Feb 3, 2022
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 3, 2022

I'm fine with the proposed changes, even if I'm starting to get confused by our own terminology. I had forgotten that we made --auto-correct an alias for --safe-auto-correct a while ago. Perhaps it'd be better to expand on the comments like "This cop support safe auto-correction (--auto-correct)" and "This cop supports unsafe auto-correction (--auto-correct-all)" or something along those lines.

@maxjacobson maxjacobson deleted the auto-gen-config-safe-auto-correct branch February 13, 2022 16:58
@maxjacobson
Copy link
Contributor Author

Thanks for merging! I tend to agree :) #10414

ydah added a commit to ydah/rubocop that referenced this pull request May 27, 2022
…: false`

This PR is enhancement: rubocop#10384

The auto-gen-config output will say "Cop supports --auto-correct".
However, Cop with `SafeAutoCorrect: false`
outputs the same as follows:

```yaml
# This cop supports safe auto-correction (--auto-correct).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
```

We'll want to execute the following commands
when we see it.

```
bundle exec rubocop --auto-correct foo.rb
```

However, it is not automatically corrected.

```
Offenses:

foo.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
[2, 1, 3].sort.first
^

1 file inspected, 5 offenses detected, 5 more offenses can be corrected with `rubocop -A`
```

So, we should suggest --auto-correct-all
for `SafeAutoCorrect: false` Cop as well.
bbatsov pushed a commit that referenced this pull request May 31, 2022
…: false`

This PR is enhancement: #10384

The auto-gen-config output will say "Cop supports --auto-correct".
However, Cop with `SafeAutoCorrect: false`
outputs the same as follows:

```yaml
# This cop supports safe auto-correction (--auto-correct).
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
```

We'll want to execute the following commands
when we see it.

```
bundle exec rubocop --auto-correct foo.rb
```

However, it is not automatically corrected.

```
Offenses:

foo.rb:1:1: C: [Correctable] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
[2, 1, 3].sort.first
^

1 file inspected, 5 offenses detected, 5 more offenses can be corrected with `rubocop -A`
```

So, we should suggest --auto-correct-all
for `SafeAutoCorrect: false` Cop as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants