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

Add new Style/ExplicitBlockArgument cop #8415

Merged
merged 1 commit into from Aug 1, 2020

Conversation

fatkodima
Copy link
Contributor

https://rubystyle.guide/#block-argument

Consider using explicit block argument to avoid writing block literal that just passes its arguments to another block.

require 'tempfile'

# bad
def with_tmp_dir
  Dir.mktmpdir do |tmp_dir|
    Dir.chdir(tmp_dir) { |dir| yield dir }  # block just passes arguments
  end
end

# good
def with_tmp_dir(&block)
  Dir.mktmpdir do |tmp_dir|
    Dir.chdir(tmp_dir, &block)
  end
end

with_tmp_dir do |dir|
  puts "dir is accessible as a parameter and pwd is set: #{dir}"
end

include RangeHelp
extend AutoCorrector

MSG = 'Consider using explicit block argument.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this message is a bit confusing, as it's not very clear where is this argument supposed to be. Perhaps something like "Consider using explicit block argument in the surrounding method's signature over yield." or something along those lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected.

@fatkodima fatkodima force-pushed the explicit_block_argument-cop branch from e52c8df to 5f02f80 Compare July 31, 2020 11:44
@marcandre
Copy link
Contributor

Please mark as unsafe (as it may change the yielding arity)

@fatkodima fatkodima force-pushed the explicit_block_argument-cop branch from 5f02f80 to a591ac2 Compare July 31, 2020 16:30
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 1, 2020

Btw, I've been thinking it'd be great if we started to add to the cop descriptions some explanations as to what exactly makes them unsafe so it's more obvious for the users.

@fatkodima
Copy link
Contributor Author

Yes, I have the same thought and already added a line about this ))

@bbatsov bbatsov merged commit e1c057d into rubocop:master Aug 1, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 1, 2020

Thanks!

@marcandre
Copy link
Contributor

marcandre commented Aug 1, 2020

Btw, I've been thinking it'd be great if we started to add to the cop descriptions some explanations as to what exactly makes them unsafe so it's more obvious for the users.

Yes! With an example too...

and already added a line about this

I don't see it Oh, I see it. We need it to show in the doc and an example would help.

@fatkodima
Copy link
Contributor Author

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 1, 2020

Going forward I think we should also add such explanation and examples to the cop description from which the documentation is generated.

@marcandre
Copy link
Contributor

I created #8431

koic added a commit to rubocop/rubocop-rails that referenced this pull request Aug 2, 2020
Follow rubocop/rubocop#8415

This commit suppresses the following `Style/ExplicitBlockArgument`'s
offenses.

```console
% bundle exec rake
(snip)

Offenses:

lib/rubocop/rails/schema_loader/schema.rb:105:13: C:
Style/ExplicitBlockArgument: Consider using explicit block argument in
the surrounding method's signature over yield.
            node.body.children.each do |child| ...
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

181 files inspected, 1 offense detected
RuboCop failed!
```
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

3 participants