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

Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).

I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.

6 files changed, 49 insertions, 63 deletions

diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb
index b6f0e18764..929045f29b 100644
--- a/activerecord/lib/active_record/attribute_assignment.rb
+++ b/activerecord/lib/active_record/attribute_assignment.rb
@@ -45,16 +45,14 @@ module ActiveRecord
       def execute_callstack_for_multiparameter_attributes(callstack)
         errors = []
         callstack.each do |name, values_with_empty_parameters|
-          begin
-            if values_with_empty_parameters.each_value.all?(&:nil?)
-              values = nil
-            else
-              values = values_with_empty_parameters
-            end
-            send("#{name}=", values)
-          rescue => ex
-            errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name)
+          if values_with_empty_parameters.each_value.all?(&:nil?)
+            values = nil
+          else
+            values = values_with_empty_parameters
           end
+          send("#{name}=", values)
+        rescue => ex
+          errors << AttributeAssignmentError.new("error on assignment #{values_with_empty_parameters.values.inspect} to #{name} (#{ex.message})", ex, name)
         end
         unless errors.empty?
           error_descriptions = errors.map(&:message).join(",")
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 0f2b1e85ff..718910b090 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -283,26 +283,24 @@ module ActiveRecord
 
       def within_new_transaction(options = {})
         @connection.lock.synchronize do
-          begin
-            transaction = begin_transaction options
-            yield
-          rescue Exception => error
-            if transaction
+          transaction = begin_transaction options
+          yield
+        rescue Exception => error
+          if transaction
+            rollback_transaction
+            after_failure_actions(transaction, error)
+          end
+          raise
+        ensure
+          if !error && transaction
+            if Thread.current.status == "aborting"
               rollback_transaction
-              after_failure_actions(transaction, error)
-            end
-            raise
-          ensure
-            unless error
-              if Thread.current.status == "aborting"
-                rollback_transaction if transaction
-              else
-                begin
-                  commit_transaction if transaction
-                rescue Exception
-                  rollback_transaction(transaction) unless transaction.state.completed?
-                  raise
-                end
+            else
+              begin
+                commit_transaction
+              rescue Exception
+                rollback_transaction(transaction) unless transaction.state.completed?
+                raise
               end
             end
           end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index f7d75e6748..1243236c09 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -631,13 +631,11 @@ module ActiveRecord
             statement_name:    statement_name,
             connection_id:     object_id,
             connection:        self) do
-            begin
-              @lock.synchronize do
-                yield
-              end
-            rescue => e
-              raise translate_exception_class(e, sql, binds)
+            @lock.synchronize do
+              yield
             end
+          rescue => e
+            raise translate_exception_class(e, sql, binds)
           end
         end
 
diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb
index 30cb0a27e7..11aed6c002 100644
--- a/activerecord/lib/active_record/database_configurations.rb
+++ b/activerecord/lib/active_record/database_configurations.rb
@@ -124,15 +124,13 @@ module ActiveRecord
       end
 
       def build_db_config_from_string(env_name, spec_name, config)
-        begin
-          url = config
-          uri = URI.parse(url)
-          if uri.try(:scheme)
-            ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url)
-          end
-        rescue URI::InvalidURIError
-          ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config)
+        url = config
+        uri = URI.parse(url)
+        if uri.try(:scheme)
+          ActiveRecord::DatabaseConfigurations::UrlConfig.new(env_name, spec_name, url)
         end
+      rescue URI::InvalidURIError
+        ActiveRecord::DatabaseConfigurations::HashConfig.new(env_name, spec_name, config)
       end
 
       def build_db_config_from_hash(env_name, spec_name, config)
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index 350b044a3e..327121a2a2 100644
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -519,11 +519,9 @@ module ActiveRecord
       def instantiate_fixtures(object, fixture_set, load_instances = true)
         return unless load_instances
         fixture_set.each do |fixture_name, fixture|
-          begin
-            object.instance_variable_set "@#{fixture_name}", fixture.find
-          rescue FixtureClassNotFound
-            nil
-          end
+          object.instance_variable_set "@#{fixture_name}", fixture.find
+        rescue FixtureClassNotFound
+          nil
         end
       end
 
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 475baa7559..d24324ecce 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -191,11 +191,9 @@ db_namespace = namespace :db do
 
   # desc "Retrieves the collation for the current environment's database"
   task collation: :load_config do
-    begin
-      puts ActiveRecord::Tasks::DatabaseTasks.collation_current
-    rescue NoMethodError
-      $stderr.puts "Sorry, your database adapter is not supported yet. Feel free to submit a patch."
-    end
+    puts ActiveRecord::Tasks::DatabaseTasks.collation_current
+  rescue NoMethodError
+    $stderr.puts "Sorry, your database adapter is not supported yet. Feel free to submit a patch."
   end
 
   desc "Retrieves the current schema version number"
@@ -361,17 +359,15 @@ db_namespace = namespace :db do
 
     # desc "Recreate the test database from an existent schema.rb file"
     task load_schema: %w(db:test:purge) do
-      begin
-        should_reconnect = ActiveRecord::Base.connection_pool.active_connection?
-        ActiveRecord::Schema.verbose = false
-        ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config|
-          filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby)
-          ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :ruby, filename, "test")
-        end
-      ensure
-        if should_reconnect
-          ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations.default_hash(ActiveRecord::Tasks::DatabaseTasks.env))
-        end
+      should_reconnect = ActiveRecord::Base.connection_pool.active_connection?
+      ActiveRecord::Schema.verbose = false
+      ActiveRecord::Base.configurations.configs_for(env_name: "test").each do |db_config|
+        filename = ActiveRecord::Tasks::DatabaseTasks.dump_filename(db_config.spec_name, :ruby)
+        ActiveRecord::Tasks::DatabaseTasks.load_schema(db_config.config, :ruby, filename, "test")
+      end
+    ensure
+      if should_reconnect
+        ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations.default_hash(ActiveRecord::Tasks::DatabaseTasks.env))
       end
     end