(PUP-11993) Style/RedundantParentheses

AriaXLi · AriaXLi · commit 1899f97809b3 · 2024-03-01T13:29:56.000-08:00
This commit enables the Style/RedundantParentheses cop and fixes 50
autocorrectable offenses.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -668,10 +668,6 @@ Style/RedundantFetchBlock:
 Style/RedundantInterpolation:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/RedundantParentheses:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 Style/RedundantPercentQ:
   Exclude:
diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
@@ -509,7 +509,7 @@ def enable_disable_client(agent)
   end
 
   def setup_agent
-    agent = Puppet::Agent.new(Puppet::Configurer, (!(Puppet[:onetime])))
+    agent = Puppet::Agent.new(Puppet::Configurer, !(Puppet[:onetime]))
 
     enable_disable_client(agent) if options[:enable] or options[:disable]
 
diff --git a/lib/puppet/application/describe.rb b/lib/puppet/application/describe.rb
@@ -11,7 +11,7 @@ def wrap(txt, opts)
     return "" unless txt && !txt.empty?
 
     work = (opts[:scrub] ? scrub(txt) : txt)
-    indent = (opts[:indent] || 0)
+    indent = opts[:indent] || 0
     textLen = @width - indent
     patt = Regexp.new("\\A(.{0,#{textLen}})[ \n]")
     prefix = " " * indent
diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
@@ -205,7 +205,7 @@ def setup
     @arguments << options
 
     # If we don't have a rendering format, set one early.
-    self.render_as ||= (@action.render_as || :console)
+    self.render_as ||= @action.render_as || :console
   end
 
   def main
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
@@ -489,7 +489,7 @@ def self.initialize_default_settings!(settings)
     },
     :daemonize => {
         :type     => :boolean,
-        :default  => (!Puppet::Util::Platform.windows?),
+        :default  => !Puppet::Util::Platform.windows?,
         :desc     => "Whether to send the process into the background.  This defaults
           to true on POSIX systems, and to false on Windows (where Puppet
           currently cannot daemonize).",
diff --git a/lib/puppet/face/help.rb b/lib/puppet/face/help.rb
@@ -147,7 +147,7 @@ def erb(name)
   # Return a list of applications that are not simply just stubs for Faces.
   def legacy_applications
     Puppet::Application.available_application_names.reject do |appname|
-      (is_face_app?(appname)) or (exclude_from_docs?(appname))
+      is_face_app?(appname) or exclude_from_docs?(appname)
     end.sort
   end
 
diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb
@@ -47,7 +47,7 @@ class MetaStat
 
     def initialize(stat, source_permissions)
       @stat = stat
-      @source_permissions_ignore = (!source_permissions || source_permissions == :ignore)
+      @source_permissions_ignore = !source_permissions || source_permissions == :ignore
     end
 
     def owner
@@ -111,16 +111,16 @@ def collect(source_permissions = nil)
 
     case stat.ftype
     when "file"
-      @checksum = ("{#{@checksum_type}}") + send("#{@checksum_type}_file", real_path).to_s
+      @checksum = "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s
     when "directory" # Always just timestamp the directory.
       @checksum_type = "ctime"
-      @checksum = ("{#{@checksum_type}}") + send("#{@checksum_type}_file", path).to_s
+      @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 = "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s rescue nil
     when "fifo", "socket"
       @checksum_type = "none"
-      @checksum = ("{#{@checksum_type}}") + send("#{@checksum_type}_file", real_path).to_s
+      @checksum = "{#{@checksum_type}}" + send("#{@checksum_type}_file", real_path).to_s
     else
       raise ArgumentError, _("Cannot manage files of type %{file_type}") % { file_type: stat.ftype }
     end
diff --git a/lib/puppet/forge.rb b/lib/puppet/forge.rb
@@ -231,7 +231,7 @@ def unpack(file, destination)
     end
 
     def deprecated?
-      @data['module'] && (!@data['module']['deprecated_at'].nil?)
+      @data['module'] && !@data['module']['deprecated_at'].nil?
     end
   end
 
diff --git a/lib/puppet/functions/regsubst.rb b/lib/puppet/functions/regsubst.rb
@@ -92,7 +92,7 @@ def regsubst_string(target, pattern, replacement, flags = nil, encoding = nil)
   end
 
   def regsubst_regexp(target, pattern, replacement, flags = nil)
-    pattern = (pattern.pattern || '') if pattern.is_a?(Puppet::Pops::Types::PRegexpType)
+    pattern = pattern.pattern || '' if pattern.is_a?(Puppet::Pops::Types::PRegexpType)
     inner_regsubst(target, pattern, replacement, flags == 'G' ? :gsub : :sub)
   end
 
diff --git a/lib/puppet/indirector/catalog/compiler.rb b/lib/puppet/indirector/catalog/compiler.rb
@@ -169,7 +169,7 @@ def inlineable?(resource, sources)
     when sources.empty?
       # TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
       return Puppet::Util::Profiler.profile(_("Not inlining resource without sources"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :no_sources]) { false }
-    when (!(sources.all? { |source| source =~ /^puppet:/ }))
+    when !(sources.all? { |source| source =~ /^puppet:/ })
       # TRANSLATORS Inlining refers to adding additional metadata (in this case we are not inlining)
       return Puppet::Util::Profiler.profile(_("Not inlining unsupported source scheme"), [:compiler, :static_compile_inlining, :skipped_file_metadata, :unsupported_scheme]) { false }
     else
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
@@ -28,7 +28,7 @@ def all_display_global_options
 
   # @api private
   def walk_inheritance_tree(start, sym)
-    result = (start || [])
+    result = start || []
     if self.is_a?(Class) and superclass.respond_to?(sym)
       result = superclass.send(sym) + result
     elsif self.class.respond_to?(sym)
diff --git a/lib/puppet/module_tool.rb b/lib/puppet/module_tool.rb
@@ -102,7 +102,7 @@ def self.build_tree(mods, dir)
         mod[:text] = "#{mod[:name]} (#{colorize(:cyan, version_string)})"
         mod[:text] += " [#{mod[:path]}]" unless mod[:path].to_s == dir.to_s
 
-        deps = (mod[:dependencies] || [])
+        deps = mod[:dependencies] || []
         deps.sort_by! { |a| a[:name] }
         build_tree(deps, dir)
       end
diff --git a/lib/puppet/parser/functions/create_resources.rb b/lib/puppet/parser/functions/create_resources.rb
@@ -61,10 +61,10 @@
   end
 
   raise ArgumentError, (_("create_resources(): wrong number of arguments (%{count}; must be 2 or 3)") % { count: args.length }) if args.length > 3
-  raise ArgumentError, (_('create_resources(): second argument must be a hash')) unless args[1].is_a?(Hash)
+  raise ArgumentError, _('create_resources(): second argument must be a hash') unless args[1].is_a?(Hash)
 
   if args.length == 3
-    raise ArgumentError, (_('create_resources(): third argument, if provided, must be a hash')) unless args[2].is_a?(Hash)
+    raise ArgumentError, _('create_resources(): third argument, if provided, must be a hash') unless args[2].is_a?(Hash)
   end
 
   type, instances, defaults = args
diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb
@@ -100,6 +100,6 @@ def result(string = nil)
   end
 
   def to_s
-    "template[#{(@__file__ || "inline")}]"
+    "template[#{@__file__ || "inline"}]"
   end
 end
diff --git a/lib/puppet/pops/lookup/explainer.rb b/lib/puppet/pops/lookup/explainer.rb
@@ -469,7 +469,7 @@ def initialize(explain_options = false, only_explain_options = false)
     end
 
     def push(qualifier_type, qualifier)
-      node = case (qualifier_type)
+      node = case qualifier_type
              when :global
                ExplainGlobal.new(@current, qualifier)
              when :location
diff --git a/lib/puppet/pops/lookup/hiera_config.rb b/lib/puppet/pops/lookup/hiera_config.rb
@@ -105,7 +105,7 @@ def self.config_exist?(config_root)
   end
 
   def self.symkeys_to_string(struct)
-    case (struct)
+    case struct
     when Hash
       map = {}
       struct.each_pair { |k, v| map[k.is_a?(Symbol) ? k.to_s : k] = symkeys_to_string(v) }
diff --git a/lib/puppet/pops/types/type_mismatch_describer.rb b/lib/puppet/pops/types/type_mismatch_describer.rb
@@ -894,7 +894,7 @@ def describe_argument_tuple(expected, actual, path)
     end
 
     def describe_tuple(expected, original, actual, path, size_mismatch_class)
-      return EMPTY_ARRAY if expected == actual || expected.types.empty? && (actual.is_a?(PArrayType))
+      return EMPTY_ARRAY if expected == actual || expected.types.empty? && actual.is_a?(PArrayType)
 
       expected_size = expected.size_type || TypeFactory.range(*expected.size_range)
 
diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
@@ -98,7 +98,7 @@ def verify(name, error, &block)
     private
 
     def op(property)
-      @ops[property.name] || ("-#{property.name}")
+      @ops[property.name] || "-#{property.name}"
     end
   end
 
diff --git a/lib/puppet/provider/package.rb b/lib/puppet/provider/package.rb
@@ -25,7 +25,7 @@ def properties
       # Purged is the "most uninstalled" a package can be, so a purged package will be in-sync with
       # either `ensure => absent` or `ensure => purged`; an absent package will be out of sync with `ensure => purged`.
       default_status = self.class.feature?(:purgeable) ? :purged : :absent
-      @property_hash = query || { :ensure => (default_status) }
+      @property_hash = query || { :ensure => default_status }
       @property_hash[:ensure] = default_status if @property_hash.empty?
     end
     @property_hash.dup
diff --git a/lib/puppet/provider/package/portage.rb b/lib/puppet/provider/package/portage.rb
@@ -121,7 +121,7 @@ def qatom
       else
         search = '=' + @resource[:name] + '-' + "#{should}"
       end
-      search_output = qatom_bin(*([search, '--format', output_format]))
+      search_output = qatom_bin(search, '--format', output_format)
       # verify if the search found anything
       match = result_format.match(search_output)
       if match
@@ -155,7 +155,7 @@ def qatom_result_fields
   end
 
   def self.get_sets
-    @sets ||= @sets = emerge(*(['--list-sets']))
+    @sets ||= @sets = emerge('--list-sets')
   end
 
   def query
diff --git a/lib/puppet/provider/package_targetable.rb b/lib/puppet/provider/package_targetable.rb
@@ -34,7 +34,7 @@ def self.prefetch(packages)
         catalog_packages.each do |catalog_package|
           if catalog_package[:name] == instance.name && catalog_package.original_parameters[:command] == command
             catalog_package.provider = instance
-            self.debug "Prefetched instance: %{name} via command: %{cmd}" % { name: instance.name, cmd: (command || :default) }
+            self.debug "Prefetched instance: %{name} via command: %{cmd}" % { name: instance.name, cmd: command || :default }
           end
         end
       end
diff --git a/lib/puppet/provider/service/systemd.rb b/lib/puppet/provider/service/systemd.rb
@@ -117,7 +117,7 @@ def enabled?
     return :false if output == 'indirect'
     return :true if code == 0
 
-    if (output.empty?) && (code > 0) && (Puppet.runtime[:facter].value('os.family').casecmp('debian').zero?)
+    if output.empty? && (code > 0) && Puppet.runtime[:facter].value('os.family').casecmp('debian').zero?
       ret = debian_enabled?
       return ret if ret
     end
diff --git a/lib/puppet/provider/user/directoryservice.rb b/lib/puppet/provider/user/directoryservice.rb
@@ -547,7 +547,7 @@ def write_password_to_users_plist(value)
       # variable IS a Hash and contains the 'SALTED-SHA512' key (indicating an
       # older 10.7-style password hash), it will be deleted and a newer
       # 10.8-style (PBKDF2) password hash will be generated.
-      if (shadow_hash_data.instance_of?(Hash)) && (shadow_hash_data.has_key?('SALTED-SHA512'))
+      if shadow_hash_data.instance_of?(Hash) && shadow_hash_data.has_key?('SALTED-SHA512')
         shadow_hash_data.delete('SALTED-SHA512')
       end
 
diff --git a/lib/puppet/provider/user/user_role_add.rb b/lib/puppet/provider/user/user_role_add.rb
@@ -76,7 +76,7 @@ def flush
   end
 
   def command(cmd)
-    cmd = ("role_#{cmd}").intern if is_role? or (!exists? and @resource[:ensure] == :role)
+    cmd = "role_#{cmd}".intern if is_role? or (!exists? and @resource[:ensure] == :role)
     super(cmd)
   end
 
diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb
@@ -235,11 +235,11 @@ def check_allow_dup
     # because by default duplicates are allowed.  This check is
     # to ensure consistent behaviour of the useradd provider when
     # using both useradd and luseradd
-    if (!@resource.allowdupe?) && @resource.forcelocal?
+    if !@resource.allowdupe? && @resource.forcelocal?
       if @resource.should(:uid) && finduser(:uid, @resource.should(:uid).to_s)
         raise(Puppet::Error, "UID #{@resource.should(:uid)} already exists, use allowdupe to force user creation")
       end
-    elsif @resource.allowdupe? && (!@resource.forcelocal?)
+    elsif @resource.allowdupe? && !@resource.forcelocal?
       return ["-o"]
     end
     []
@@ -324,7 +324,7 @@ def addcmd
     else
       cmd = [command(:add)]
     end
-    if (!@resource.should(:gid)) && Puppet::Util.gid(@resource[:name])
+    if !@resource.should(:gid) && Puppet::Util.gid(@resource[:name])
       cmd += ["-g", @resource[:name]]
     end
     cmd += add_properties
diff --git a/lib/puppet/settings.rb b/lib/puppet/settings.rb
@@ -517,7 +517,7 @@ def include?(name)
   def print_config_options
     if Puppet::Util::Log.sendlevel?(:info)
       Puppet::Util::Log.newdestination(:console)
-      message = (_("Using --configprint is deprecated. Use 'puppet config <subcommand>' instead."))
+      message = _("Using --configprint is deprecated. Use 'puppet config <subcommand>' instead.")
       Puppet.deprecation_warning(message)
     end
 
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
@@ -312,7 +312,7 @@ def failed_dependencies?(resource)
         @failed_class_dependencies_already_notified.merge(class_dependencies_to_be_notified)
       else
         unless @merge_dependency_warnings || is_puppet_class
-          s.failed_dependencies.find_all { |d| !(@failed_dependencies_already_notified.include?(d.ref)) }.each do |dep|
+          s.failed_dependencies.find_all { |d| !@failed_dependencies_already_notified.include?(d.ref) }.each do |dep|
             resource.notice _("Dependency %{dep} has failures: %{status}") % { dep: dep, status: resource_status(dep).failed }
             @failed_dependencies_already_notified.add(dep.ref)
           end
@@ -414,7 +414,7 @@ def skip?(resource)
       unless resource.instance_of?(Puppet::Type.type(:whit))
         if @merge_dependency_warnings && resource.parent && failed_dependencies?(resource.parent)
           ps = resource_status(resource.parent)
-          ps.failed_dependencies.find_all { |d| !(@failed_class_dependencies_already_warned.include?(d.ref)) }.each do |dep|
+          ps.failed_dependencies.find_all { |d| !@failed_class_dependencies_already_warned.include?(d.ref) }.each do |dep|
             resource.parent.warning _("Skipping resources in class because of failed class dependencies")
             @failed_class_dependencies_already_warned.add(dep.ref)
           end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
@@ -1772,7 +1772,7 @@ def self.providers
   def self.validprovider?(name)
     name = name.intern
 
-    (provider_hash.has_key?(name) && provider_hash[name].suitable?)
+    provider_hash.has_key?(name) && provider_hash[name].suitable?
   end
 
   # Creates a new provider of a type.
diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb
@@ -19,7 +19,7 @@ def perform_backup(file = nil)
   private
 
   def perform_backup_with_bucket(fileobj)
-    file = (fileobj.instance_of?(String)) ? fileobj : fileobj.name
+    file = fileobj.instance_of?(String) ? fileobj : fileobj.name
     case Puppet::FileSystem.lstat(file).ftype
     when "directory"
       # we don't need to backup directories when recurse is on
@@ -34,7 +34,7 @@ def perform_backup_with_bucket(fileobj)
   end
 
   def perform_backup_with_backuplocal(fileobj, backup)
-    file = (fileobj.instance_of?(String)) ? fileobj : fileobj.name
+    file = fileobj.instance_of?(String) ? fileobj : fileobj.name
     newfile = file + backup
 
     remove_backup(newfile)
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
@@ -224,7 +224,7 @@ def Log.flushqueue
   #  error scenario.
   # @return nil
   def Log.force_flushqueue
-    if @destinations.empty? and !(@queued.empty?)
+    if @destinations.empty? and !@queued.empty?
       newdestination(:console)
     end
     flushqueue
diff --git a/lib/puppet/util/logging.rb b/lib/puppet/util/logging.rb
@@ -160,7 +160,7 @@ def puppet_deprecation_warning(message, options = {})
     file = options[:file]
     line = options[:line]
     # TRANSLATORS the literals ":file", ":line", and ":key" should not be translated
-    raise Puppet::DevError, _("Need either :file and :line, or :key") if (key.nil?) && (file.nil? || line.nil?)
+    raise Puppet::DevError, _("Need either :file and :line, or :key") if key.nil? && (file.nil? || line.nil?)
 
     key ||= "#{file}:#{line}"
     issue_deprecation_warning(message, key, file, line, false)
@@ -243,7 +243,7 @@ def log_deprecations_to_file(deprecations_file, pattern = nil)
           next if $logged_deprecation_warnings.has_key?(offender)
 
           $logged_deprecation_warnings[offender] = true
-          next unless (pattern.nil?) || (message =~ pattern)
+          next unless pattern.nil? || (message =~ pattern)
 
           f.puts(message)
           f.puts(offender)
diff --git a/lib/puppet/util/package/version/pip.rb b/lib/puppet/util/package/version/pip.rb
@@ -38,7 +38,7 @@ class Pip
     def self.parse(version)
       raise ValidationFailure, version.to_s unless version.is_a? String
 
-      matched = version.match(Regexp.new(("^\\s*") + VERSION_PATTERN + ("\\s*$"), Regexp::EXTENDED | Regexp::MULTILINE | Regexp::IGNORECASE))
+      matched = version.match(Regexp.new("^\\s*" + VERSION_PATTERN + "\\s*$", Regexp::EXTENDED | Regexp::MULTILINE | Regexp::IGNORECASE))
       raise ValidationFailure, version unless matched
 
       new(matched)
diff --git a/lib/puppet/util/rpm_compare.rb b/lib/puppet/util/rpm_compare.rb
@@ -149,8 +149,8 @@ def rpm_parse_evr(full_version)
   # found in python/header-py.c, as used by rpm.
   def compare_values(s1, s2)
     return 0 if s1.nil? && s2.nil?
-    return 1 if (!s1.nil?) && s2.nil?
-    return -1 if s1.nil? && (!s2.nil?)
+    return 1 if !s1.nil? && s2.nil?
+    return -1 if s1.nil? && !s2.nil?
 
     return rpmvercmp(s1, s2)
   end
