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.

5 files changed, 126 insertions, 148 deletions

diff --git a/activestorage/test/service/azure_storage_service_test.rb b/activestorage/test/service/azure_storage_service_test.rb
index 09c2e7f99c..2b07902d07 100644
--- a/activestorage/test/service/azure_storage_service_test.rb
+++ b/activestorage/test/service/azure_storage_service_test.rb
@@ -18,20 +18,18 @@ if SERVICE_CONFIGURATIONS[:azure]
     end
 
     test "uploading a tempfile" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-
-        Tempfile.open do |file|
-          file.write(data)
-          file.rewind
-          @service.upload(key, file)
-        end
-
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete(key)
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+
+      Tempfile.open do |file|
+        file.write(data)
+        file.rewind
+        @service.upload(key, file)
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete(key)
     end
   end
 else
diff --git a/activestorage/test/service/gcs_service_test.rb b/activestorage/test/service/gcs_service_test.rb
index af27946357..6bca428f50 100644
--- a/activestorage/test/service/gcs_service_test.rb
+++ b/activestorage/test/service/gcs_service_test.rb
@@ -10,74 +10,66 @@ if SERVICE_CONFIGURATIONS[:gcs]
     include ActiveStorage::Service::SharedServiceTests
 
     test "direct upload" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-        checksum = Digest::MD5.base64digest(data)
-        url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-        uri = URI.parse url
-        request = Net::HTTP::Put.new uri.request_uri
-        request.body = data
-        request.add_field "Content-Type", ""
-        request.add_field "Content-MD5", checksum
-        Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-          http.request request
-        end
-
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      checksum = Digest::MD5.base64digest(data)
+      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
+
+      uri = URI.parse url
+      request = Net::HTTP::Put.new uri.request_uri
+      request.body = data
+      request.add_field "Content-Type", ""
+      request.add_field "Content-MD5", checksum
+      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+        http.request request
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "upload with content_type and content_disposition" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
-
-        url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
-        response = Net::HTTP.get_response(URI(url))
-        assert_equal "text/plain", response.content_type
-        assert_match(/attachment;.*test.txt/, response["Content-Disposition"])
-      ensure
-        @service.delete key
-      end
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
+
+      url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
+      response = Net::HTTP.get_response(URI(url))
+      assert_equal "text/plain", response.content_type
+      assert_match(/attachment;.*test.txt/, response["Content-Disposition"])
+    ensure
+      @service.delete key
     end
 
     test "upload with content_type" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain")
-
-        url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
-        response = Net::HTTP.get_response(URI(url))
-        assert_equal "text/plain", response.content_type
-        assert_match(/inline;.*test.html/, response["Content-Disposition"])
-      ensure
-        @service.delete key
-      end
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), content_type: "text/plain")
+
+      url = @service.url(key, expires_in: 2.minutes, disposition: :inline, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
+      response = Net::HTTP.get_response(URI(url))
+      assert_equal "text/plain", response.content_type
+      assert_match(/inline;.*test.html/, response["Content-Disposition"])
+    ensure
+      @service.delete key
     end
 
     test "update metadata" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html")
-
-        @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
-        url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
-
-        response = Net::HTTP.get_response(URI(url))
-        assert_equal "text/plain", response.content_type
-        assert_match(/inline;.*test.txt/, response["Content-Disposition"])
-      ensure
-        @service.delete key
-      end
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data), disposition: :attachment, filename: ActiveStorage::Filename.new("test.html"), content_type: "text/html")
+
+      @service.update_metadata(key, disposition: :inline, filename: ActiveStorage::Filename.new("test.txt"), content_type: "text/plain")
+      url = @service.url(key, expires_in: 2.minutes, disposition: :attachment, content_type: "text/html", filename: ActiveStorage::Filename.new("test.html"))
+
+      response = Net::HTTP.get_response(URI(url))
+      assert_equal "text/plain", response.content_type
+      assert_match(/inline;.*test.txt/, response["Content-Disposition"])
+    ensure
+      @service.delete key
     end
 
     test "signed URL generation" do
diff --git a/activestorage/test/service/mirror_service_test.rb b/activestorage/test/service/mirror_service_test.rb
index bb502dde60..94c751a4ff 100644
--- a/activestorage/test/service/mirror_service_test.rb
+++ b/activestorage/test/service/mirror_service_test.rb
@@ -18,22 +18,20 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
   include ActiveStorage::Service::SharedServiceTests
 
   test "uploading to all services" do
-    begin
-      key      = SecureRandom.base58(24)
-      data     = "Something else entirely!"
-      io       = StringIO.new(data)
-      checksum = Digest::MD5.base64digest(data)
+    key      = SecureRandom.base58(24)
+    data     = "Something else entirely!"
+    io       = StringIO.new(data)
+    checksum = Digest::MD5.base64digest(data)
 
-      @service.upload key, io.tap(&:read), checksum: checksum
-      assert_predicate io, :eof?
+    @service.upload key, io.tap(&:read), checksum: checksum
+    assert_predicate io, :eof?
 
-      assert_equal data, @service.primary.download(key)
-      @service.mirrors.each do |mirror|
-        assert_equal data, mirror.download(key)
-      end
-    ensure
-      @service.delete key
+    assert_equal data, @service.primary.download(key)
+    @service.mirrors.each do |mirror|
+      assert_equal data, mirror.download(key)
     end
+  ensure
+    @service.delete key
   end
 
   test "downloading from primary service" do
diff --git a/activestorage/test/service/s3_service_test.rb b/activestorage/test/service/s3_service_test.rb
index 0e5b06dd78..0a6004267f 100644
--- a/activestorage/test/service/s3_service_test.rb
+++ b/activestorage/test/service/s3_service_test.rb
@@ -11,25 +11,23 @@ if SERVICE_CONFIGURATIONS[:s3]
     include ActiveStorage::Service::SharedServiceTests
 
     test "direct upload" do
-      begin
-        key      = SecureRandom.base58(24)
-        data     = "Something else entirely!"
-        checksum = Digest::MD5.base64digest(data)
-        url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
-
-        uri = URI.parse url
-        request = Net::HTTP::Put.new uri.request_uri
-        request.body = data
-        request.add_field "Content-Type", "text/plain"
-        request.add_field "Content-MD5", checksum
-        Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
-          http.request request
-        end
+      key      = SecureRandom.base58(24)
+      data     = "Something else entirely!"
+      checksum = Digest::MD5.base64digest(data)
+      url      = @service.url_for_direct_upload(key, expires_in: 5.minutes, content_type: "text/plain", content_length: data.size, checksum: checksum)
 
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
+      uri = URI.parse url
+      request = Net::HTTP::Put.new uri.request_uri
+      request.body = data
+      request.add_field "Content-Type", "text/plain"
+      request.add_field "Content-MD5", checksum
+      Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http|
+        http.request request
       end
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "upload a zero byte file" do
diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb
index b97030b320..17f3736056 100644
--- a/activestorage/test/service/shared_service_tests.rb
+++ b/activestorage/test/service/shared_service_tests.rb
@@ -20,48 +20,42 @@ module ActiveStorage::Service::SharedServiceTests
     end
 
     test "uploading with integrity" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data))
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+      @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest(data))
 
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
-      end
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "uploading without integrity" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
 
-        assert_raises(ActiveStorage::IntegrityError) do
-          @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data"))
-        end
-
-        assert_not @service.exist?(key)
-      ensure
-        @service.delete key
+      assert_raises(ActiveStorage::IntegrityError) do
+        @service.upload(key, StringIO.new(data), checksum: Digest::MD5.base64digest("bad data"))
       end
+
+      assert_not @service.exist?(key)
+    ensure
+      @service.delete key
     end
 
     test "uploading with integrity and multiple keys" do
-      begin
-        key  = SecureRandom.base58(24)
-        data = "Something else entirely!"
-        @service.upload(
-          key,
-          StringIO.new(data),
-          checksum: Digest::MD5.base64digest(data),
-          filename: "racecar.jpg",
-          content_type: "image/jpg"
-        )
-
-        assert_equal data, @service.download(key)
-      ensure
-        @service.delete key
-      end
+      key  = SecureRandom.base58(24)
+      data = "Something else entirely!"
+      @service.upload(
+        key,
+        StringIO.new(data),
+        checksum: Digest::MD5.base64digest(data),
+        filename: "racecar.jpg",
+        content_type: "image/jpg"
+      )
+
+      assert_equal data, @service.download(key)
+    ensure
+      @service.delete key
     end
 
     test "downloading" do
@@ -129,20 +123,18 @@ module ActiveStorage::Service::SharedServiceTests
     end
 
     test "deleting by prefix" do
-      begin
-        @service.upload("a/a/a", StringIO.new(FIXTURE_DATA))
-        @service.upload("a/a/b", StringIO.new(FIXTURE_DATA))
-        @service.upload("a/b/a", StringIO.new(FIXTURE_DATA))
-
-        @service.delete_prefixed("a/a/")
-        assert_not @service.exist?("a/a/a")
-        assert_not @service.exist?("a/a/b")
-        assert @service.exist?("a/b/a")
-      ensure
-        @service.delete("a/a/a")
-        @service.delete("a/a/b")
-        @service.delete("a/b/a")
-      end
+      @service.upload("a/a/a", StringIO.new(FIXTURE_DATA))
+      @service.upload("a/a/b", StringIO.new(FIXTURE_DATA))
+      @service.upload("a/b/a", StringIO.new(FIXTURE_DATA))
+
+      @service.delete_prefixed("a/a/")
+      assert_not @service.exist?("a/a/a")
+      assert_not @service.exist?("a/a/b")
+      assert @service.exist?("a/b/a")
+    ensure
+      @service.delete("a/a/a")
+      @service.delete("a/a/b")
+      @service.delete("a/b/a")
     end
   end
 end