(maint) - rubocop fixes

Author: jordanbreen28

.rubocop.yml

@@ -1,4 +1,6 @@
 ---
+inherit_from: .rubocop_todo.yml
+
 require: rubocop-rspec
 AllCops:
   TargetRubyVersion: '2.5'
@@ -12,7 +14,7 @@ AllCops:
   - pkg/**/*
   - spec/fixtures/**/*
   - vendor/**/*
-Metrics/LineLength:
+Layout/LineLength:
   Description: People have wide screens, use them.
   Max: 200
 RSpec/BeforeAfterAll:
@@ -34,8 +36,7 @@ Style/ClassAndModuleChildren:
   Description: Compact style reduces the required amount of indentation.
   EnforcedStyle: compact
 # this needs to be easily usable in the gemspec
-  Exclude:
-    - 'lib/puppet/resource_api/version.rb'
+  Enabled: false
 Style/EmptyElse:
   Description: Enforce against empty else clauses, but allow `nil` for clarity.
   EnforcedStyle: empty
@@ -121,13 +122,9 @@ Lint/NestedMethodDefinition:
 RSpec/MessageSpies:
   Enabled: false
 
-# This cop causes issues on our current version of Rubocop (0.57.2) and newer (>= 3.0) Rubies because of keyword arguments.
-Style/BracesAroundHashParameters:
-  Enabled: false
-
 # requires 2.3's squiggly HEREDOC support, which we can't use, yet
 # see http://www.virtuouscode.com/2016/01/06/about-the-ruby-squiggly-heredoc-syntax/
-Layout/IndentHeredoc:
+Layout/HeredocIndentation:
   Enabled: false
 
 # Updated in 0.53 (or thereabouts)
@@ -141,7 +138,7 @@ Style/TrailingCommaInHashLiteral:
   EnforcedStyleForMultiline: comma
 
 # Add allowing "is", as it is a technical term in puppet
-Naming/UncommunicativeMethodParamName:
+Naming/MethodParameterName:
   Description: Checks for method parameter names that contain capital letters, end in
     numbers, or do not meet a minimal length.
   Enabled: true

.rubocop_todo.yml

@@ -0,0 +1,79 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-12-13 15:50:43 UTC using RuboCop version 1.48.1.
+# 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
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 6
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: enums
+Lint/ConstantDefinitionInBlock:
+  Exclude:
+    - 'spec/puppet/resource_api/base_context_spec.rb'
+    - 'spec/puppet/resource_api/transport/wrapper_spec.rb'
+    - 'spec/puppet/resource_api/transport_spec.rb'
+    - 'spec/puppet/resource_api_spec.rb'
+
+# Offense count: 1
+# Configuration parameters: EnforcedStyleForLeadingUnderscores.
+# SupportedStylesForLeadingUnderscores: disallowed, required, optional
+Naming/MemoizedInstanceVariableName:
+  Exclude:
+    - 'lib/puppet/resource_api/type_definition.rb'
+
+# Offense count: 1
+# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
+# SupportedStyles: snake_case, normalcase, non_integer
+# AllowedIdentifiers: capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64
+Naming/VariableNumber:
+  Exclude:
+    - 'spec/puppet/resource_api_spec.rb'
+
+# Offense count: 1
+# Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly.
+# Include: **/*_spec*rb*, **/spec/**/*
+RSpec/FilePath:
+  Exclude:
+    - 'spec/acceptance/custom_insync_spec_hidden_property.rb'
+
+# Offense count: 18
+# Configuration parameters: AssignmentOnly.
+RSpec/InstanceVariable:
+  Exclude:
+    - 'spec/acceptance/namevar_spec.rb'
+    - 'spec/puppet/resource_api/data_type_handling_spec.rb'
+
+# Offense count: 15
+RSpec/LeakyConstantDeclaration:
+  Exclude:
+    - 'spec/puppet/resource_api/base_context_spec.rb'
+    - 'spec/puppet/resource_api/transport/wrapper_spec.rb'
+    - 'spec/puppet/resource_api/transport_spec.rb'
+    - 'spec/puppet/resource_api_spec.rb'
+
+# Offense count: 126
+# Configuration parameters: AllowSubject.
+RSpec/MultipleMemoizedHelpers:
+  Max: 14
+
+# Offense count: 2
+RSpec/RepeatedExampleGroupDescription:
+  Exclude:
+    - 'spec/puppet/resource_api_spec.rb'
+
+# Offense count: 25
+RSpec/StubbedMock:
+  Exclude:
+    - 'spec/puppet/resource_api/property_spec.rb'
+    - 'spec/puppet/resource_api/transport/wrapper_spec.rb'
+    - 'spec/puppet/resource_api/transport_spec.rb'
+    - 'spec/puppet/resource_api_spec.rb'
+
+# Offense count: 2
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: respond_to_missing?
+Style/OptionalBooleanParameter:
+  Exclude:
+    - 'lib/puppet/resource_api/data_type_handling.rb'

lib/puppet/resource_api.rb

@@ -139,7 +139,7 @@ def rsapi_canonicalized_target_state
         @rsapi_canonicalized_target_state ||= begin
           # skip puppet's injected metaparams
           actual_params = @parameters.select { |k, _v| type_definition.attributes.key? k }
-          target_state = Hash[actual_params.map { |k, v| [k, v.rs_value] }]
+          target_state = actual_params.transform_values { |v| v.rs_value }
           target_state = my_provider.canonicalize(context, [target_state]).first if type_definition.feature?('canonicalize')
           target_state
         end
@@ -150,21 +150,23 @@ def rsapi_canonicalized_target_state
       def generate
         # If feature `custom_generate` has been set then call the generate function within the provider and return the given results
         return unless type_definition&.feature?('custom_generate')
+
         should_hash = rsapi_canonicalized_target_state
         is_hash     = rsapi_current_state
         title       = rsapi_title
 
         # Ensure that a custom `generate` method has been created within the provider
         raise(Puppet::DevError, 'No generate method found within the types provider') unless my_provider.respond_to?(:generate)
+
         # Call the providers custom `generate` method
-        rules_resources = my_provider.generate(context, title, is_hash, should_hash)
+        my_provider.generate(context, title, is_hash, should_hash)
 
         # Return array of resources
-        rules_resources
       end
 
       def rsapi_current_state
         return @rsapi_current_state if @rsapi_current_state
+
         # If the current state is not set, then check the cache and, if a value is
         # found, ensure it passes strict_check before allowing it to be used:
         cached_value = rsapi_provider_get_cache.get(rsapi_title)
@@ -238,10 +240,8 @@ def to_resource_shim(resource)
       definition[:attributes].each do |name, options|
         # puts "#{name}: #{options.inspect}"
 
-        if options[:behaviour]
-          unless [:read_only, :namevar, :parameter, :init_only].include? options[:behaviour]
-            raise Puppet::ResourceError, "`#{options[:behaviour]}` is not a valid behaviour value"
-          end
+        if options[:behaviour] && !([:read_only, :namevar, :parameter, :init_only].include? options[:behaviour])
+          raise Puppet::ResourceError, "`#{options[:behaviour]}` is not a valid behaviour value"
         end
 
         # TODO: using newparam everywhere would suppress change reporting
@@ -365,6 +365,7 @@ def flush
         if Puppet.settings[:strict] != :off && rsapi_current_state && (rsapi_current_state[:ensure] == 'present' && target_state[:ensure] == 'present')
           target_state.each do |name, value|
             next unless type_definition.attributes[name][:behaviour] == :init_only && value != rsapi_current_state[name]
+
             message = "Attempting to change `#{name}` init_only attribute value from `#{rsapi_current_state[name]}` to `#{value}`"
             case Puppet.settings[:strict]
             when :warning
@@ -390,12 +391,12 @@ def flush
       end
 
       def raise_missing_attrs
-        error_msg = "The following mandatory attributes were not provided:\n    *  " + @missing_attrs.join(", \n    *  ")
+        error_msg = "The following mandatory attributes were not provided:\n    *  #{@missing_attrs.join(", \n    *  ")}"
         raise Puppet::ResourceError, error_msg if @missing_attrs.any? && (value(:ensure) != :absent && !value(:ensure).nil?)
       end
 
       def raise_missing_params
-        error_msg = "The following mandatory parameters were not provided:\n    *  " + @missing_params.join(", \n    *  ")
+        error_msg = "The following mandatory parameters were not provided:\n    *  #{@missing_params.join(", \n    *  ")}"
         raise Puppet::ResourceError, error_msg
       end
 
@@ -426,14 +427,14 @@ def strict_check_canonicalize(current_state)
         # compare the clone against the current state to see if changes have been made by canonicalize
         return unless state_clone && (current_state != state_clone)
 
-        #:nocov:
+        # :nocov:
         # codecov fails to register this multiline as covered, even though simplecov does.
         message = <<MESSAGE.strip
 #{type_definition.name}[#{@title}]#get has not provided canonicalized values.
 Returned values:       #{current_state.inspect}
 Canonicalized values:  #{state_clone.inspect}
 MESSAGE
-        #:nocov:
+        # :nocov:
         strict_message(message)
       end
 
@@ -448,6 +449,7 @@ def strict_check_title_parameter(current_state)
         self.class.title_patterns.each do |regexp, symbols|
           captures = regexp.match(current_state[:title])
           next if captures.nil?
+
           symbols.zip(captures[1..-1]).each do |symbol_and_lambda, capture|
             # The Resource API does not support passing procs in title_patterns
             # so, unlike Puppet::Resource, we do not need to handle that here.
@@ -461,15 +463,15 @@ def strict_check_title_parameter(current_state)
 
         namevars = type_definition.namevars.reject { |namevar| title_hash[namevar] == rsapi_title[namevar] }
 
-        #:nocov:
+        # :nocov:
         # codecov fails to register this multiline as covered, even though simplecov does.
         message = <<MESSAGE.strip
 #{type_definition.name}[#{@title}]#get has provided a title attribute which does not match all namevars.
 Namevars which do not match: #{namevars.inspect}
 Returned parsed title hash:  #{title_hash.inspect}
 Expected hash:               #{rsapi_title.inspect}
 MESSAGE
-        #:nocov:
+        # :nocov:
         strict_message(message)
       end
 
@@ -528,7 +530,7 @@ def self.parse_title_patterns(patterns)
       end
     end
   end
-  module_function :register_type # rubocop:disable Style/AccessModifierDeclarations
+  module_function :register_type
 
   def load_provider(type_name)
     class_name = class_name_from_type_name(type_name)
@@ -557,14 +559,14 @@ def load_provider(type_name)
       raise Puppet::DevError, "provider class Puppet::Provider::#{class_name}::#{class_name} not found in puppet/provider/#{type_name}/#{type_name}"
     end
   end
-  module_function :load_provider # rubocop:disable Style/AccessModifierDeclarations
+  module_function :load_provider
 
   def load_default_provider(class_name, type_name_sym)
     # loads the "puppet/provider/#{type_name}/#{type_name}" file through puppet
     Puppet::Type.type(type_name_sym).provider(type_name_sym)
     Puppet::Provider.const_get(class_name, false).const_get(class_name, false)
   end
-  module_function :load_default_provider # rubocop:disable Style/AccessModifierDeclarations
+  module_function :load_default_provider
 
   def load_device_provider(class_name, type_name_sym, device_class_name, device_name_sym)
     # loads the "puppet/provider/#{type_name}/#{device_name}" file through puppet
@@ -576,13 +578,13 @@ def load_device_provider(class_name, type_name_sym, device_class_name, device_na
       load_default_provider(class_name, type_name_sym)
     end
   end
-  module_function :load_device_provider # rubocop:disable Style/AccessModifierDeclarations
+  module_function :load_device_provider
 
   # keeps the existing register API format. e.g. Puppet::ResourceApi.register_type
   def register_transport(schema)
     Puppet::ResourceApi::Transport.register(schema)
   end
-  module_function :register_transport # rubocop:disable Style/AccessModifierDeclarations
+  module_function :register_transport
 
   def self.class_name_from_type_name(type_name)
     type_name.to_s.split('_').map(&:capitalize).join

lib/puppet/resource_api/base_context.rb

@@ -92,6 +92,7 @@ def failing(titles, message: 'Failing')
 
   def processing(title, is, should, message = 'Processing')
     raise "#{__method__} only accepts a single resource title" if title.respond_to?(:each)
+
     start_time = Time.now
     setup_context(title, message)
     begin
@@ -114,11 +115,13 @@ def processing(title, is, should, message = 'Processing')
 
   def processed(title, is, should)
     raise "#{__method__} only accepts a single resource title" if title.respond_to?(:each)
+
     notice("Processed #{title} from #{is} to #{should}")
   end
 
   def attribute_changed(title, attribute, is, should, message: nil)
     raise "#{__method__} only accepts a single resource title" if title.respond_to?(:each)
+
     printable_is = 'nil'
     printable_should = 'nil'
     if is
@@ -156,7 +159,7 @@ def send_log(_level, _message)
   private
 
   def format_titles(titles)
-    if titles.length.zero? && !titles.is_a?(String)
+    if titles.empty? && !titles.is_a?(String)
       @type.name
     else
       "#{@type.name}[#{[titles].flatten.compact.join(', ')}]"
@@ -170,6 +173,7 @@ def setup_context(titles, message = nil)
 
   def format_seconds(seconds)
     return '%.6f' % seconds if seconds < 1
+
     '%.2f' % seconds
   end
 end

lib/puppet/resource_api/data_type_handling.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation
 
 # This module is used to handle data inside types, contains methods for munging
 # and validation of the type values.
@@ -49,6 +49,7 @@ def self.mungify_core(type, value, error_msg_prefix, unpack_strings = false)
       # to 1 (an integer)
       cleaned_value, error_msg = try_mungify(type, value, error_msg_prefix)
       raise Puppet::ResourceError, error_msg if error_msg
+
       cleaned_value
     elsif value == :false # rubocop:disable Lint/BooleanSymbol
       # work around https://tickets.puppetlabs.com/browse/PUP-2368
@@ -121,6 +122,7 @@ def self.try_mungify(type, value, error_msg_prefix)
 
     error_msg = try_validate(type, value, error_msg_prefix)
     return [nil, error_msg] if error_msg # an error
+
     [value, nil]                         # match
   end
 
@@ -171,16 +173,16 @@ def self.try_validate(type, value, error_msg_prefix)
 
     # an error :-(
     inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
-    error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch(
+    Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch(
       error_msg_prefix,
       type,
       inferred_type,
     )
-    error_msg
   end
 
   def self.validate_ensure(definition)
     return unless definition[:attributes].key? :ensure
+
     options = definition[:attributes][:ensure]
     type = parse_puppet_type(:ensure, options[:type])
 

lib/puppet/resource_api/glue.rb

@@ -3,6 +3,7 @@
 require 'yaml'
 
 module Puppet; end # rubocop:disable Style/Documentation
+
 module Puppet::ResourceApi
   # A trivial class to provide the functionality required to push data through the existing type/provider parts of puppet
   class ResourceShim
@@ -45,7 +46,7 @@ def to_hiera_hash
     # Convert our resource to yaml for Hiera purposes.
     def to_hierayaml
       attributes = Hash[filtered_keys.map { |k| [k.to_s, values[k]] }]
-      YAML.dump('type' => { title => attributes }).split("\n").drop(2).join("\n") + "\n"
+      "#{YAML.dump('type' => { title => attributes }).split("\n").drop(2).join("\n")}\n"
     end
 
     def to_json(*)

lib/puppet/resource_api/parameter.rb

@@ -3,7 +3,7 @@
 require 'puppet/util'
 require 'puppet/parameter'
 
-module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation,Style/ClassAndModuleChildren
+module Puppet; module ResourceApi; end; end # predeclare the main module # rubocop:disable Style/Documentation
 
 # Class containing parameter functionality for ResourceApi.
 class Puppet::ResourceApi::Parameter < Puppet::Parameter