Add new Lint/ConstantDefinitionInBlock cop #8707
Conversation
| # describe 'making a request' do | ||
| # let(:test_request) { Class.new } | ||
| # end | ||
| class ConstantDefinedInBlock < Base |
There was a problem hiding this comment.
Probably we should name this ConstantDefinitionInBlock to be more consistent with existing cop names.
There was a problem hiding this comment.
Good call, I updated the name.
|
Nice cop! I think we should also mention this in the Ruby Style Guide, as it might not be super obvious to some people. |
Constants defined within a block are added to the current lexical scope, which means the block has no effect on where the constant is defined or how it can be accessed. I often see constants defined inside Rake tasks or RSpec `describe` blocks where the intent was clearly for them to be scoped to the block, but which actually leak into the top-level scope.
eugeneius
force-pushed
the
constant_defined_in_block
branch
from
a88d464
to
d76e91e
Compare
eugeneius
changed the title
|
I opened rubocop/ruby-style-guide#841 to add a corresponding guideline to the Ruby Style Guide. |
|
Thanks! I'll merge the cop as it is, as I might cut a new release today, but we should add a link to the new guideline to the cop metadata. |
Follow rubocop/rubocop#8707 This PR suppresses the following RuboCop's offense. ```console % bundle exec rake (snip) Offenses: spec/rubocop/cop/active_record_helper_spec.rb:6:3: W: Lint/ConstantDefinitionInBlock: Do not define constants within a block. module RuboCop ... ^^^^^^^^^^^^^^ 185 files inspected, 1 offense detected, 1 offense auto-correctable RuboCop failed! ```
…en fixed * Update Rubocop from [0.90](https://github.com/rubocop-hq/rubocop/releases/tag/v0.90.0) to [0.91](https://github.com/rubocop-hq/rubocop/releases/tag/v0.91.0), enabling: * [`Lint/UselessTimes`](rubocop/rubocop#8702) * [`Layout/BeginEndAlignment`](rubocop/rubocop#8628) * [`Lint/ConstantDefinitionInBlock`](rubocop/rubocop#8707) * [`Lint/IdentityComparison`](rubocop/rubocop#8699) re-enabling after bug fixes: * [`Bundler/DuplicatedGem`](rubocop/rubocop#8666) * [`Naming/BinaryOperatorParameterName`](rubocop/rubocop#8664)
|
FYI for anyone who comes across this, this cop isn't compatible with @marcandre do you want me to do a docs PR for this one too? |
Yes, that would be a great starting point. At a minimum we should document these cases, and afterwards see what is the best to address them. In this case, we could exclude |
Constants defined within a block are added to the current lexical scope, which means the block has no effect on where the constant is defined or how it can be accessed. I often see constants defined inside Rake tasks or RSpec
describeblocks where the intent was clearly for them to be scoped to the block, but which actually leak into the top-level scope.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.