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.

CHANGELOG.md

@@ -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
 

config/default.yml

@@ -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

docs/modules/ROOT/pages/cops.adoc

@@ -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]

docs/modules/ROOT/pages/cops_lint.adoc

@@ -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
 
 |===

lib/rubocop.rb

@@ -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'

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

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

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|
@@ -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
@@ -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