(maint) Fix simple Lint/AssignmentInConditional rubocop violations.

.rubocop.yml

@@ -60,12 +60,6 @@ Style/RedundantParentheses:
 Lint/AmbiguousOperator:
   Enabled: false
 
-# DISABLED since for all the checked, we are basically checking nil
-# TODO: Change the checking so that if the variable being assigned to has
-# a value ALREADY, then raise an error.
-Lint/AssignmentInCondition:
-  Enabled: false
-
 # DISABLED - not useful
 Layout/SpaceBeforeComment:
   Enabled: false

benchmarks/evaluations/benchmarker.rb

@@ -45,7 +45,8 @@ def run(args = {})
     measurements = []
     @micro_benchmarks.each do |name, source|
       # skip if all but the wanted if a single benchmark is wanted
-      next unless details == 'all' || match = details.match(/#{name}(?:[\._\s](parse|eval))?$/)
+      match = details.match(/#{name}(?:[\._\s](parse|eval))?$/)
+      next unless details == 'all' || match
       # if name ends with .parse or .eval only do that part, else do both parts
       ending = match ? match[1] : nil # parse, eval or nil ending
       unless ending == 'eval'

ext/yaml_nodes.rb

@@ -74,7 +74,7 @@ def read_node(node)
 
 # Iterate over any provided parents, merging in there information.
 parents_seen = []
-while parent = info["parent"]
+while parent = info["parent"] #rubocop:disable Lint/AssignmentInCondition
   raise "Found inheritance loop with parent #{parent}" if parents_seen.include?(parent)
 
   parents_seen << parent
@@ -84,21 +84,21 @@ def read_node(node)
   parent_info = read_node(parent)
 
   # Include any parent classes in our list.
-  if pclasses = parent_info["classes"]
-    info["classes"] += pclasses
+  if parent_info["classes"]
+    info["classes"] += parent_info["classes"]
     info["classes"].uniq!
   end
 
   # And inherit parameters from our parent, while preferring our own values.
-  if pparams = parent_info["parameters"]
+  if parent_info["parameters"]
     # When using Hash#merge, the hash being merged in wins, and we
     # want the subnode parameters to be the parent node parameters.
-    info["parameters"] = pparams.merge(info["parameters"])
+    info["parameters"] = parent_info["parameters"].merge(info["parameters"])
   end
 
   # Copy over any parent node name.
-  if pparent = parent_info["parent"]
-    info["parent"] = pparent
+  if parent_info["parent"]
+    info["parent"] = parent_info["parent"]
   end
 end
 

lib/hiera_puppet.rb

@@ -58,7 +58,8 @@ def hiera
   def hiera_config
     config = {}
 
-    if config_file = hiera_config_file
+    config_file = hiera_config_file
+    if config_file
       config = Hiera::Config.load(config_file)
     end
 

lib/puppet/application/apply.rb

@@ -349,9 +349,8 @@ def get_facts()
     facts = nil
     unless Puppet[:node_name_fact].empty?
       # Collect our facts.
-      unless facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value])
-        raise _("Could not find facts for %{node}") % { node: Puppet[:node_name_value] }
-      end
+      facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value])
+      raise _("Could not find facts for %{node}") % { node: Puppet[:node_name_value] } unless facts
 
       Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]]
       facts.name = Puppet[:node_name_value]

lib/puppet/application/doc.rb

@@ -156,7 +156,8 @@ def other
     exit_code = 0
     require 'puppet/util/reference'
     options[:references].sort { |a,b| a.to_s <=> b.to_s }.each do |name|
-      raise _("Could not find reference %{name}") % { name: name } unless section = Puppet::Util::Reference.reference(name)
+      section = Puppet::Util::Reference.reference(name)
+      raise _("Could not find reference %{name}") % { name: name } unless section
 
       begin
         # Add the per-section text, but with no ToC

lib/puppet/application/face_base.rb

@@ -74,32 +74,38 @@ def parse_options
     index       = -1
     until action_name or (index += 1) >= command_line.args.length do
       item = command_line.args[index]
-      if item =~ /^-/ then
+      if item =~ /^-/
         option = @face.options.find do |name|
           item =~ /^-+#{name.to_s.gsub(/[-_]/, '[-_]')}(?:[ =].*)?$/
         end
-        if option then
+        if option
           option = @face.get_option(option)
           # If we have an inline argument, just carry on.  We don't need to
           # care about optional vs mandatory in that case because we do a real
           # parse later, and that will totally take care of raising the error
           # when we get there. --daniel 2011-04-04
-          if option.takes_argument? and !item.index('=') then
+          if option.takes_argument? and !item.index('=')
             index += 1 unless
               (option.optional_argument? and command_line.args[index + 1] =~ /^-/)
           end
-        elsif option = find_global_settings_argument(item) then
-          unless Puppet.settings.boolean? option.name then
-            # As far as I can tell, we treat non-bool options as always having
-            # a mandatory argument. --daniel 2011-04-05
-            # ... But, the mandatory argument will not be the next item if an = is
-            # employed in the long form of the option. --jeffmccune 2012-09-18
-            index += 1 unless item =~ /^--#{option.name}=/
-          end
-        elsif option = find_application_argument(item) then
-          index += 1 if (option[:argument] and not option[:optional])
         else
-          raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
+          option = find_global_settings_argument(item)
+          if option
+            unless Puppet.settings.boolean? option.name
+              # As far as I can tell, we treat non-bool options as always having
+              # a mandatory argument. --daniel 2011-04-05
+              # ... But, the mandatory argument will not be the next item if an = is
+              # employed in the long form of the option. --jeffmccune 2012-09-18
+              index += 1 unless item =~ /^--#{option.name}=/
+            end
+          else 
+            option = find_application_argument(item)
+            if option
+              index += 1 if (option[:argument] and not option[:optional])
+            else
+              raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
+            end
+          end
         end
       else
         # Stash away the requested action name for later, and try to fetch the
@@ -112,7 +118,8 @@ def parse_options
     end
 
     if @action.nil?
-      if @action = @face.get_default_action() then
+      @action = @face.get_default_action()
+      if @action
         @is_default_action = true
       else
         # First try to handle global command line options

lib/puppet/application/script.rb

@@ -140,18 +140,16 @@ def main
 
     unless Puppet[:node_name_fact].empty?
       # Collect the facts specified for that node
-      unless facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value])
-        raise _("Could not find facts for %{node}") % { node: Puppet[:node_name_value] }
-      end
+      facts = Puppet::Node::Facts.indirection.find(Puppet[:node_name_value])
+      raise _("Could not find facts for %{node}") % { node: Puppet[:node_name_value] } unless facts
 
       Puppet[:node_name_value] = facts.values[Puppet[:node_name_fact]]
       facts.name = Puppet[:node_name_value]
     end
 
     # Find the Node
-    unless node = Puppet::Node.indirection.find(Puppet[:node_name_value])
-      raise _("Could not find node %{node}") % { node: Puppet[:node_name_value] }
-    end
+    node = Puppet::Node.indirection.find(Puppet[:node_name_value])
+    raise _("Could not find node %{node}") % { node: Puppet[:node_name_value] } unless node
 
     configured_environment = node.environment || Puppet.lookup(:current_environment)
 

lib/puppet/configurer.rb

@@ -67,7 +67,8 @@ def initialize(transaction_uuid = nil, job_id = nil)
   # Get the remote catalog, yo.  Returns nil if no catalog can be found.
   def retrieve_catalog(query_options)
     query_options ||= {}
-    if (Puppet[:use_cached_catalog] && result = retrieve_catalog_from_cache(query_options))
+    result = retrieve_catalog_from_cache(query_options) if Puppet[:use_cached_catalog]
+    if result
       @cached_catalog_status = 'explicitly_requested'
 
       Puppet.info _("Using cached catalog from environment '%{environment}'") % { environment: result.environment }
@@ -104,7 +105,8 @@ def convert_catalog(result, duration, options = {})
 
     catalog_conversion_time = thinmark do
       # Will mutate the result and replace all Deferred values with resolved values
-      if facts = options[:convert_with_facts]
+      facts = options[:convert_with_facts]
+      if facts
         Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(facts, result)
       end
 
@@ -257,7 +259,8 @@ def run_internal(options)
       Puppet::GettextConfig.reset_text_domain('agent')
       Puppet::ModuleTranslations.load_from_vardir(Puppet[:vardir])
 
-      if catalog = prepare_and_retrieve_catalog_from_cache(options)
+      catalog = prepare_and_retrieve_catalog_from_cache(options)
+      if catalog
         options[:catalog] = catalog
         @cached_catalog_status = 'explicitly_requested'
 
@@ -339,7 +342,8 @@ def run_internal(options)
       query_options[:configured_environment] = configured_environment
       options[:convert_for_node] = node
 
-      unless catalog = prepare_and_retrieve_catalog(options, query_options)
+      catalog = prepare_and_retrieve_catalog(options, query_options)
+      unless catalog
         return nil
       end
 
@@ -364,7 +368,8 @@ def run_internal(options)
         query_options = get_facts(options)
         query_options[:configured_environment] = configured_environment
 
-        return nil unless catalog = prepare_and_retrieve_catalog(options, query_options)
+        catalog = prepare_and_retrieve_catalog(options, query_options)
+        return nil unless catalog
         tries += 1
       end
 

lib/puppet/confine_collection.rb

@@ -11,7 +11,8 @@ def confine(hash)
       for_binary = false
     end
     hash.each do |test, values|
-      if klass = Puppet::Confine.test(test)
+      klass = Puppet::Confine.test(test)
+      if klass
         @confines << klass.new(values)
         @confines[-1].for_binary = true if for_binary
       else

lib/puppet/daemon.rb

@@ -38,7 +38,8 @@ def daemonname
 
   # Put the daemon into the background.
   def daemonize
-    if pid = fork
+    pid = fork
+    if pid
       Process.detach(pid)
       exit(0)
     end
@@ -181,7 +182,7 @@ def run_event_loop
     end
 
     signal_loop = Puppet::Scheduler.create_job(SIGNAL_CHECK_INTERVAL) do
-      while method = @signals.shift
+      while method = @signals.shift #rubocop:disable Lint/AssignmentInCondition
         Puppet.notice "Processing #{method}"
         send(method)
       end

lib/puppet/environments.rb

@@ -268,7 +268,8 @@ def list
     # @!macro loader_get
     def get(name)
       @loaders.each do |loader|
-        if env = loader.get(name)
+        env = loader.get(name)
+        if env 
           return env
         end
       end
@@ -278,7 +279,8 @@ def get(name)
     # @!macro loader_get_conf
     def get_conf(name)
       @loaders.each do |loader|
-        if conf = loader.get_conf(name)
+        conf = loader.get_conf(name)
+        if conf
           return conf
         end
       end
@@ -349,7 +351,8 @@ def get(name)
       # This strategy favors smaller memory footprint over environment
       # retrieval time.
       clear_all_expired
-      if result = @cache[name]
+      result = @cache[name]
+      if result
         # found in cache
         return result.value
       elsif (result = @loader.get(name))

lib/puppet/etc.rb

@@ -115,7 +115,7 @@ def group
 
       setgrent
       begin
-        while cur_group = getgrent
+        while cur_group = getgrent #rubocop:disable Lint/AssignmentInCondition
           yield cur_group
         end
       ensure

lib/puppet/external/pson/pure/parser.rb

@@ -180,7 +180,8 @@ def parse_string
         if scan(STRING)
           return '' if self[1].empty?
           string = self[1].gsub(%r{(?:\\[\\bfnrt"/]|(?:\\u(?:[A-Fa-f\d]{4}))+|\\[\x20-\xff])}n) do |c|
-            if u = UNESCAPE_MAP[$MATCH[1]]
+            u = UNESCAPE_MAP[$MATCH[1]]
+            if u
               u
             else # \uXXXX
               bytes = ''

lib/puppet/face/epp.rb

@@ -402,7 +402,8 @@ def dump_parse(source, filename, options, show_filename = true)
 
   def get_values(compiler, options)
     template_values = nil
-    if values_file = options[:values_file]
+    values_file = options[:values_file]
+    if values_file
       begin
         if values_file =~ /\.yaml$/
           template_values = Puppet::Util::Yaml.safe_load_file(values_file, [Symbol])
@@ -420,7 +421,8 @@ def get_values(compiler, options)
       end
     end
 
-    if values = options[:values]
+    values = options[:values]
+    if values
       evaluating_parser = Puppet::Pops::Parser::EvaluatingParser.new
       result = evaluating_parser.evaluate_string(compiler.topscope, values, 'values-hash')
       case result

lib/puppet/face/help.rb

@@ -182,8 +182,9 @@ 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
-      if md = /^puppet-#{appname}\([^\)]+\) -- (.*)$/.match(line)
+    while line = help.shift do #rubocop:disable Lint/AssignmentInCondition
+      md = /^puppet-#{appname}\([^\)]+\) -- (.*)$/.match(line)
+      if md
         return md[1]
       end
     end

lib/puppet/face/module/changes.rb

@@ -22,7 +22,8 @@
 
     when_invoked do |path, options|
       Puppet::ModuleTool.set_option_defaults options
-      unless root_path = Puppet::ModuleTool.find_module_root(path)
+      root_path = Puppet::ModuleTool.find_module_root(path)
+      unless root_path
         raise ArgumentError, _("Could not find a valid module at %{path}") % { path: path.inspect }
       end
       Puppet::ModuleTool::Applications::Checksummer.run(root_path, options)

lib/puppet/file_bucket/dipper.rb

@@ -126,7 +126,8 @@ def restore(file, sum)
     end
 
     if restore
-      if newcontents = get_bucket_file(sum)
+      newcontents = get_bucket_file(sum)
+      if newcontents
         newsum = newcontents.checksum_data
         changed = nil
         if Puppet::FileSystem.exist?(file_handle) and ! Puppet::FileSystem.writable?(file_handle)

lib/puppet/file_serving/configuration.rb

@@ -55,7 +55,8 @@ def split_path(request)
     raise(ArgumentError, _("Cannot find file: Invalid mount '%{mount_name}'") % { mount_name: mount_name }) unless mount_name =~ %r{^[-\w]+$}
     raise(ArgumentError, _("Cannot find file: Invalid relative path '%{path}'") % { path: path }) if path and path.split('/').include?('..')
 
-    return nil unless mount = find_mount(mount_name, request.environment)
+    mount = find_mount(mount_name, request.environment)
+    return nil unless mount
     if mount.name == "modules" and mount_name != "modules"
       # yay backward-compatibility
       path = "#{mount_name}/#{path}"

lib/puppet/file_serving/fileset.rb

@@ -148,7 +148,7 @@ def perform_recursion
 
     result = []
 
-    while entry = current_dirs.shift
+    while entry = current_dirs.shift #rubocop:disable Lint/AssignmentInCondition
       if continue_recursion_at?(entry.depth + 1)
         entry.children.each do |child|
           result << child.path

lib/puppet/file_serving/http_metadata.rb

@@ -15,13 +15,15 @@ def initialize(http_response, path = '/dev/null')
     # use a default mtime in case there is no usable HTTP header
     @checksums[:mtime] = "{mtime}#{Time.now}"
 
-    if checksum = http_response['content-md5']
+    checksum = http_response['content-md5']
+    if checksum
       # convert base64 digest to hex
       checksum = checksum.unpack("m0").first.unpack("H*").first
       @checksums[:md5] = "{md5}#{checksum}"
     end
 
-    if last_modified = http_response['last-modified']
+    last_modified = http_response['last-modified']
+    if last_modified
       mtime = DateTime.httpdate(last_modified).to_time
       @checksums[:mtime] = "{mtime}#{mtime.utc}"
     end