(PUP-11993) Style Cops (N-P)

.rubocop_todo.yml

@@ -629,175 +629,23 @@ Style/MultipleComparison:
 Style/MutableConstant:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: both, prefix, postfix
-Style/NegatedIf:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/NegatedWhile:
-  Exclude:
-    - 'lib/puppet/file_system/file_impl.rb'
-    - 'lib/puppet/graph/simple_graph.rb'
-    - 'lib/puppet/http/client.rb'
-    - 'lib/puppet/pops/parser/heredoc_support.rb'
-    - 'lib/puppet/pops/types/p_object_type.rb'
-    - 'lib/puppet/scheduler/scheduler.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/NestedModifier:
-  Exclude:
-    - 'lib/puppet/util/command_line/trollop.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: AllowedMethods.
-# AllowedMethods: be, be_a, be_an, be_between, be_falsey, be_kind_of, be_instance_of, be_truthy, be_within, eq, eql, end_with, include, match, raise_error, respond_to, start_with
-Style/NestedParenthesizedCalls:
-  Exclude:
-    - 'lib/puppet/provider/user/directoryservice.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/NestedTernaryOperator:
-  Exclude:
-    - 'lib/puppet/external/dot.rb'
-    - 'lib/puppet/module_tool/shared_behaviors.rb'
-    - 'lib/puppet/pops/loader/ruby_legacy_function_instantiator.rb'
-    - 'lib/puppet/pops/lookup/hiera_config.rb'
-    - 'lib/puppet/pops/parser/lexer2.rb'
-    - 'lib/puppet/pops/types/string_converter.rb'
-    - 'lib/puppet/pops/types/type_factory.rb'
-    - 'lib/puppet/pops/types/types.rb'
-    - 'lib/puppet/util/log.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, MinBodyLength.
-# SupportedStyles: skip_modifier_ifs, always
-Style/Next:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: predicate, comparison
-Style/NilComparison:
-  Exclude:
-    - 'lib/puppet/pops/types/type_factory.rb'
-    - 'lib/puppet/pops/types/types.rb'
-    - 'lib/puppet/provider/package/dnfmodule.rb'
-    - 'lib/puppet/provider/package/nim.rb'
-    - 'lib/puppet/provider/package/windows/exe_package.rb'
-    - 'lib/puppet/provider/user/directoryservice.rb'
-    - 'lib/puppet/type/file.rb'
-    - 'lib/puppet/util/pidlock.rb'
-    - 'lib/puppet/util/suidmanager.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: IncludeSemanticChanges.
-Style/NonNilCheck:
-  Exclude:
-    - 'lib/puppet/forge.rb'
-    - 'lib/puppet/http/session.rb'
-    - 'lib/puppet/network/http/request.rb'
-    - 'lib/puppet/provider/package/pip.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/Not:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedOctalStyle.
-# SupportedOctalStyles: zero_with_o, zero_only
-Style/NumericLiteralPrefix:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: Strict, AllowedNumbers.
-Style/NumericLiterals:
-  MinDigits: 11
-
 # This cop supports unsafe auto-correction (--auto-correct-all).
 # Configuration parameters: EnforcedStyle, IgnoredMethods.
 # SupportedStyles: predicate, comparison
 Style/NumericPredicate:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: AlwaysCorrectToMultiline.
-Style/OneLineConditional:
-  Exclude:
-    - 'lib/puppet/pops/parser/evaluating_parser.rb'
-    - 'lib/puppet/provider/package/yum.rb'
-    - 'lib/puppet/provider/service/openwrt.rb'
-
 # Configuration parameters: AllowedMethods.
 # AllowedMethods: respond_to_missing?
 Style/OptionalBooleanParameter:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/OrAssignment:
-  Exclude:
-    - 'lib/puppet/application/lookup.rb'
-    - 'lib/puppet/file_system/windows.rb'
-    - 'lib/puppet/pops/parser/lexer2.rb'
-    - 'lib/puppet/pops/types/types.rb'
-    - 'lib/puppet/provider/user/directoryservice.rb'
-    - 'lib/puppet/ssl/base.rb'
-    - 'lib/puppet/util/errors.rb'
-    - 'util/rspec_grouper'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/ParallelAssignment:
-  Exclude:
-    - 'lib/puppet/configurer/downloader.rb'
-    - 'lib/puppet/graph/rb_tree_map.rb'
-    - 'lib/puppet/module_tool/shared_behaviors.rb'
-    - 'lib/puppet/network/client_request.rb'
-    - 'lib/puppet/parser/relationship.rb'
-    - 'lib/puppet/provider/package/ports.rb'
-    - 'lib/puppet/provider/service/smf.rb'
-    - 'lib/puppet/provider/user/aix.rb'
-    - 'lib/puppet/relationship.rb'
-    - 'lib/puppet/settings/config_file.rb'
-    - 'lib/puppet/type/file/ensure.rb'
-    - 'lib/puppet/util/suidmanager.rb'
-    - 'lib/puppet/util/windows/registry.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: AllowSafeAssignment, AllowInMultilineConditions.
-Style/ParenthesesAroundCondition:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: PreferredDelimiters.
-Style/PercentLiteralDelimiters:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: lower_case_q, upper_case_q
-Style/PercentQLiterals:
-  Exclude:
-    - 'lib/puppet/provider/package/dpkg.rb'
-    - 'lib/puppet/provider/package/rpm.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/PerlBackrefs:
-  Enabled: false
-
 # This cop supports unsafe auto-correction (--auto-correct-all).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: short, verbose
 Style/PreferredHashMethods:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/Proc:
-  Exclude:
-    - 'lib/puppet/pops/evaluator/deferred_resolver.rb'
-    - 'lib/puppet/pops/lookup/hiera_config.rb'
-    - 'lib/puppet/util/windows/daemon.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle, AllowedCompactTypes.
 # SupportedStyles: compact, exploded

ext/windows/service/daemon.rb

@@ -36,7 +36,7 @@ def service_main(*argsv)
 
     @LOG_TO_FILE = (argsv.index('--logtofile') ? true : false)
 
-    if (@LOG_TO_FILE)
+    if @LOG_TO_FILE
       FileUtils.mkdir_p(File.dirname(LOG_FILE))
       args = args.gsub("--logtofile", "")
     end
@@ -131,7 +131,7 @@ def log_exception(e)
 
   def log(msg, level)
     if LEVELS.index(level) >= @loglevel
-      if (@LOG_TO_FILE)
+      if @LOG_TO_FILE
         # without this change its possible that we get Encoding errors trying to write UTF-8 messages in current codepage
         File.open(LOG_FILE, 'a:UTF-8') { |f| f.puts("#{Time.now} Puppet (#{level}): #{msg}") }
       end
@@ -153,15 +153,15 @@ def report_windows_event(type, id, message)
     rescue Exception
       # Ignore all errors
     ensure
-      if (!eventlog.nil?)
+      unless eventlog.nil?
         eventlog.close
       end
     end
   end
 
   def parse_runinterval(puppet_path)
     begin
-      runinterval = %x{#{puppet_path} config --section agent --log_level notice print runinterval}.to_i
+      runinterval = %x(#{puppet_path} config --section agent --log_level notice print runinterval).to_i
       if runinterval == 0
         runinterval = 1800
         log_err("Failed to determine runinterval, defaulting to #{runinterval} seconds")
@@ -176,7 +176,7 @@ def parse_runinterval(puppet_path)
 
   def parse_log_level(puppet_path, cmdline_debug)
     begin
-      loglevel = %x{#{puppet_path} config --section agent --log_level notice print log_level}.chomp
+      loglevel = %x(#{puppet_path} config --section agent --log_level notice print log_level).chomp
       unless loglevel && respond_to?("log_#{loglevel}")
         loglevel = :notice
         log_err("Failed to determine loglevel, defaulting to #{loglevel}")

lib/puppet/agent/disabler.rb

@@ -26,7 +26,7 @@ def enable
   def disable(msg = nil)
     data = {}
     Puppet.notice _("Disabling Puppet.")
-    if (!msg.nil?)
+    unless msg.nil?
       data[DISABLED_MESSAGE_JSON_KEY] = msg
     end
     disable_lockfile.lock(data)

lib/puppet/application.rb

@@ -180,7 +180,7 @@ def controlled_run(&block)
     def option(*options, &block)
       long = options.find { |opt| opt =~ /^--/ }.gsub(/^--(?:\[no-\])?([^ =]+).*$/, '\1').tr('-', '_')
       fname = "handle_#{long}".intern
-      if (block_given?)
+      if block_given?
         define_method(fname, &block)
       else
         define_method(fname) do |value|
@@ -242,7 +242,7 @@ def find(application_name)
       ####  eventually we need to issue a deprecation warning here,
       ####  and then get rid of this stanza in a subsequent release.
       ################################################################
-      if (clazz.nil?)
+      if clazz.nil?
         class_name = application_name.capitalize
         clazz = try_load_class(class_name)
       end
@@ -284,7 +284,7 @@ def run_mode(mode_name = nil)
         Puppet.settings.preferred_run_mode = mode_name
       end
 
-      return @run_mode if @run_mode and not mode_name
+      return @run_mode if @run_mode and !mode_name
 
       require_relative '../puppet/util/run_mode'
       @run_mode = Puppet::Util::RunMode[mode_name || Puppet.settings.preferred_run_mode]
@@ -452,7 +452,7 @@ def setup
   # @return [void]
   # @api public
   def setup_logs
-    handle_logdest_arg(Puppet[:logdest]) if !options[:setdest]
+    handle_logdest_arg(Puppet[:logdest]) unless options[:setdest]
 
     unless options[:setdest]
       if options[:debug] || options[:verbose]

lib/puppet/application/agent.rb

@@ -441,7 +441,7 @@ def onetime(daemon)
 
     daemon.stop(:exit => false)
 
-    if not exitstatus
+    if !exitstatus
       exit(1)
     elsif options[:detailed_exitcodes] then
       exit(exitstatus)
@@ -509,7 +509,7 @@ def enable_disable_client(agent)
   end
 
   def setup_agent
-    agent = Puppet::Agent.new(Puppet::Configurer, (not(Puppet[:onetime])))
+    agent = Puppet::Agent.new(Puppet::Configurer, (!(Puppet[:onetime])))
 
     enable_disable_client(agent) if options[:enable] or options[:disable]
 

lib/puppet/application/apply.rb

@@ -276,7 +276,7 @@ def main
 
         exit_status = apply_catalog(catalog)
 
-        if not exit_status
+        if !exit_status
           exit(1)
         elsif options[:detailed_exitcodes] then
           exit(exit_status)

lib/puppet/application/describe.rb

@@ -20,8 +20,8 @@ def wrap(txt, opts)
 
     while work.length > textLen
       if work =~ patt
-        res << $1
-        work.slice!(0, $MATCH.length)
+        res << ::Regexp.last_match(1)
+        work.slice!(0, ::Regexp.last_match(0).length)
       else
         res << work.slice!(0, textLen)
       end
@@ -43,7 +43,7 @@ def scrub(text)
     # If we can match an indentation, then just remove that same level of
     # indent from every line.
     if text =~ /^(\s+)/
-      indent = $1
+      indent = ::Regexp.last_match(1)
       return text.gsub(/^#{indent}/, '')
     else
       return text

lib/puppet/application/face_base.rb

@@ -88,7 +88,7 @@ def parse_options
           # when we get there. --daniel 2011-04-04
           if option.takes_argument? and !item.index('=')
             index += 1 unless
-              (option.optional_argument? and command_line.args[index + 1] =~ /^-/)
+              option.optional_argument? and command_line.args[index + 1] =~ /^-/
           end
         else
           option = find_global_settings_argument(item)
@@ -103,7 +103,7 @@ def parse_options
           else
             option = find_application_argument(item)
             if option
-              index += 1 if (option[:argument] and not option[:optional])
+              index += 1 if option[:argument] and !(option[:optional])
             else
               raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
             end

lib/puppet/application/filebucket.rb

@@ -213,7 +213,7 @@ def help
   def run_command
     @args = command_line.args
     command = args.shift
-    return send(command) if %w{get backup restore diff list}.include? command
+    return send(command) if %w[get backup restore diff list].include? command
 
     help
   end

lib/puppet/application/lookup.rb

@@ -354,7 +354,7 @@ def generate_scope
         given_facts = Puppet::Util::Yaml.safe_load_file(fact_file)
       else
         given_facts = Puppet::Util::Json.load_file_if_valid(fact_file)
-        given_facts = Puppet::Util::Yaml.safe_load_file_if_valid(fact_file) unless given_facts
+        given_facts ||= Puppet::Util::Yaml.safe_load_file_if_valid(fact_file)
       end
 
       unless given_facts.instance_of?(Hash)

lib/puppet/application/resource.rb

@@ -207,7 +207,7 @@ def parse_args(args)
     params = {}
     args.each do |setting|
       if setting =~ /^(\w+)=(.+)$/
-        params[$1] = $2
+        params[::Regexp.last_match(1)] = ::Regexp.last_match(2)
       else
         raise _("Invalid parameter setting %{setting}") % { setting: setting }
       end

lib/puppet/application/ssl.rb

@@ -160,7 +160,7 @@ def main
 
       clean(certname)
     when 'bootstrap'
-      if !Puppet::Util::Log.sendlevel?(:info)
+      unless Puppet::Util::Log.sendlevel?(:info)
         Puppet::Util::Log.level = :info
       end
       @machine.ensure_client_certificate

lib/puppet/configurer.rb

@@ -83,8 +83,8 @@ def retrieve_catalog(facts, query_options)
     else
       result = retrieve_new_catalog(facts, query_options)
 
-      if !result
-        if !Puppet[:usecacheonfailure]
+      unless result
+        unless Puppet[:usecacheonfailure]
           Puppet.warning _("Not using cache on failed catalog")
           return nil
         end
@@ -390,7 +390,7 @@ def run_internal(options)
       configured_environment = Puppet[:environment] if Puppet.settings.set_by_config?(:environment)
 
       # We only need to find out the environment to run in if we don't already have a catalog
-      unless (cached_catalog || options[:catalog] || Puppet.settings.set_by_cli?(:environment) || Puppet[:strict_environment_mode])
+      unless cached_catalog || options[:catalog] || Puppet.settings.set_by_cli?(:environment) || Puppet[:strict_environment_mode]
         Puppet.debug(_("Environment not passed via CLI and no catalog was given, attempting to find out the last server-specified environment"))
         initial_environment, loaded_last_environment = last_server_specified_environment
 
@@ -442,7 +442,7 @@ def run_internal(options)
       # facts may be used to determine which catalog we get, we need to
       # rerun the process if the environment is changed.
       tries = 0
-      while catalog.environment and not catalog.environment.empty? and catalog.environment != @environment
+      while catalog.environment and !catalog.environment.empty? and catalog.environment != @environment
         if tries > 3
           raise Puppet::Error, _("Catalog environment didn't stabilize after %{tries} fetches, aborting run") % { tries: tries }
         end