(PUP-11993) Style/RescueModifier

AriaXLi · AriaXLi · commit 17358f58ab41 · 2024-03-04T15:34:08.000-08:00
This commit enables the Style/RescueModifier cop and fixes 143
autocorrectable offenses.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -679,23 +679,6 @@ Style/RedundantSelfAssignment:
   Exclude:
     - 'lib/puppet/context.rb'
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/RescueModifier:
-  Exclude:
-    - 'lib/puppet/file_serving/metadata.rb'
-    - 'lib/puppet/file_system/uniquefile.rb'
-    - 'lib/puppet/file_system/windows.rb'
-    - 'lib/puppet/module_tool/applications/application.rb'
-    - 'lib/puppet/module_tool/applications/installer.rb'
-    - 'lib/puppet/module_tool/applications/upgrader.rb'
-    - 'lib/puppet/module_tool/metadata.rb'
-    - 'lib/puppet/module_tool/shared_behaviors.rb'
-    - 'lib/puppet/provider/file/posix.rb'
-    - 'lib/puppet/util.rb'
-    - 'lib/puppet/util/execution.rb'
-    - 'lib/puppet/util/reference.rb'
-    - 'lib/puppet/util/windows/adsi.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: implicit, explicit
diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb
@@ -117,7 +117,11 @@ def collect(source_permissions = nil)
       @checksum = "{#{@checksum_type}}" + send("#{@checksum_type}_file", path).to_s
     when "link"
       @destination = Puppet::FileSystem.readlink(real_path)
-      @checksum = "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s rescue nil
+      @checksum = begin
+        "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s
+      rescue
+        nil
+      end
     when "fifo", "socket"
       @checksum_type = "none"
       @checksum = "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s
diff --git a/lib/puppet/file_system/uniquefile.rb b/lib/puppet/file_system/uniquefile.rb
@@ -151,10 +151,14 @@ def tmpdir
     tmp = '.'
     [ENV.fetch('TMPDIR', nil), ENV.fetch('TMP', nil), ENV.fetch('TEMP', nil), @@systmpdir, '/tmp'].each do |dir|
       stat = File.stat(dir) if dir
-      if stat && stat.directory? && stat.writable?
-        tmp = dir
-        break
-      end rescue nil
+      begin
+        if stat && stat.directory? && stat.writable?
+          tmp = dir
+          break
+        end
+      rescue
+        nil
+      end
     end
     File.expand_path(tmp)
   end
diff --git a/lib/puppet/file_system/windows.rb b/lib/puppet/file_system/windows.rb
@@ -78,11 +78,19 @@ def unlink(*file_names)
 
     file_names.each do |file_name|
       file_name = file_name.to_s # handle PathName
-      stat = Puppet::Util::Windows::File.stat(file_name) rescue nil
+      stat = begin
+        Puppet::Util::Windows::File.stat(file_name)
+      rescue
+        nil
+      end
 
       # sigh, Ruby + Windows :(
       if !stat
-        ::File.unlink(file_name) rescue Dir.rmdir(file_name)
+        begin
+          ::File.unlink(file_name)
+        rescue
+          Dir.rmdir(file_name)
+        end
       elsif stat.ftype == 'directory'
         if Puppet::Util::Windows::File.symlink?(file_name)
           Dir.rmdir(file_name)
diff --git a/lib/puppet/module_tool/applications/application.rb b/lib/puppet/module_tool/applications/application.rb
@@ -28,7 +28,11 @@ def discuss(response, success, failure)
         when Net::HTTPOK, Net::HTTPCreated
           Puppet.notice success
         else
-          errors = Puppet::Util::Json.load(response.body)['error'] rescue "HTTP #{response.code}, #{response.body}"
+          errors = begin
+            Puppet::Util::Json.load(response.body)['error']
+          rescue
+            "HTTP #{response.code}, #{response.body}"
+          end
           Puppet.warning "#{failure} (#{errors})"
         end
       end
diff --git a/lib/puppet/module_tool/applications/installer.rb b/lib/puppet/module_tool/applications/installer.rb
@@ -72,7 +72,11 @@ def run
                 results[:version] = installed_module.version
                 return results
               else
-                changes = Checksummer.run(installed_modules[name].mod.path) rescue []
+                changes = begin
+                  Checksummer.run(installed_modules[name].mod.path)
+                rescue
+                  []
+                end
                 raise AlreadyInstalledError,
                       :module_name => name,
                       :installed_version => installed_modules[name].version,
diff --git a/lib/puppet/module_tool/applications/upgrader.rb b/lib/puppet/module_tool/applications/upgrader.rb
@@ -76,7 +76,11 @@ def installed_release.priority
           vstring = mod.version ? "v#{mod.version}" : '???'
           Puppet.notice _("Found '%{name}' (%{version}) in %{dir} ...") % { name: name, version: colorize(:cyan, vstring), dir: dir }
           unless @ignore_changes
-            changes = Checksummer.run(mod.path) rescue []
+            changes = begin
+              Checksummer.run(mod.path)
+            rescue
+              []
+            end
             if mod.has_metadata? && !changes.empty?
               raise LocalChangesError,
                     :action => :upgrade,
diff --git a/lib/puppet/module_tool/metadata.rb b/lib/puppet/module_tool/metadata.rb
@@ -102,7 +102,11 @@ def to_json
       data = @data.dup.merge('dependencies' => dependencies)
 
       contents = data.keys.map do |k|
-        value = (Puppet::Util::Json.dump(data[k], :pretty => true) rescue data[k].to_json)
+        value = begin
+          Puppet::Util::Json.dump(data[k], :pretty => true)
+        rescue
+          data[k].to_json
+        end
         %Q("#{k}": #{value})
       end
 
diff --git a/lib/puppet/module_tool/shared_behaviors.rb b/lib/puppet/module_tool/shared_behaviors.rb
@@ -38,7 +38,11 @@ def get_remote_constraints(forge)
       mod_name, releases = pair
       mod_name = mod_name.tr('/', '-')
       releases.each do |rel|
-        semver = SemanticPuppet::Version.parse(rel['version']) rescue SemanticPuppet::Version::MIN
+        semver = begin
+          SemanticPuppet::Version.parse(rel['version'])
+        rescue
+          SemanticPuppet::Version::MIN
+        end
         @versions[mod_name] << { :vstring => rel['version'], :semver => semver }
         @versions[mod_name].sort_by! { |a| a[:semver] }
         @urls["#{mod_name}@#{rel['version']}"] = rel['file']
@@ -79,10 +83,16 @@ def resolve_constraints(dependencies, source = [{ :name => :you }], seen = {}, a
       }
 
       if forced?
-        range = Puppet::Module.parse_range(@version) rescue Puppet::Module.parse_range('>= 0.0.0')
+        range = begin
+          Puppet::Module.parse_range(@version)
+        rescue
+          Puppet::Module.parse_range('>= 0.0.0')
+        end
       else
         range = (@conditions[mod]).map do |r|
-          Puppet::Module.parse_range(r[:dependency]) rescue Puppet::Module.parse_range('>= 0.0.0')
+          Puppet::Module.parse_range(r[:dependency])
+        rescue
+          Puppet::Module.parse_range('>= 0.0.0')
         end.inject(&:&)
       end
 
diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb
@@ -35,7 +35,9 @@ def uid2name(id)
 
   # Determine if the user is valid, and if so, return the UID
   def name2uid(value)
-    Integer(value) rescue uid(value) || false
+    Integer(value)
+  rescue
+    uid(value) || false
   end
 
   def gid2name(id)
@@ -56,7 +58,9 @@ def gid2name(id)
   end
 
   def name2gid(value)
-    Integer(value) rescue gid(value) || false
+    Integer(value)
+  rescue
+    gid(value) || false
   end
 
   def owner
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
@@ -480,11 +480,21 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block)
       begin
         Dir.foreach('/proc/self/fd') do |f|
           if f != '.' && f != '..' && f.to_i >= 3
-            IO.new(f.to_i).close rescue nil
+            begin
+              IO.new(f.to_i).close
+            rescue
+              nil
+            end
           end
         end
       rescue Errno::ENOENT, Errno::ENOTDIR # /proc/self/fd not found, /proc/self not a dir
-        3.upto(256) { |fd| IO.new(fd).close rescue nil }
+        3.upto(256) { |fd|
+          begin
+            IO.new(fd).close
+          rescue
+            nil
+          end
+        }
       end
 
       block.call if block
@@ -540,7 +550,11 @@ def self.format_backtrace_array(primary_stack, puppetstack = [])
   def self.resolve_stackframe(frame)
     _, path, rest = /^(.*):(\d+.*)$/.match(frame).to_a
     if path
-      path = Pathname(path).realpath rescue path
+      path = begin
+        Pathname(path).realpath
+      rescue
+        path
+      end
       "#{path}:#{rest}"
     else
       frame
diff --git a/lib/puppet/util/execution.rb b/lib/puppet/util/execution.rb
@@ -212,13 +212,25 @@ def self.execute(command, options = NoOptionsSpecified)
       execution_stub = Puppet::Util::ExecutionStub.current_value
       if execution_stub
         child_pid = execution_stub.call(*exec_args)
-        [stdin, stdout, stderr].each { |io| io.close rescue nil }
+        [stdin, stdout, stderr].each { |io|
+          begin
+            io.close
+          rescue
+            nil
+          end
+        }
         return child_pid
       elsif Puppet.features.posix?
         child_pid = nil
         begin
           child_pid = execute_posix(*exec_args)
-          [stdin, stdout, stderr].each { |io| io.close rescue nil }
+          [stdin, stdout, stderr].each { |io|
+            begin
+              io.close
+            rescue
+              nil
+            end
+          }
           if options[:squelch]
             exit_status = Process.waitpid2(child_pid).last.exitstatus
           else
@@ -276,7 +288,13 @@ def self.execute(command, options = NoOptionsSpecified)
       elsif Puppet::Util::Platform.windows?
         process_info = execute_windows(*exec_args)
         begin
-          [stdin, stderr].each { |io| io.close rescue nil }
+          [stdin, stderr].each { |io|
+            begin
+              io.close
+            rescue
+              nil
+            end
+          }
           exit_status = Puppet::Util::Windows::Process.wait_process(process_info.process_handle)
 
           # read output in if required
@@ -295,7 +313,13 @@ def self.execute(command, options = NoOptionsSpecified)
       end
     ensure
       # Make sure all handles are closed in case an exception was thrown attempting to execute.
-      [stdin, stdout, stderr].each { |io| io.close rescue nil }
+      [stdin, stdout, stderr].each { |io|
+        begin
+          io.close
+        rescue
+          nil
+        end
+      }
       unless options[:squelch]
         # if we opened a pipe, we need to clean it up.
         reader.close if reader
diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb
@@ -42,7 +42,11 @@ def self.pdf(text)
     # There used to be an attempt to use secure_open / replace_file to secure
     # the target, too, but that did nothing: the race was still here.  We can
     # get exactly the same benefit from running this effort:
-    Puppet::FileSystem.unlink('/tmp/puppetdoc.tex') rescue nil
+    begin
+      Puppet::FileSystem.unlink('/tmp/puppetdoc.tex')
+    rescue
+      nil
+    end
     output = %x(#{cmd})
     unless $CHILD_STATUS == 0
       $stderr.puts _("rst2latex failed")
diff --git a/lib/puppet/util/windows/adsi.rb b/lib/puppet/util/windows/adsi.rb
@@ -328,7 +328,11 @@ def password_is?(password)
     end
 
     def add_flag(flag_name, value)
-      flag = native_object.Get(flag_name) rescue 0
+      flag = begin
+        native_object.Get(flag_name)
+      rescue
+        0
+      end
 
       native_object.Put(flag_name, flag | value)
 
@@ -350,7 +354,11 @@ def groups
       # WIN32OLE objects aren't enumerable, so no map
       groups = []
       # Setting WIN32OLE.codepage ensures values are returned as UTF-8
-      native_object.Groups.each { |g| groups << g.Name } rescue nil
+      begin
+        native_object.Groups.each { |g| groups << g.Name }
+      rescue
+        nil
+      end
       groups
     end
 
