Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new Lint/ConstantDefinitionInBlock cop
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.
- Loading branch information
Showing
8 changed files
with
246 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Lint | ||
# Do not define constants within a block, since the block's scope does not | ||
# isolate or namespace the constant in any way. | ||
# | ||
# Define the constant outside of the block instead, or use a variable or | ||
# method if defining the constant in the outer scope would be problematic. | ||
# | ||
# @example | ||
# # bad | ||
# task :lint do | ||
# FILES_TO_LINT = Dir['lib/*.rb'] | ||
# end | ||
# | ||
# # bad | ||
# describe 'making a request' do | ||
# class TestRequest; end | ||
# end | ||
# | ||
# # good | ||
# task :lint do | ||
# files_to_lint = Dir['lib/*.rb'] | ||
# end | ||
# | ||
# # good | ||
# describe 'making a request' do | ||
# let(:test_request) { Class.new } | ||
# end | ||
class ConstantDefinitionInBlock < Base | ||
MSG = 'Do not define constants within a block.' | ||
|
||
def_node_matcher :constant_assigned_in_block?, <<~PATTERN | ||
({^block_type? [^begin_type? ^^block_type?]} nil? ...) | ||
PATTERN | ||
|
||
def_node_matcher :module_defined_in_block?, <<~PATTERN | ||
({^block_type? [^begin_type? ^^block_type?]} ...) | ||
PATTERN | ||
|
||
def on_casgn(node) | ||
add_offense(node) if constant_assigned_in_block?(node) | ||
end | ||
|
||
def on_class(node) | ||
add_offense(node) if module_defined_in_block?(node) | ||
end | ||
alias on_module on_class | ||
end | ||
end | ||
end | ||
end |
137 changes: 137 additions & 0 deletions
137
spec/rubocop/cop/lint/constant_definition_in_block_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe RuboCop::Cop::Lint::ConstantDefinitionInBlock do | ||
subject(:cop) { described_class.new(config) } | ||
|
||
let(:config) { RuboCop::Config.new } | ||
|
||
it 'does not register an offense for a top-level constant' do | ||
expect_no_offenses(<<~RUBY) | ||
FOO = 1 | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a top-level constant followed by another statement' do | ||
expect_no_offenses(<<~RUBY) | ||
FOO = 1 | ||
bar | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a constant defined within a block' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
FOO = 1 | ||
^^^^^^^ Do not define constants within a block. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a constant defined within a block followed by another statement' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
FOO = 1 | ||
^^^^^^^ Do not define constants within a block. | ||
bar | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a top-level class' do | ||
expect_no_offenses(<<~RUBY) | ||
class Foo; end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a top-level class followed by another statement' do | ||
expect_no_offenses(<<~RUBY) | ||
class Foo; end | ||
bar | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a class defined within a block' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
class Foo; end | ||
^^^^^^^^^^^^^^ Do not define constants within a block. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a class defined within a block followed by another statement' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
class Foo; end | ||
^^^^^^^^^^^^^^ Do not define constants within a block. | ||
bar | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a top-level module' do | ||
expect_no_offenses(<<~RUBY) | ||
module Foo; end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a top-level module followed by another statement' do | ||
expect_no_offenses(<<~RUBY) | ||
module Foo; end | ||
bar | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a module defined within a block' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
module Foo; end | ||
^^^^^^^^^^^^^^^ Do not define constants within a block. | ||
end | ||
RUBY | ||
end | ||
|
||
it 'registers an offense for a module defined within a block followed by another statement' do | ||
expect_offense(<<~RUBY) | ||
describe do | ||
module Foo; end | ||
^^^^^^^^^^^^^^^ Do not define constants within a block. | ||
bar | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a constant with an explicit self scope' do | ||
expect_no_offenses(<<~RUBY) | ||
describe do | ||
self::FOO = 1 | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a constant with an explicit self scope followed by another statement' do | ||
expect_no_offenses(<<~RUBY) | ||
describe do | ||
self::FOO = 1 | ||
bar | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a constant with an explicit top-level scope' do | ||
expect_no_offenses(<<~RUBY) | ||
describe do | ||
::FOO = 1 | ||
end | ||
RUBY | ||
end | ||
|
||
it 'does not register an offense for a constant with an explicit top-level scope followed by another statement' do | ||
expect_no_offenses(<<~RUBY) | ||
describe do | ||
::FOO = 1 | ||
bar | ||
end | ||
RUBY | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters