lint

Author: Zarne

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2023-08-17 21:26:09 UTC using RuboCop version 1.50.2.
+# on 2024-08-06 17:00:26 UTC using RuboCop version 1.50.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -12,6 +12,44 @@ Lint/NonAtomicFileOperation:
   Exclude:
     - 'lib/puppet/provider/archive/ruby.rb'
 
+# Offense count: 1
+Naming/AccessorMethodName:
+  Exclude:
+    - 'lib/puppet/type/archive.rb'
+
+# Offense count: 1
+# Configuration parameters: MinNameLength, AllowNamesEndingInNumbers, AllowedNames, ForbiddenNames.
+# AllowedNames: as, at, by, cc, db, id, if, in, io, ip, of, on, os, pp, to
+Naming/MethodParameterName:
+  Exclude:
+    - 'spec/lib/puppet_spec/files.rb'
+
+# Offense count: 9
+# Configuration parameters: AssignmentOnly.
+RSpec/InstanceVariable:
+  Exclude:
+    - 'spec/unit/puppet/provider/archive/ruby_spec.rb'
+    - 'spec/unit/puppet/type/archive_spec.rb'
+
+# Offense count: 1
+# Configuration parameters: EnforcedStyle.
+# SupportedStyles: have_received, receive
+RSpec/MessageSpies:
+  Exclude:
+    - 'spec/lib/puppet_spec/compiler.rb'
+
+# Offense count: 4
+# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
+RSpec/VerifiedDoubles:
+  Exclude:
+    - 'spec/unit/puppet/type/archive_spec.rb'
+
+# Offense count: 4
+# Configuration parameters: AllowedVariables.
+Style/GlobalVars:
+  Exclude:
+    - 'spec/lib/puppet_spec/files.rb'
+
 # Offense count: 1
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: AllowComments.

Gemfile

@@ -23,7 +23,6 @@ group :release do
   gem 'voxpupuli-release', '~> 3.0',  :require => false
 end
 
-gem 'pry'
 gem 'rake', :require => false
 gem 'facter', ENV['FACTER_GEM_VERSION'], :require => false, :groups => [:test]
 

lib/puppet/provider/archive/ruby.rb

@@ -256,33 +256,32 @@ def checkexe(command)
     if Facter.value(:osfamily) == 'windows'
       if absolute_path?(exe)
         if !Puppet::FileSystem.exist?(exe)
-          raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe }
+          raise ArgumentError, format(_("Could not find command '%{exe}'"), exe: exe)
         elsif !File.file?(exe)
-          raise ArgumentError, _("'%{exe}' is a %{klass}, not a file") % { exe: exe, klass: File.ftype(exe) }
+          raise ArgumentError, format(_("'%{exe}' is a %{klass}, not a file"), exe: exe, klass: File.ftype(exe))
         end
       end
-    else
-      if File.expand_path(exe) == exe
-        if !Puppet::FileSystem.exist?(exe)
-          raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe }
-        elsif !File.file?(exe)
-          raise ArgumentError, _("'%{exe}' is a %{klass}, not a file") % { exe: exe, klass: File.ftype(exe) }
-        elsif !File.executable?(exe)
-          raise ArgumentError, _("'%{exe}' is not executable") % { exe: exe }
-        end
+    elsif File.expand_path(exe) == exe
+      if !Puppet::FileSystem.exist?(exe)
+        raise ArgumentError, format(_("Could not find command '%{exe}'"), exe: exe)
+      elsif !File.file?(exe)
+        raise ArgumentError, format(_("'%{exe}' is a %{klass}, not a file"), exe: exe, klass: File.ftype(exe))
+      elsif !File.executable?(exe)
+        raise ArgumentError, format(_("'%{exe}' is not executable"), exe: exe)
       end
     end
 
     if resource[:env_path]
-      Puppet::Util.withenv :PATH => resource[:env_path].join(File::PATH_SEPARATOR) do
+      Puppet::Util.withenv PATH: resource[:env_path].join(File::PATH_SEPARATOR) do
         return if which(exe)
       end
     end
 
     # 'which' will only return the command if it's executable, so we can't
     # distinguish not found from not executable
-    raise ArgumentError, _("Could not find command '%{exe}'") % { exe: exe }
+    raise ArgumentError, format(_("Could not find command '%{exe}'"), exe: exe)
   end
+
   def environment
     env = {}
 
@@ -294,19 +293,17 @@ def environment
 
     envlist = [envlist] unless envlist.is_a? Array
     envlist.each do |setting|
-      unless (match = /^(\w+)=((.|\n)*)$/.match(setting))
-        warning _("Cannot understand environment setting %{setting}") % { setting: setting.inspect }
+      unless (match = %r{^(\w+)=((.|\n)*)$}.match(setting))
+        warning format(_('Cannot understand environment setting %{setting}'), setting: setting.inspect)
         next
       end
       var = match[1]
       value = match[2]
 
-      if env.include?(var) || env.include?(var.to_sym)
-        warning _("Overriding environment setting '%{var}' with '%{value}'") % { var: var, value: value }
-      end
+      warning format(_("Overriding environment setting '%{var}' with '%{value}'"), var: var, value: value) if env.include?(var) || env.include?(var.to_sym)
 
       if value.nil? || value.empty?
-        msg = _("Empty environment setting '%{var}'") % { var: var }
+        msg = format(_("Empty environment setting '%{var}'"), var: var)
         Puppet.warn_once('undefined_variables', "empty_env_var_#{var}", msg, resource.file, resource.line)
       end
 
@@ -317,10 +314,9 @@ def environment
   end
 
   def run(command, check = false)
-    output = nil
     checkexe(command)
 
-    debug "Executing#{check ? " check": ""} #{command}"
+    debug "Executing#{check ? ' check' : ''} #{command}"
 
     cwd = resource[:extract] ? resource[:extract_path] : File.dirname(resource[:path])
     # It's ok if cwd is nil. In that case Puppet::Util::Execution.execute() simply will not attempt to
@@ -329,7 +325,7 @@ def run(command, check = false)
     # the working directory can fail under some circumstances, so avoiding the directory change attempt
     # is preferable to defaulting cwd to that directory.
 
-    # note that we are passing "false" for the "override_locale" parameter, which ensures that the user's
+    # NOTE: that we are passing "false" for the "override_locale" parameter, which ensures that the user's
     # default/system locale will be respected.  Callers may override this behavior by setting locale-related
     # environment variables (LANG, LC_ALL, etc.) in their 'environment' configuration.
     output = Puppet::Util::Execution.execute(
@@ -344,9 +340,8 @@ def run(command, check = false)
       sensitive: false
     )
     # The shell returns 127 if the command is missing.
-    if output.exitstatus == 127
-      raise ArgumentError, output
-    end
+    raise ArgumentError, output if output.exitstatus == 127
+
     # Return output twice as processstatus was returned before, but only exitstatus was ever called.
     # Output has the exitstatus on it so it is returned instead. This is here twice as changing this
     #  would result in a change to the underlying API.
@@ -357,19 +352,19 @@ def extractexe(command)
     if command.is_a? Array
       command.first
     else
-      match = /^"([^"]+)"|^'([^']+)'/.match(command)
+      match = %r{^"([^"]+)"|^'([^']+)'}.match(command)
       if match
         # extract whichever of the two sides matched the content.
         match[1] or match[2]
       else
-        command.split(/ /)[0]
+        command.split(%r{ })[0]
       end
     end
   end
 
   def validatecmd(command)
     exe = extractexe(command)
     # if we're not fully qualified, require a path
-    self.fail _("'%{exe}' is not qualified and no path was specified. Please qualify the command or specify a path.") % { exe: exe } if !absolute_path?(exe) and resource[:path].nil?
+    self.fail format(_("'%{exe}' is not qualified and no path was specified. Please qualify the command or specify a path."), exe: exe) if !absolute_path?(exe) && resource[:path].nil?
   end
 end

lib/puppet/type/archive.rb

@@ -4,7 +4,6 @@
 require 'uri'
 require 'puppet/util'
 require 'puppet/parameter/boolean'
-require 'pry'
 
 Puppet::Type.newtype(:archive) do
   @doc = 'Manage archive file download, extraction, and cleanup.'
@@ -264,9 +263,9 @@ def change_to_s(currentvalue, newvalue)
 
     # Support both arrays and colon-separated fields.
     def value=(*values)
-      @value = values.flatten.collect { |val|
+      @value = values.flatten.map do |val|
         val.split(File::PATH_SEPARATOR)
-      }.flatten
+      end.flatten
     end
   end
 
@@ -280,29 +279,27 @@ def value=(*values)
     validate do |values|
       values = [values] unless values.is_a? Array
       values.each do |value|
-        unless value =~ /\w+=/
-          raise ArgumentError, _("Invalid environment setting '%{value}'") % { value: value }
-        end
+        raise ArgumentError, "Invalid environment setting '#{value}'" unless value =~ %r{\w+=}
       end
     end
   end
 
-  newcheck(:creates, :parent => Puppet::Parameter::Path) do
+  newcheck(:creates, parent: Puppet::Parameter::Path) do
     desc 'if file/directory exists, will not download/extract archive.'
 
     accept_arrays
 
     # If the file exists, return false (i.e., don't run the command),
     # else return true
     def check(value)
-      #TRANSLATORS 'creates' is a parameter name and should not be translated
-      debug(_("Checking that 'creates' path '%{creates_path}' exists") % { creates_path: value })
+      # TRANSLATORS 'creates' is a parameter name and should not be translated
+      debug("Checking that 'creates' path '#{value}' exists")
       Puppet::FileSystem.exist?(value)
     end
   end
 
   newcheck(:unless) do
-    desc <<-'EOT'
+    desc <<-EOT
         A test command that checks the state of the target system and restricts
         when the `archive` can run. If present, Puppet runs this test command
         first, then runs the main command unless the test has an exit code of 0
@@ -333,7 +330,7 @@ def check(value)
 
         This `archive` would only run if every command in the array has a
         non-zero exit code.
-      EOT
+    EOT
 
     validate do |cmds|
       cmds = [cmds] unless cmds.is_a? Array
@@ -348,24 +345,24 @@ def check(value)
       begin
         output, status = provider.run(value, true)
       rescue Timeout::Error
-        err _("Check %{value} exceeded timeout") % { value: value.inspect }
+        err format('Check %{value} exceeded timeout', value: value.inspect)
         return false
       end
 
       if sensitive
-        self.debug("[output redacted]")
+        debug('[output redacted]')
       else
-        output.split(/\n/).each { |line|
-          self.debug(line)
-        }
+        output.split(%r{\n}).each do |line|
+          debug(line)
+        end
       end
 
       status.exitstatus != 0
     end
   end
 
   newcheck(:onlyif) do
-    desc <<-'EOT'
+    desc <<-EOT
         A test command that checks the state of the target system and restricts
         when the `archive` can run. If present, Puppet runs this test command
         first, and only runs the main command if the test has an exit code of 0
@@ -396,7 +393,7 @@ def check(value)
 
         This `archive` would only run if every command in the array has an
         exit code of 0 (success).
-      EOT
+    EOT
 
     validate do |cmds|
       cmds = [cmds] unless cmds.is_a? Array
@@ -411,19 +408,19 @@ def check(value)
       begin
         output, status = provider.run(value, true)
       rescue Timeout::Error
-        err _("Check %{value} exceeded timeout") % { value: value.inspect }
+        err format('Check %{value} exceeded timeout', value: value.inspect)
         return false
       end
 
       if sensitive
-        self.debug("[output redacted]")
+        debug('[output redacted]')
       else
-        output.split(/\n/).each { |line|
-          self.debug(line)
-        }
+        output.split(%r{\n}).each do |line|
+          debug(line)
+        end
       end
 
-      status.exitstatus == 0
+      status.exitstatus.zero?
     end
   end
 
@@ -460,9 +457,8 @@ def check(value)
   # Verify that we pass all of the checks.  The argument determines whether
   # we skip the :refreshonly check, which is necessary because we now check
   # within refresh
-  def check_all_attributes(refreshing = false)
-    self.class.checks.each { |check|
-
+  def check_all_attributes(_refreshing = false)
+    self.class.checks.each do |check|
       next unless @parameters.include?(check)
 
       val = @parameters[check].value
@@ -472,13 +468,13 @@ def check_all_attributes(refreshing = false)
 
         # Give a debug message so users can figure out what command would have been
         # but don't print sensitive commands or parameters in the clear
-        sourcestring = @parameters[:source].sensitive ? "[command redacted]" : @parameters[:source].value
+        sourcestring = @parameters[:source].sensitive ? '[command redacted]' : @parameters[:source].value
 
-        debug(_("'%{source}' won't be executed because of failed check '%{check}'") % { source: sourcestring, check: check })
+        debug(format("'%{source}' won't be executed because of failed check '%{check}'", source: sourcestring, check: check))
 
         return false
       end
-    }
+    end
     true
   end
 
@@ -492,13 +488,13 @@ def output
 
   # Run the command, or optionally run a separately-specified command.
   def refresh
-    if check_all_attributes(true)
-      cmd = self[:refresh]
-      if cmd
-        provider.run(cmd)
-      else
-        property(:returns).sync
-      end
+    return unless check_all_attributes(true)
+
+    cmd = self[:refresh]
+    if cmd
+      provider.run(cmd)
+    else
+      property(:returns).sync
     end
   end
 
@@ -507,7 +503,7 @@ def refresh
   def set_sensitive_parameters(sensitive_parameters)
     # If any are sensitive, mark all as sensitive
     sensitive = false
-    parameters_to_check = [:command, :unless, :onlyif]
+    parameters_to_check = %i[command unless onlyif]
 
     parameters_to_check.each do |p|
       if sensitive_parameters.include?(p)
@@ -518,9 +514,7 @@ def set_sensitive_parameters(sensitive_parameters)
 
     if sensitive
       parameters_to_check.each do |p|
-        if parameters.include?(p)
-          parameter(p).sensitive = true
-        end
+        parameter(p).sensitive = true if parameters.include?(p)
       end
     end