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 Lint/ConstantDefinitionInBlock cop #8707

Merged
merged 1 commit into from Sep 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@
* [#8582](https://github.com/rubocop-hq/rubocop/issues/8582): Add new `Layout/BeginEndAlignment` cop. ([@koic][])
* [#8699](https://github.com/rubocop-hq/rubocop/pull/8699): Add new `Lint/IdentityComparison` cop. ([@koic][])
* Add new `Lint/UselessTimes` cop. ([@dvandersluis][])
* [#8707](https://github.com/rubocop-hq/rubocop/pull/8707): Add new `Lint/ConstantDefinitionInBlock` cop. ([@eugeneius][])

### Bug fixes

Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -1395,6 +1395,11 @@ Lint/CircularArgumentReference:
Enabled: true
VersionAdded: '0.33'

Lint/ConstantDefinitionInBlock:
Description: 'Do not define constants within a block.'
Enabled: pending
VersionAdded: '0.91'

Lint/ConstantResolution:
Description: 'Check that constants are fully qualified with `::`.'
Enabled: false
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Expand Up @@ -190,6 +190,7 @@ In the following section you find all available cops:
* xref:cops_lint.adoc#lintbinaryoperatorwithidenticaloperands[Lint/BinaryOperatorWithIdenticalOperands]
* xref:cops_lint.adoc#lintbooleansymbol[Lint/BooleanSymbol]
* xref:cops_lint.adoc#lintcircularargumentreference[Lint/CircularArgumentReference]
* xref:cops_lint.adoc#lintconstantdefinitioninblock[Lint/ConstantDefinitionInBlock]
* xref:cops_lint.adoc#lintconstantresolution[Lint/ConstantResolution]
* xref:cops_lint.adoc#lintdebugger[Lint/Debugger]
* xref:cops_lint.adoc#lintdeprecatedclassmethods[Lint/DeprecatedClassMethods]
Expand Down
43 changes: 43 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -350,6 +350,49 @@ def cook(dry_ingredients = self.dry_ingredients)
end
----

== Lint/ConstantDefinitionInBlock

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| No
| 0.91
| -
|===

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.

=== Examples

[source,ruby]
----
# 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
----

== Lint/ConstantResolution

|===
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -252,6 +252,7 @@
require_relative 'rubocop/cop/lint/binary_operator_with_identical_operands'
require_relative 'rubocop/cop/lint/boolean_symbol'
require_relative 'rubocop/cop/lint/circular_argument_reference'
require_relative 'rubocop/cop/lint/constant_definition_in_block'
require_relative 'rubocop/cop/lint/constant_resolution'
require_relative 'rubocop/cop/lint/debugger'
require_relative 'rubocop/cop/lint/deprecated_class_methods'
Expand Down
54 changes: 54 additions & 0 deletions lib/rubocop/cop/lint/constant_definition_in_block.rb
@@ -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 spec/rubocop/cop/lint/constant_definition_in_block_spec.rb
@@ -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
8 changes: 4 additions & 4 deletions tasks/prof.rake
@@ -1,7 +1,7 @@
# frozen_string_literal: true

namespace :prof do
DUMP_PATH = 'tmp/stackprof.dump'
dump_path = 'tmp/stackprof.dump'

desc 'Run RuboCop on itself with profiling on'
task :run, [:path] do |_task, args|
Expand All @@ -11,13 +11,13 @@ namespace :prof do
end

task :run_if_needed, [:path] do
Rake::Task[:run].run unless File.exist?(DUMP_PATH)
Rake::Task[:run].run unless File.exist?(dump_path)
end

desc 'List the slowest cops'
task slow_cops: :run_if_needed do
method = 'RuboCop::Cop::Commissioner#trigger_responding_cops'
cmd = "stackprof #{DUMP_PATH} --text --method '#{method}'"
cmd = "stackprof #{dump_path} --text --method '#{method}'"
puts cmd
output = `#{cmd}`
_header, list, _code = *output
Expand All @@ -33,7 +33,7 @@ namespace :prof do
warn 'usage: bundle exec rake walk[Class#method]'
exit!
end
cmd = "stackprof #{DUMP_PATH} --walk --method '#{method}'"
cmd = "stackprof #{dump_path} --walk --method '#{method}'"
puts cmd
system cmd
end
Expand Down