Merge pull request #9227 from tvpartytonight/PUP-11767

(PUP-11767) Add Style Cops

.rubocop.yml

@@ -199,3 +199,12 @@ RSpec/FactoryBot/SyntaxMethods: # new in 2.7
 
 RSpec/Rails/AvoidSetupHook: # new in 2.4
   Enabled: true
+
+Style/AutoResourceCleanup:
+  Enabled: true
+
+Style/CaseEquality:
+  Enabled: true
+
+Style/CaseLikeIf:
+  Enabled: true

lib/hiera/scope.rb

@@ -20,11 +20,12 @@ def initialize(real)
     end
 
     def [](key)
-      if key == CALLING_CLASS
+      case key
+      when CALLING_CLASS
         ans = find_hostclass(@real)
-      elsif key == CALLING_CLASS_PATH
+      when CALLING_CLASS_PATH
         ans = find_hostclass(@real).gsub(/::/, '/')
-      elsif key == CALLING_MODULE
+      when CALLING_MODULE
         ans = safe_lookupvar(MODULE_NAME)
       else
         ans = safe_lookupvar(key)

lib/puppet/etc.rb

@@ -165,10 +165,11 @@ def override_field_values_to_utf8(struct)
 
       new_struct = struct.is_a?(Etc::Passwd) ? puppet_etc_passwd_class.new : puppet_etc_group_class.new
       struct.each_pair do |member, value|
-        if value.is_a?(String)
+        case value
+        when String
           new_struct["canonical_#{member}".to_sym] = value.dup
           new_struct[member] = Puppet::Util::CharacterEncoding.override_encoding_to_utf_8(value).scrub
-        elsif value.is_a?(Array)
+        when Array
           new_struct["canonical_#{member}".to_sym] = value.inject([]) { |acc, elem| acc << elem.dup }
           new_struct[member] = value.inject([]) do |acc, elem|
             acc << Puppet::Util::CharacterEncoding.override_encoding_to_utf_8(elem).scrub

lib/puppet/face/epp.rb

@@ -406,9 +406,10 @@ def get_values(compiler, options)
     values_file = options[:values_file]
     if values_file
       begin
-        if values_file =~ /\.yaml$/
+        case values_file
+        when /\.yaml$/
           template_values = Puppet::Util::Yaml.safe_load_file(values_file, [Symbol])
-        elsif values_file =~ /\.pp$/
+        when /\.pp$/
           evaluating_parser = Puppet::Pops::Parser::EvaluatingParser.new
           template_values = evaluating_parser.evaluate_file(compiler.topscope, values_file)
         else

lib/puppet/face/module/install.rb

@@ -131,10 +131,11 @@
     end
 
     when_rendering :console do |return_value, name, _options|
-      if return_value[:result] == :noop
+      case return_value[:result]
+      when :noop
         Puppet.notice _("Module %{name} %{version} is already installed.") % { name: name, version: return_value[:version] }
         exit 0
-      elsif return_value[:result] == :failure
+      when :failure
         Puppet.err(return_value[:error][:multiline])
         exit 1
       else

lib/puppet/face/module/upgrade.rb

@@ -71,10 +71,11 @@
     end
 
     when_rendering :console do |return_value|
-      if return_value[:result] == :noop
+      case return_value[:result]
+      when :noop
         Puppet.notice return_value[:error][:multiline]
         exit 0
-      elsif return_value[:result] == :failure
+      when :failure
         Puppet.err(return_value[:error][:multiline])
         exit 1
       else

lib/puppet/functions/abs.rb

@@ -52,9 +52,10 @@ def on_string(x)
     # These patterns for conversion are backwards compatible with the stdlib
     # version of this function.
     #
-    if x =~ %r{^-?(?:\d+)(?:\.\d+){1}$}
+    case x
+    when %r{^-?(?:\d+)(?:\.\d+){1}$}
       x.to_f.abs
-    elsif x =~ %r{^-?\d+$}
+    when %r{^-?\d+$}
       x.to_i.abs
     else
       raise(ArgumentError, 'abs(): Requires float or integer to work with - was given non decimal string')

lib/puppet/indirector/catalog/compiler.rb

@@ -116,11 +116,12 @@ def require_environment?
   # @return [Puppet::Node::Facts] facts object deserialized from supplied string
   # @api private
   def convert_wire_facts(facts, format)
-    if format == 'pson'
+    case format
+    when 'pson'
       # We unescape here because the corresponding code in Puppet::Configurer::FactHandler encodes with Puppet::Util.uri_query_encode
       # PSON is deprecated, but continue to accept from older agents
       return Puppet::Node::Facts.convert_from('pson', CGI.unescape(facts))
-    elsif format == 'application/json'
+    when 'application/json'
       return Puppet::Node::Facts.convert_from('json', CGI.unescape(facts))
     else
       raise ArgumentError, _("Unsupported facts format")

lib/puppet/network/formats.rb

@@ -193,11 +193,12 @@ def render_multiple(data)
                                       :weight => 0) do
   def flatten_hash(hash)
     hash.each_with_object({}) do |(k, v), h|
-      if v.is_a? Hash
+      case v
+      when Hash
         flatten_hash(v).map do |h_k, h_v|
           h["#{k}.#{h_k}"] = h_v
         end
-      elsif v.is_a? Array
+      when Array
         v.each_with_index do |el, i|
           if el.is_a? Hash
             flatten_hash(el).map do |el_k, el_v|
@@ -240,10 +241,11 @@ def render(datum)
     return datum if datum.is_a?(String) || datum.is_a?(Numeric)
 
     # Simple hash
-    if datum.is_a?(Hash)
+    case datum
+    when Hash
       data = flatten_hash(datum)
       return construct_output(data)
-    elsif datum.is_a?(Array)
+    when Array
       data = flatten_array(datum)
       return construct_output(data)
     end

lib/puppet/node/facts.rb

@@ -138,16 +138,14 @@ def quote_special_strings(fact_hash)
   end
 
   def sanitize_fact(fact)
-    if fact.is_a? Hash then
+    case fact
+    when Hash
       ret = {}
       fact.each_pair { |k, v| ret[sanitize_fact k] = sanitize_fact v }
       ret
-    elsif fact.is_a? Array then
+    when Array
       fact.collect { |i| sanitize_fact i }
-    elsif fact.is_a? Numeric \
-      or fact.is_a? TrueClass \
-      or fact.is_a? FalseClass \
-      or fact.is_a? String
+    when Numeric, TrueClass, FalseClass, String
       fact
     else
       result = fact.to_s

lib/puppet/parser/scope.rb

@@ -743,11 +743,12 @@ def merge_settings(env_name, set_in_this_scope = true)
   end
 
   def transform_setting(val)
-    if val.is_a?(String) || val.is_a?(Numeric) || true == val || false == val || nil == val
+    case val
+    when String, Numeric, true, false, nil
       val
-    elsif val.is_a?(Array)
+    when Array
       val.map { |entry| transform_setting(entry) }
-    elsif val.is_a?(Hash)
+    when Hash
       result = {}
       val.each { |k, v| result[transform_setting(k)] = transform_setting(v) }
       result

lib/puppet/pops/evaluator/compare_operator.rb

@@ -61,9 +61,10 @@ def equals_String(a, b)
   end
 
   def cmp_Numeric(a, b)
-    if b.is_a?(Numeric)
+    case b
+    when Numeric
       a <=> b
-    elsif b.is_a?(Time::Timespan) || b.is_a?(Time::Timestamp)
+    when Time::Timespan, Time::Timestamp
       -(b <=> a) # compare other way and invert result
     else
       raise ArgumentError.new(_("A Numeric is not comparable to non Numeric"))
@@ -218,9 +219,10 @@ def match_Regexp(regexp, left, scope)
 
   # Matches against semvers and strings
   def match_Version(version, left, scope)
-    if left.is_a?(SemanticPuppet::Version)
+    case left
+    when SemanticPuppet::Version
       version == left
-    elsif left.is_a? String
+    when String
       begin
         version == SemanticPuppet::Version.parse(left)
       rescue ArgumentError

lib/puppet/pops/evaluator/deferred_resolver.rb

@@ -92,12 +92,13 @@ def resolve_futures(catalog)
         # If the value is instance of Sensitive - assign the unwrapped value
         # and mark it as sensitive if not already marked
         #
-        if resolved.is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)
+        case resolved
+        when Puppet::Pops::Types::PSensitiveType::Sensitive
           resolved = resolved.unwrap
           mark_sensitive_parameters(r, k)
         # If the value is a DeferredValue and it has an argument of type PSensitiveType, mark it as sensitive
         # The DeferredValue.resolve method will unwrap it during catalog application
-        elsif resolved.is_a?(Puppet::Pops::Evaluator::DeferredValue)
+        when Puppet::Pops::Evaluator::DeferredValue
           if v.arguments.any? { |arg| arg.is_a?(Puppet::Pops::Types::PSensitiveType) }
             mark_sensitive_parameters(r, k)
           end
@@ -136,15 +137,16 @@ def resolve(x)
   end
 
   def resolve_lazy_args(x)
-    if x.is_a?(DeferredValue)
+    case x
+    when DeferredValue
       x.resolve
-    elsif x.is_a?(Array)
+    when Array
       x.map { |v| resolve_lazy_args(v) }
-    elsif x.is_a?(Hash)
+    when Hash
       result = {}
       x.each_pair { |k, v| result[k] = resolve_lazy_args(v) }
       result
-    elsif x.is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)
+    when Puppet::Pops::Types::PSensitiveType::Sensitive
       # rewrap in a new Sensitive after resolving any nested deferred values
       Puppet::Pops::Types::PSensitiveType::Sensitive.new(resolve_lazy_args(x.unwrap))
     else

lib/puppet/pops/evaluator/evaluator_impl.rb

@@ -225,13 +225,14 @@ def assign_Object(name, value, o, scope)
   end
 
   def assign_Array(lvalues, values, o, scope)
-    if values.is_a?(Hash)
+    case values
+    when Hash
       lvalues.map do |lval|
         assign(lval,
                values.fetch(lval) { |k| fail(Issues::MISSING_MULTI_ASSIGNMENT_KEY, o, :key => k) },
                o, scope)
       end
-    elsif values.is_a?(Puppet::Pops::Types::PClassType)
+    when Puppet::Pops::Types::PClassType
       if Puppet[:tasks]
         fail(Issues::CATALOG_OPERATION_NOT_SUPPORTED_WHEN_SCRIPTING, o, { :operation => _('multi var assignment from class') })
       end

lib/puppet/pops/evaluator/runtime3_converter.rb

@@ -206,11 +206,12 @@ def convert_Timestamp(o, scope, undef_value)
   # Converts result back to 4.x by replacing :undef with nil in Array and Hash objects
   #
   def self.convert_return(val3x)
-    if val3x == :undef
+    case val3x
+    when :undef
       nil
-    elsif val3x.is_a?(Array)
+    when Array
       val3x.map { |v| convert_return(v) }
-    elsif val3x.is_a?(Hash)
+    when Hash
       hsh = {}
       val3x.each_pair { |k, v| hsh[convert_return(k)] = convert_return(v) }
       hsh

lib/puppet/pops/lookup/hiera_config.rb

@@ -232,13 +232,14 @@ def find_line_matching(regexp, start_line = 1)
       quote = nil
       stripped = ''.dup
       line.each_codepoint do |cp|
-        if cp == 0x22 || cp == 0x27 # double or single quote
+        case cp
+        when 0x22, 0x27 # double or single quote
           if quote == cp
             quote = nil
           elsif quote.nil?
             quote = cp
           end
-        elsif cp == 0x23 # unquoted hash mark
+        when 0x23 # unquoted hash mark
           break
         end
         stripped << cp

lib/puppet/pops/merge_strategy.rb

@@ -377,11 +377,12 @@ def checked_merge(e1, e2)
     end
 
     def deep_clone(value)
-      if value.is_a?(Hash)
+      case value
+      when Hash
         result = value.clone
         value.each { |k, v| result[k] = deep_clone(v) }
         result
-      elsif value.is_a?(Array)
+      when Array
         value.map { |v| deep_clone(v) }
       else
         value

lib/puppet/pops/parser/lexer2.rb

@@ -254,9 +254,10 @@ def initialize()
       '@' => lambda do
         scn = @scanner
         la = scn.peek(2)
-        if la[1] == '@'
+        case la[1]
+        when '@'
           emit(TOKEN_ATAT, scn.pos) # TODO; Check if this is good for the grammar
-        elsif la[1] == '('
+        when '('
           heredoc
         else
           emit(TOKEN_AT, scn.pos)

lib/puppet/pops/parser/lexer_support.rb

@@ -82,13 +82,14 @@ def create_lex_error(issue, args = {}, pos = nil)
   # An error is raised if the given value does not comply.
   #
   def assert_numeric(value, pos)
-    if value =~ /^0[xX]/
+    case value
+    when /^0[xX]/
       lex_error(Issues::INVALID_HEX_NUMBER, { :value => value }, pos)     unless value =~ /^0[xX][0-9A-Fa-f]+$/
 
-    elsif value =~ /^0[^.]/
+    when /^0[^.]/
       lex_error(Issues::INVALID_OCTAL_NUMBER, { :value => value }, pos)   unless value =~ /^0[0-7]+$/
 
-    elsif value =~ /^\d+[eE.]/
+    when /^\d+[eE.]/
       lex_error(Issues::INVALID_DECIMAL_NUMBER, { :value => value }, pos) unless value =~ /^\d+(?:\.\d+)?(?:[eE]-?\d+)?$/
 
     else

lib/puppet/pops/serialization/from_data_converter.rb

@@ -20,9 +20,10 @@ def []=(key, value)
     def resolve
       unless @resolved
         @resolved = true
-        if @values.is_a?(Array)
+        case @values
+        when Array
           @values.each_with_index { |v, idx| @values[idx] = v.resolve if v.is_a?(Builder) }
-        elsif @values.is_a?(Hash)
+        when Hash
           @values.each_pair { |k, v| @values[k] = v.resolve if v.is_a?(Builder) }
         end
       end
@@ -153,14 +154,15 @@ def initialize(options = EMPTY_HASH)
     #
     # @api public
     def convert(value)
-      if value.is_a?(Hash)
+      case value
+      when Hash
         pcore_type = value[PCORE_TYPE_KEY]
         if pcore_type && (pcore_type.is_a?(String) || pcore_type.is_a?(Hash))
           @pcore_type_procs[pcore_type].call(value, pcore_type)
         else
           build({}) { value.each_pair { |key, elem| with(key) { convert(elem) } } }
         end
-      elsif value.is_a?(Array)
+      when Array
         build([]) { value.each_with_index { |elem, idx| with(idx) { convert(elem) } } }
       else
         build(value)
@@ -207,7 +209,8 @@ def build_object(builder, &block)
     end
 
     def pcore_type_hash_to_value(pcore_type, value)
-      if value.is_a?(Hash)
+      case value
+      when Hash
         # Complex object
         if value.empty?
           build(pcore_type.create)
@@ -216,7 +219,7 @@ def pcore_type_hash_to_value(pcore_type, value)
         else
           build_object(ObjectArrayBuilder.new(pcore_type.allocate)) { value.each_pair { |key, elem| with(key) { convert(elem) } } }
         end
-      elsif value.is_a?(String)
+      when String
         build(pcore_type.create(value))
       else
         raise SerializationError, _('Cannot create a %{type_name} from a %{arg_class}') %