Merge pull request #34764 from kamipo/avoid_redundant_begin

Enable `Style/RedundantBegin` cop to avoid newly adding redundant begin block

9 files changed, 153 insertions, 178 deletions

diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb
index 53a2b07536..de1fb1886c 100644
--- a/activesupport/lib/active_support/cache/file_store.rb
+++ b/activesupport/lib/active_support/cache/file_store.rb
@@ -108,12 +108,10 @@ module ActiveSupport
         def lock_file(file_name, &block)
           if File.exist?(file_name)
             File.open(file_name, "r+") do |f|
-              begin
-                f.flock File::LOCK_EX
-                yield
-              ensure
-                f.flock File::LOCK_UN
-              end
+              f.flock File::LOCK_EX
+              yield
+            ensure
+              f.flock File::LOCK_UN
             end
           else
             yield
diff --git a/activesupport/lib/active_support/evented_file_update_checker.rb b/activesupport/lib/active_support/evented_file_update_checker.rb
index 1ef8584c52..54bc5c0ae5 100644
--- a/activesupport/lib/active_support/evented_file_update_checker.rb
+++ b/activesupport/lib/active_support/evented_file_update_checker.rb
@@ -60,11 +60,9 @@ module ActiveSupport
         # usage of attr_* macros for private attributes, but adds a lot of noise
         # to our test suite. Thus, we lazy load it and disable warnings locally.
         silence_warnings do
-          begin
-            require "listen"
-          rescue LoadError => e
-            raise LoadError, "Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile", e.backtrace
-          end
+          require "listen"
+        rescue LoadError => e
+          raise LoadError, "Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile", e.backtrace
         end
       end
       boot!
diff --git a/activesupport/lib/active_support/reloader.rb b/activesupport/lib/active_support/reloader.rb
index fea18e9712..2f81cd4f80 100644
--- a/activesupport/lib/active_support/reloader.rb
+++ b/activesupport/lib/active_support/reloader.rb
@@ -50,11 +50,9 @@ module ActiveSupport
     def self.reload!
       executor.wrap do
         new.tap do |instance|
-          begin
-            instance.run!
-          ensure
-            instance.complete!
-          end
+          instance.run!
+        ensure
+          instance.complete!
         end
       end
       prepare!
diff --git a/activesupport/lib/active_support/testing/parallelization.rb b/activesupport/lib/active_support/testing/parallelization.rb
index c5d3a88131..63440069b1 100644
--- a/activesupport/lib/active_support/testing/parallelization.rb
+++ b/activesupport/lib/active_support/testing/parallelization.rb
@@ -69,31 +69,29 @@ module ActiveSupport
       def start
         @pool = @queue_size.times.map do |worker|
           fork do
-            begin
-              DRb.stop_service
-
-              after_fork(worker)
-
-              queue = DRbObject.new_with_uri(@url)
-
-              while job = queue.pop
-                klass    = job[0]
-                method   = job[1]
-                reporter = job[2]
-                result   = Minitest.run_one_method(klass, method)
-
-                begin
-                  queue.record(reporter, result)
-                rescue DRb::DRbConnError
-                  result.failures.each do |failure|
-                    failure.exception = DRb::DRbRemoteError.new(failure.exception)
-                  end
-                  queue.record(reporter, result)
+            DRb.stop_service
+
+            after_fork(worker)
+
+            queue = DRbObject.new_with_uri(@url)
+
+            while job = queue.pop
+              klass    = job[0]
+              method   = job[1]
+              reporter = job[2]
+              result   = Minitest.run_one_method(klass, method)
+
+              begin
+                queue.record(reporter, result)
+              rescue DRb::DRbConnError
+                result.failures.each do |failure|
+                  failure.exception = DRb::DRbRemoteError.new(failure.exception)
                 end
+                queue.record(reporter, result)
               end
-            ensure
-              run_cleanup(worker)
             end
+          ensure
+            run_cleanup(worker)
           end
         end
       end
diff --git a/activesupport/test/cache/behaviors/connection_pool_behavior.rb b/activesupport/test/cache/behaviors/connection_pool_behavior.rb
index 4d1901a173..5e66e0b202 100644
--- a/activesupport/test/cache/behaviors/connection_pool_behavior.rb
+++ b/activesupport/test/cache/behaviors/connection_pool_behavior.rb
@@ -7,24 +7,22 @@ module ConnectionPoolBehavior
     threads = []
 
     emulating_latency do
-      begin
-        cache = ActiveSupport::Cache.lookup_store(store, { pool_size: 2, pool_timeout: 1 }.merge(store_options))
-        cache.clear
-
-        assert_raises Timeout::Error do
-          # One of the three threads will fail in 1 second because our pool size
-          # is only two.
-          3.times do
-            threads << Thread.new do
-              cache.read("latency")
-            end
+      cache = ActiveSupport::Cache.lookup_store(store, { pool_size: 2, pool_timeout: 1 }.merge(store_options))
+      cache.clear
+
+      assert_raises Timeout::Error do
+        # One of the three threads will fail in 1 second because our pool size
+        # is only two.
+        3.times do
+          threads << Thread.new do
+            cache.read("latency")
           end
-
-          threads.each(&:join)
         end
-      ensure
-        threads.each(&:kill)
+
+        threads.each(&:join)
       end
+    ensure
+      threads.each(&:kill)
     end
   ensure
     Thread.report_on_exception = original_report_on_exception
@@ -34,24 +32,22 @@ module ConnectionPoolBehavior
     threads = []
 
     emulating_latency do
-      begin
-        cache = ActiveSupport::Cache.lookup_store(store, store_options)
-        cache.clear
-
-        assert_nothing_raised do
-          # Default connection pool size is 5, assuming 10 will make sure that
-          # the connection pool isn't used at all.
-          10.times do
-            threads << Thread.new do
-              cache.read("latency")
-            end
+      cache = ActiveSupport::Cache.lookup_store(store, store_options)
+      cache.clear
+
+      assert_nothing_raised do
+        # Default connection pool size is 5, assuming 10 will make sure that
+        # the connection pool isn't used at all.
+        10.times do
+          threads << Thread.new do
+            cache.read("latency")
           end
-
-          threads.each(&:join)
         end
-      ensure
-        threads.each(&:kill)
+
+        threads.each(&:join)
       end
+    ensure
+      threads.each(&:kill)
     end
   end
 
diff --git a/activesupport/test/core_ext/load_error_test.rb b/activesupport/test/core_ext/load_error_test.rb
index 126aa51cb4..8afff4fb9c 100644
--- a/activesupport/test/core_ext/load_error_test.rb
+++ b/activesupport/test/core_ext/load_error_test.rb
@@ -13,10 +13,9 @@ class TestLoadError < ActiveSupport::TestCase
   end
 
   def test_path
-    begin load "nor/this/one.rb"
-    rescue LoadError => e
-      assert_equal "nor/this/one.rb", e.path
-    end
+    load "nor/this/one.rb"
+   rescue LoadError => e
+     assert_equal "nor/this/one.rb", e.path
   end
 
   def test_is_missing_with_nil_path
diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb
index 8062873386..7589ffd0ea 100644
--- a/activesupport/test/json/encoding_test.rb
+++ b/activesupport/test/json/encoding_test.rb
@@ -21,20 +21,18 @@ class TestJSONEncoding < ActiveSupport::TestCase
 
   JSONTest::EncodingTestCases.constants.each do |class_tests|
     define_method("test_#{class_tests[0..-6].underscore}") do
-      begin
-        prev = ActiveSupport.use_standard_json_time_format
-
-        standard_class_tests = /Standard/.match?(class_tests)
-
-        ActiveSupport.escape_html_entities_in_json  = !standard_class_tests
-        ActiveSupport.use_standard_json_time_format = standard_class_tests
-        JSONTest::EncodingTestCases.const_get(class_tests).each do |pair|
-          assert_equal pair.last, sorted_json(ActiveSupport::JSON.encode(pair.first))
-        end
-      ensure
-        ActiveSupport.escape_html_entities_in_json  = false
-        ActiveSupport.use_standard_json_time_format = prev
+      prev = ActiveSupport.use_standard_json_time_format
+
+      standard_class_tests = /Standard/.match?(class_tests)
+
+      ActiveSupport.escape_html_entities_in_json  = !standard_class_tests
+      ActiveSupport.use_standard_json_time_format = standard_class_tests
+      JSONTest::EncodingTestCases.const_get(class_tests).each do |pair|
+        assert_equal pair.last, sorted_json(ActiveSupport::JSON.encode(pair.first))
       end
+    ensure
+      ActiveSupport.escape_html_entities_in_json  = false
+      ActiveSupport.use_standard_json_time_format = prev
     end
   end
 
diff --git a/activesupport/test/share_lock_test.rb b/activesupport/test/share_lock_test.rb
index 34479020e1..30a1ddad3f 100644
--- a/activesupport/test/share_lock_test.rb
+++ b/activesupport/test/share_lock_test.rb
@@ -115,68 +115,66 @@ class ShareLockTest < ActiveSupport::TestCase
   def test_exclusive_conflicting_purpose
     [true, false].each do |use_upgrading|
       with_thread_waiting_in_lock_section(:sharing) do |sharing_thread_release_latch|
-        begin
-          together = Concurrent::CyclicBarrier.new(2)
-          conflicting_exclusive_threads = [
-            Thread.new do
-              @lock.send(use_upgrading ? :sharing : :tap) do
-                together.wait
-                @lock.exclusive(purpose: :red, compatible: [:green, :purple]) { }
-              end
-            end,
-            Thread.new do
-              @lock.send(use_upgrading ? :sharing : :tap) do
-                together.wait
-                @lock.exclusive(purpose: :blue, compatible: [:green]) { }
-              end
+        together = Concurrent::CyclicBarrier.new(2)
+        conflicting_exclusive_threads = [
+          Thread.new do
+            @lock.send(use_upgrading ? :sharing : :tap) do
+              together.wait
+              @lock.exclusive(purpose: :red, compatible: [:green, :purple]) { }
+            end
+          end,
+          Thread.new do
+            @lock.send(use_upgrading ? :sharing : :tap) do
+              together.wait
+              @lock.exclusive(purpose: :blue, compatible: [:green]) { }
             end
-          ]
-
-          assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks
-
-          # This thread will be stuck as long as any other thread is in
-          # a sharing block. While it's blocked, it holds no lock, so it
-          # doesn't interfere with any other attempts.
-          no_purpose_thread = Thread.new do
-            @lock.exclusive { }
           end
-          assert_threads_stuck no_purpose_thread
+        ]
 
-          # This thread is compatible with both of the "primary"
-          # attempts above. It's initially stuck on the outer share
-          # lock, but as soon as that's released, it can run --
-          # regardless of whether those threads hold share locks.
-          compatible_thread = Thread.new do
-            @lock.exclusive(purpose: :green, compatible: []) { }
-          end
-          assert_threads_stuck compatible_thread
+        assert_threads_stuck conflicting_exclusive_threads # wait for threads to get into their respective `exclusive {}` blocks
 
-          assert_threads_stuck conflicting_exclusive_threads
+        # This thread will be stuck as long as any other thread is in
+        # a sharing block. While it's blocked, it holds no lock, so it
+        # doesn't interfere with any other attempts.
+        no_purpose_thread = Thread.new do
+          @lock.exclusive { }
+        end
+        assert_threads_stuck no_purpose_thread
+
+        # This thread is compatible with both of the "primary"
+        # attempts above. It's initially stuck on the outer share
+        # lock, but as soon as that's released, it can run --
+        # regardless of whether those threads hold share locks.
+        compatible_thread = Thread.new do
+          @lock.exclusive(purpose: :green, compatible: []) { }
+        end
+        assert_threads_stuck compatible_thread
 
-          sharing_thread_release_latch.count_down
+        assert_threads_stuck conflicting_exclusive_threads
 
-          assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through
+        sharing_thread_release_latch.count_down
 
-          if use_upgrading
-            # The "primary" threads both each hold a share lock, and are
-            # mutually incompatible; they're still stuck.
-            assert_threads_stuck conflicting_exclusive_threads
+        assert_threads_not_stuck compatible_thread # compatible thread is now able to squeak through
 
-            # The thread without a specified purpose is also stuck; it's
-            # not compatible with anything.
-            assert_threads_stuck no_purpose_thread
-          else
-            # As the primaries didn't hold a share lock, as soon as the
-            # outer one was released, all the exclusive locks are free
-            # to be acquired in turn.
+        if use_upgrading
+          # The "primary" threads both each hold a share lock, and are
+          # mutually incompatible; they're still stuck.
+          assert_threads_stuck conflicting_exclusive_threads
 
-            assert_threads_not_stuck conflicting_exclusive_threads
-            assert_threads_not_stuck no_purpose_thread
-          end
-        ensure
-          conflicting_exclusive_threads.each(&:kill)
-          no_purpose_thread.kill
+          # The thread without a specified purpose is also stuck; it's
+          # not compatible with anything.
+          assert_threads_stuck no_purpose_thread
+        else
+          # As the primaries didn't hold a share lock, as soon as the
+          # outer one was released, all the exclusive locks are free
+          # to be acquired in turn.
+
+          assert_threads_not_stuck conflicting_exclusive_threads
+          assert_threads_not_stuck no_purpose_thread
         end
+      ensure
+        conflicting_exclusive_threads.each(&:kill)
+        no_purpose_thread.kill
       end
     end
   end
diff --git a/activesupport/test/time_travel_test.rb b/activesupport/test/time_travel_test.rb
index fbd33a6036..9c61ab0ab5 100644
--- a/activesupport/test/time_travel_test.rb
+++ b/activesupport/test/time_travel_test.rb
@@ -14,16 +14,14 @@ class TimeTravelTest < ActiveSupport::TestCase
 
   def test_time_helper_travel
     Time.stub(:now, Time.now) do
-      begin
-        expected_time = Time.now + 1.day
-        travel 1.day
+      expected_time = Time.now + 1.day
+      travel 1.day
 
-        assert_equal expected_time.to_s(:db), Time.now.to_s(:db)
-        assert_equal expected_time.to_date, Date.today
-        assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db)
-      ensure
-        travel_back
-      end
+      assert_equal expected_time.to_s(:db), Time.now.to_s(:db)
+      assert_equal expected_time.to_date, Date.today
+      assert_equal expected_time.to_datetime.to_s(:db), DateTime.now.to_s(:db)
+    ensure
+      travel_back
     end
   end
 
@@ -45,16 +43,14 @@ class TimeTravelTest < ActiveSupport::TestCase
 
   def test_time_helper_travel_to
     Time.stub(:now, Time.now) do
-      begin
-        expected_time = Time.new(2004, 11, 24, 01, 04, 44)
-        travel_to expected_time
+      expected_time = Time.new(2004, 11, 24, 01, 04, 44)
+      travel_to expected_time
 
-        assert_equal expected_time, Time.now
-        assert_equal Date.new(2004, 11, 24), Date.today
-        assert_equal expected_time.to_datetime, DateTime.now
-      ensure
-        travel_back
-      end
+      assert_equal expected_time, Time.now
+      assert_equal Date.new(2004, 11, 24), Date.today
+      assert_equal expected_time.to_datetime, DateTime.now
+    ensure
+      travel_back
     end
   end
 
@@ -90,21 +86,19 @@ class TimeTravelTest < ActiveSupport::TestCase
 
   def test_time_helper_travel_back
     Time.stub(:now, Time.now) do
-      begin
-        expected_time = Time.new(2004, 11, 24, 01, 04, 44)
+      expected_time = Time.new(2004, 11, 24, 01, 04, 44)
 
-        travel_to expected_time
-        assert_equal expected_time, Time.now
-        assert_equal Date.new(2004, 11, 24), Date.today
-        assert_equal expected_time.to_datetime, DateTime.now
-        travel_back
+      travel_to expected_time
+      assert_equal expected_time, Time.now
+      assert_equal Date.new(2004, 11, 24), Date.today
+      assert_equal expected_time.to_datetime, DateTime.now
+      travel_back
 
-        assert_not_equal expected_time, Time.now
-        assert_not_equal Date.new(2004, 11, 24), Date.today
-        assert_not_equal expected_time.to_datetime, DateTime.now
-      ensure
-        travel_back
-      end
+      assert_not_equal expected_time, Time.now
+      assert_not_equal Date.new(2004, 11, 24), Date.today
+      assert_not_equal expected_time.to_datetime, DateTime.now
+    ensure
+      travel_back
     end
   end
 
@@ -139,20 +133,18 @@ class TimeTravelTest < ActiveSupport::TestCase
 
   def test_time_helper_travel_to_with_subsequent_calls
     Time.stub(:now, Time.now) do
-      begin
-        initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44)
-        subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44)
-        assert_nothing_raised do
-          travel_to initial_expected_time
-          travel_to subsequent_expected_time
+      initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44)
+      subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44)
+      assert_nothing_raised do
+        travel_to initial_expected_time
+        travel_to subsequent_expected_time
 
-          assert_equal subsequent_expected_time, Time.now
+        assert_equal subsequent_expected_time, Time.now
 
-          travel_back
-        end
-      ensure
         travel_back
       end
+    ensure
+      travel_back
     end
   end