Merge pull request #9284 from AriaXLi/PUP-11993/more_style_cops

(PUP-11993) Even more Style cops!

.rubocop.yml

@@ -51,6 +51,12 @@ Style/RedundantSelfAssignment:
   Exclude:
     - 'lib/puppet/context.rb'
 
+# Enabling this cop causes failures in rb_tree_map_spec relating to important
+# function slike being unable to delete nodes and returning nil when the key
+# cannot be found
+Style/PreferredHashMethods:
+  Enabled: false
+
 # Explicitly enables this cop new in 1.7
 Layout/SpaceBeforeBrackets:
   Enabled: true

.rubocop_todo.yml

@@ -646,12 +646,6 @@ Style/NumericPredicate:
 Style/OptionalBooleanParameter:
   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).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: implicit, explicit
@@ -664,19 +658,6 @@ Style/RescueStandardError:
 Style/SafeNavigation:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/SelfAssignment:
-  Exclude:
-    - 'lib/puppet/pops/evaluator/access_operator.rb'
-    - 'lib/puppet/pops/evaluator/collectors/fixed_set_collector.rb'
-    - 'lib/puppet/pops/evaluator/epp_evaluator.rb'
-    - 'lib/puppet/pops/parser/epp_parser.rb'
-    - 'lib/puppet/pops/parser/parser_support.rb'
-    - 'lib/puppet/pops/time/timestamp.rb'
-    - 'lib/puppet/pops/types/recursion_guard.rb'
-    - 'lib/puppet/property/ordered_list.rb'
-    - 'lib/puppet/util/autoload.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: AllowAsExpressionSeparator.
 Style/Semicolon:
@@ -712,12 +693,6 @@ Style/SingleLineMethods:
 Style/SoleNestedConditional:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: RequireEnglish, EnforcedStyle.
-# SupportedStyles: use_perl_names, use_english_names, use_builtin_english_names
-Style/SpecialGlobalVars:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 Style/StderrPuts:
   Exclude:
@@ -747,77 +722,18 @@ Style/StringConcatenation:
 Style/StringLiterals:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: single_quotes, double_quotes
-Style/StringLiteralsInInterpolation:
-  Enabled: false
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/StructInheritance:
-  Exclude:
-    - 'lib/puppet/settings.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle, MinSize.
 # SupportedStyles: percent, brackets
 Style/SymbolArray:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/SymbolLiteral:
-  Exclude:
-    - 'lib/puppet/functions/defined.rb'
-
-# This cop supports unsafe auto-correction (--auto-correct-all).
-# Configuration parameters: AllowMethodsWithArguments, IgnoredMethods, AllowComments.
-# IgnoredMethods: respond_to, define_method
-Style/SymbolProc:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyle, AllowSafeAssignment.
 # SupportedStyles: require_parentheses, require_no_parentheses, require_parentheses_when_complex
 Style/TernaryParentheses:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyleForMultiline.
-# SupportedStylesForMultiline: comma, consistent_comma, no_comma
-Style/TrailingCommaInArguments:
-  Exclude:
-    - 'lib/puppet/defaults.rb'
-    - 'lib/puppet/ffi/windows/functions.rb'
-    - 'lib/puppet/ffi/windows/structs.rb'
-    - 'lib/puppet/http/service/compiler.rb'
-    - 'lib/puppet/http/service/report.rb'
-    - 'lib/puppet/indirector/file_bucket_file/rest.rb'
-    - 'lib/puppet/indirector/file_content/rest.rb'
-    - 'lib/puppet/indirector/file_metadata/rest.rb'
-    - 'lib/puppet/pops/types/p_uri_type.rb'
-    - 'lib/puppet/util/windows/adsi.rb'
-    - 'lib/puppet/util/windows/service.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyleForMultiline.
-# SupportedStylesForMultiline: comma, consistent_comma, no_comma
-Style/TrailingCommaInArrayLiteral:
-  Exclude:
-    - 'ext/windows/service/daemon.rb'
-    - 'lib/puppet/functions.rb'
-    - 'lib/puppet/parser/compiler.rb'
-    - 'lib/puppet/pops/types/p_uri_type.rb'
-    - 'lib/puppet/pops/types/string_converter.rb'
-    - 'lib/puppet/pops/types/types.rb'
-    - 'lib/puppet/provider/package/windows/exe_package.rb'
-    - 'lib/puppet/provider/service/upstart.rb'
-    - 'lib/puppet/settings.rb'
-    - 'lib/puppet/ssl/certificate_request.rb'
-    - 'lib/puppet/ssl/oids.rb'
-    - 'lib/puppet/transaction/persistence.rb'
-    - 'lib/puppet/util/at_fork/solaris.rb'
-    - 'lib/puppet/util/windows/registry.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: EnforcedStyleForMultiline.
 # SupportedStylesForMultiline: comma, consistent_comma, no_comma
@@ -846,39 +762,10 @@ Style/TrailingUnderscoreVariable:
 Style/TrivialAccessors:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/UnlessElse:
-  Exclude:
-    - 'lib/puppet/application/lookup.rb'
-    - 'lib/puppet/pops/evaluator/evaluator_impl.rb'
-    - 'lib/puppet/pops/types/type_mismatch_describer.rb'
-    - 'lib/puppet/provider/package/aix.rb'
-    - 'lib/puppet/provider/package/yum.rb'
-    - 'lib/puppet/ssl/openssl_loader.rb'
-    - 'lib/puppet/type/exec.rb'
-    - 'lib/puppet/util/command_line/trollop.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/UnpackFirst:
-  Exclude:
-    - 'lib/puppet/file_serving/http_metadata.rb'
-    - 'lib/puppet/provider/nameservice/directoryservice.rb'
-    - 'lib/puppet/provider/user/directoryservice.rb'
-
-# This cop supports safe auto-correction (--auto-correct).
-Style/VariableInterpolation:
-  Exclude:
-    - 'lib/puppet/util/command_line/trollop.rb'
-    - 'lib/puppet/util/rdoc/generators/puppet_generator.rb'
-
 # This cop supports safe auto-correction (--auto-correct).
 Style/WhenThen:
   Enabled: false
 
-# This cop supports safe auto-correction (--auto-correct).
-Style/WhileUntilDo:
-  Enabled: false
-
 # This cop supports safe auto-correction (--auto-correct).
 Style/WhileUntilModifier:
   Exclude:
@@ -889,12 +776,6 @@ Style/WhileUntilModifier:
     - 'lib/puppet/pops/types/p_object_type.rb'
     - 'lib/puppet/util/windows/process.rb'
 
-# This cop supports safe auto-correction (--auto-correct).
-# Configuration parameters: EnforcedStyle, MinSize, WordRegex.
-# SupportedStyles: percent, brackets
-Style/WordArray:
-  Enabled: false
-
 # This cop supports unsafe auto-correction (--auto-correct-all).
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: forbid_for_all_comparison_operators, forbid_for_equality_operators_only, require_for_all_comparison_operators, require_for_equality_operators_only

ext/windows/service/daemon.rb

@@ -75,7 +75,7 @@ def service_main(*argsv)
 
     service = self
     @run_thread = Thread.new do
-      while service.running? do
+      while service.running?
         runinterval = service.parse_runinterval(ruby_puppet_cmd)
 
         if service.state == RUNNING or service.state == IDLE
@@ -197,7 +197,7 @@ def load_env(base_dir)
     ENV['SSL_CERT_FILE'] = File.join(base_dir, 'puppet', 'ssl', 'cert.pem').tr('/', '\\')
     ENV['Path'] = [
       File.join(base_dir, 'puppet', 'bin'),
-      File.join(base_dir, 'bin'),
+      File.join(base_dir, 'bin')
     ].join(';').tr('/', '\\') + ';' + ENV.fetch('Path', nil)
 
     # ENV that uses forward slashes
@@ -207,6 +207,6 @@ def load_env(base_dir)
   end
 end
 
-if __FILE__ == $0
+if __FILE__ == $PROGRAM_NAME
   WindowsDaemon.mainloop
 end

lib/puppet/application/face_base.rb

@@ -74,7 +74,7 @@ def parse_options
     # non-option word to use as the action.
     action_name = nil
     index       = -1
-    until action_name or (index += 1) >= command_line.args.length do
+    until action_name or (index += 1) >= command_line.args.length
       item = command_line.args[index]
       if item =~ /^-/
         option = @face.options.find do |name|

lib/puppet/application/lookup.rb

@@ -8,7 +8,7 @@
 class Puppet::Application::Lookup < Puppet::Application
   RUN_HELP = _("Run 'puppet lookup --help' for more details").freeze
   DEEP_MERGE_OPTIONS = '--knock-out-prefix, --sort-merged-arrays, and --merge-hash-arrays'
-  TRUSTED_INFORMATION_FACTS = ["hostname", "domain", "fqdn", "clientcert"].freeze
+  TRUSTED_INFORMATION_FACTS = %w[hostname domain fqdn clientcert].freeze
 
   run_mode :server
 
@@ -369,7 +369,9 @@ def generate_scope
       end
     end
 
-    unless node.is_a?(Puppet::Node) # to allow unit tests to pass a node instance
+    if node.is_a?(Puppet::Node)
+      node.add_extra_facts(given_facts) if given_facts
+    else # to allow unit tests to pass a node instance
       facts = retrieve_node_facts(node, given_facts)
       ni = Puppet::Node.indirection
       tc = ni.terminus_class
@@ -399,8 +401,6 @@ def generate_scope
         node = ni.find(node, facts: facts, environment: Puppet[:environment])
         ni.terminus_class = tc
       end
-    else
-      node.add_extra_facts(given_facts) if given_facts
     end
     node.environment = Puppet[:environment] if Puppet.settings.set_by_cli?(:environment)
     Puppet[:code] = 'undef' unless options[:compile]

lib/puppet/coercion.rb

@@ -37,6 +37,6 @@ def self.boolean(value)
   # @raise
   # @api private
   def self.boolean_values
-    ['true', 'false', 'yes', 'no']
+    %w[true false yes no]
   end
 end

lib/puppet/confine/feature.rb

@@ -4,7 +4,7 @@
 
 class Puppet::Confine::Feature < Puppet::Confine
   def self.summarize(confines)
-    confines.collect { |c| c.values }.flatten.uniq.find_all { |value| !confines[0].pass?(value) }
+    confines.collect(&:values).flatten.uniq.find_all { |value| !confines[0].pass?(value) }
   end
 
   # Is the named feature available?

lib/puppet/confine/variable.rb

@@ -29,7 +29,7 @@ def initialize(values)
   end
 
   def message(value)
-    "facter value '#{test_value}' for '#{name}' not in required list '#{values.join(",")}'"
+    "facter value '#{test_value}' for '#{name}' not in required list '#{values.join(',')}'"
   end
 
   # Compare the passed-in value to the retrieved value.

lib/puppet/daemon.rb

@@ -83,7 +83,7 @@ def close_streams
   def reexec
     raise Puppet::DevError, _("Cannot reexec unless ARGV arguments are set") unless argv
 
-    command = $0 + " " + argv.join(" ")
+    command = $PROGRAM_NAME + " " + argv.join(" ")
     Puppet.notice "Restarting with '#{command}'"
     stop(:exit => false)
     exec(command)

lib/puppet/defaults.rb

@@ -132,7 +132,7 @@ def self.initialize_default_settings!(settings)
     :log_level => {
       :default    => 'notice',
       :type       => :enum,
-      :values     => ["debug","info","notice","warning","err","alert","emerg","crit"],
+      :values     => %w[debug info notice warning err alert emerg crit],
       :desc       => "Default logging level for messages from Puppet. Allowed values are:
 
         * debug
@@ -1298,7 +1298,7 @@ def self.initialize_default_settings!(settings)
       :sourceaddress => {
         :default    => nil,
         :desc       => "The address the agent should use to initiate requests.",
-      },
+      }
     )
 
   settings.define_settings(:environment,

lib/puppet/environments.rb

@@ -313,7 +313,7 @@ def get_conf(name)
     end
 
     def clear_all
-      @loaders.each { |loader| loader.clear_all }
+      @loaders.each(&:clear_all)
     end
   end
 

lib/puppet/external/dot.rb

@@ -100,17 +100,17 @@ def change_tab(t)
 
   # options for graph declaration
 
-  GRAPH_OPTS = [
-    'bgcolor',
-    'center', 'clusterrank', 'color', 'concentrate',
-    'fontcolor', 'fontname', 'fontsize',
-    'label', 'layerseq',
-    'margin', 'mclimit',
-    'nodesep', 'nslimit',
-    'ordering', 'orientation',
-    'page',
-    'rank', 'rankdir', 'ranksep', 'ratio',
-    'size'
+  GRAPH_OPTS = %w[
+    bgcolor
+    center clusterrank color concentrate
+    fontcolor fontname fontsize
+    label layerseq
+    margin mclimit
+    nodesep nslimit
+    ordering orientation
+    page
+    rank rankdir ranksep ratio
+    size
   ]
 
   # a root class for any element in dot notation

lib/puppet/face/catalog/select.rb

@@ -44,7 +44,7 @@
       if value.nil? then
         _("no matching resources found")
       else
-        value.map { |x| x.to_s }.join("\n")
+        value.map(&:to_s).join("\n")
       end
     end
   end

lib/puppet/face/help.rb

@@ -207,7 +207,7 @@ def horribly_extract_summary_from(appname)
     # depends on the implementation coincidence of how our pages are
     # formatted.  If we can't match the pattern we expect we return the empty
     # string to ensure we don't blow up in the summary. --daniel 2011-04-11
-    while line = help.shift do # rubocop:disable Lint/AssignmentInCondition
+    while line = help.shift # rubocop:disable Lint/AssignmentInCondition
       md = /^puppet-#{appname}\([^)]+\) -- (.*)$/.match(line)
       if md
         return md[1]

lib/puppet/face/module/changes.rb

@@ -38,9 +38,7 @@
       else
         Puppet.warning _("%{count} files modified") % { count: return_value.size }
       end
-      return_value.map do |changed_file|
-        changed_file.to_s
-      end.join("\n")
+      return_value.map(&:to_s).join("\n")
     end
   end
 end

lib/puppet/face/module/list.rb

@@ -117,7 +117,7 @@ def unmet_dependencies(environment)
     end
 
     # Prepare the unmet dependencies for display on the console.
-    environment.modules.sort_by { |mod| mod.name }.each do |mod|
+    environment.modules.sort_by(&:name).each do |mod|
       unmet_grouped = Hash.new { |h, k| h[k] = [] }
       unmet_grouped = mod.unmet_dependencies.each_with_object(unmet_grouped) do |dep, acc|
         acc[dep[:reason]] << dep