Merge pull request #397 from puppetlabs/CAT-2453

pmcmaw · web-flow · commit 07aff8ef6e09 · 2025-09-30T15:05:45.000+01:00
(CAT-2453) Update rubocop to remove rexml dependency
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,13 +1,13 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-04-09 15:40:01 UTC using RuboCop version 1.64.1.
+# on 2025-09-30 12:45:13 UTC using RuboCop version 1.73.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
 # versions of RuboCop, may require this file to be generated again.
 
 # Offense count: 1
-# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches.
+# Configuration parameters: IgnoreLiteralBranches, IgnoreConstantBranches, IgnoreDuplicateElseBranch.
 Lint/DuplicateBranch:
   Exclude:
     - 'lib/puppet-languageserver-sidecar/puppet_helper.rb'
@@ -36,7 +36,13 @@ Lint/OrAssignmentToConstant:
   Exclude:
     - 'lib/puppet-languageserver/session_state/document_store.rb'
 
-# Offense count: 169
+# Offense count: 2
+Lint/UselessConstantScoping:
+  Exclude:
+    - 'lib/puppet-languageserver/manifest/format_on_type_provider.rb'
+    - 'lib/puppet-languageserver/sidecar_protocol.rb'
+
+# Offense count: 168
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
   Max: 152
@@ -86,10 +92,11 @@ Naming/MemoizedInstanceVariableName:
     - 'lib/puppet-languageserver-sidecar/puppet_strings_monkey_patches.rb'
     - 'lib/puppet-languageserver/server_capabilities.rb'
 
-# Offense count: 3
+# Offense count: 4
 # Configuration parameters: MinSize.
 Performance/CollectionLiteralInLoop:
   Exclude:
+    - 'lib/puppet-languageserver-sidecar/puppet_helper.rb'
     - 'lib/puppet-languageserver/sidecar_protocol.rb'
 
 # Offense count: 1
@@ -109,14 +116,13 @@ Security/IoMethods:
 Style/Documentation:
   Enabled: false
 
-# Offense count: 4
+# Offense count: 3
 # This cop supports unsafe autocorrection (--autocorrect-all).
 # Configuration parameters: AllowedReceivers.
 # AllowedReceivers: Thread.current
 Style/HashEachMethods:
   Exclude:
     - 'lib/puppet-languageserver-sidecar/facter_helper.rb'
-    - 'lib/puppet-languageserver-sidecar/puppet_modulepath_monkey_patches.rb'
     - 'lib/puppet-languageserver-sidecar/puppet_strings_helper.rb'
     - 'lib/puppet-languageserver/manifest/folding_provider.rb'
 
@@ -211,7 +217,7 @@ Style/ZeroLengthPredicate:
 
 # Offense count: 622
 # This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings.
 # URISchemes: http, https
 Layout/LineLength:
   Max: 1529
diff --git a/Gemfile b/Gemfile
@@ -14,9 +14,9 @@ group :development do
   gem 'puppet-lint', '~> 4.0',            :require => false
   gem 'puppetfile-resolver', '~> 0.6.2',  :require => false
   gem 'yard', '~> 0.9.28',                :require => false
-  gem "rubocop", '~> 1.64.0',             :require => false
-  gem "rubocop-performance", '~> 1.16',   :require => false
-  gem "rubocop-rspec", '~> 3.0',          :require => false
+  gem "rubocop", '~> 1.73.0',             :require => false
+  gem "rubocop-performance", '~> 1.24.0',   :require => false
+  gem "rubocop-rspec", '~> 3.5.0',          :require => false
   gem 'simplecov',                        :require => false
   gem 'simplecov-console',                :require => false
   gem 'json', "< 2.8.0",                  :require => false
diff --git a/lib/puppet-debugserver/debug_session/flow_control.rb b/lib/puppet-debugserver/debug_session/flow_control.rb
@@ -56,7 +56,7 @@ def assert_flag(flag_name)
           @flags[flag_name] = true
           PuppetDebugServer.log_message(:debug, "Asserting flag #{flag_name} is true")
           # Any custom logic for when flags are asserted
-          # rubocop:disable Style/MultipleComparison, Style/SoleNestedConditional This is faster and doesn't require creation of an array
+          # rubocop:disable Style/MultipleComparison, Style/SoleNestedConditional -- This is faster and doesn't require creation of an array
           if flag_name == :client_completed_configuration || flag_name == :session_setup
             # If the client_completed_configuration and session_setup flag are asserted but the session isn't active yet
             # assert the flag start_puppet so puppet can start in the main thread.
diff --git a/lib/puppet-debugserver/debug_session/hook_handlers.rb b/lib/puppet-debugserver/debug_session/hook_handlers.rb
@@ -214,7 +214,7 @@ def on_hook_log_message(args)
         level = msg.level.to_s.capitalize
 
         category = 'stderr'
-        category = 'stdout' if msg.level == :notice || msg.level == :info || msg.level == :debug
+        category = 'stdout' if %i[notice info debug].include?(msg.level)
 
         @debug_session.send_output_event(
           'category' => category,
diff --git a/lib/puppet-debugserver/hooks.rb b/lib/puppet-debugserver/hooks.rb
@@ -89,7 +89,7 @@ def get_hook(event_name, hook_name)
     # @note Modifying the returned hash does not alter the hooks, use
     # `add_hook`/`delete_hook` for that.
     def get_hooks(event_name)
-      (@hooks[event_name.to_s]).to_h
+      @hooks[event_name.to_s].to_h
     end
 
     # @param [Symbol] event_name The name of the event.
diff --git a/lib/puppet-debugserver/message_handler.rb b/lib/puppet-debugserver/message_handler.rb
@@ -81,7 +81,7 @@ def request_evaluate(_connection_id, request_message)
           request_message,
           'result' => debug_session.evaluate_string(obj.arguments), 'variablesReference' => 0
         )
-      rescue => e # rubocop:disable Style/RescueStandardError  Anything could be thrown here. Catch 'em all
+      rescue => e # rubocop:disable Style/RescueStandardError -- Anything could be thrown here. Catch 'em all
         PuppetEditorServices::Protocol::DebugAdapterMessages.reply_error(
           request_message,
           e.to_s
diff --git a/lib/puppet-debugserver/puppet_debug_session.rb b/lib/puppet-debugserver/puppet_debug_session.rb
@@ -34,7 +34,7 @@ class PuppetDebugSession
     attr_reader :puppet_session_state
 
     # Use to track the default instance of the debug session
-    @@session_instance = nil # rubocop:disable Style/ClassVars  This class method (not instance) should be inherited
+    @@session_instance = nil # rubocop:disable Style/ClassVars -- This class method (not instance) should be inherited
 
     VARIABLES_REFERENCE_TOP_SCOPE = 1
     ERROR_LOG_LEVELS = %i[warning err alert emerg crit].freeze
@@ -44,7 +44,7 @@ def self.instance
       # This can be called from any thread
       return @@session_instance unless @@session_instance.nil? # This class method (not instance) should be inherited
 
-      @@session_instance = PuppetDebugSession.new # rubocop:disable Style/ClassVars  This class method (not instance) should be inherited
+      @@session_instance = PuppetDebugSession.new # rubocop:disable Style/ClassVars -- This class method (not instance) should be inherited
     end
 
     def initialize
@@ -258,7 +258,7 @@ def generate_variables_list(arguments)
       result = nil
 
       # Check if this is the topscope
-      if variables_reference == VARIABLES_REFERENCE_TOP_SCOPE # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+      if variables_reference == VARIABLES_REFERENCE_TOP_SCOPE # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
         result = variable_list_from_hash(puppet_session_state.actual.compiler.topscope.to_hash(false))
       end
       return result unless result.nil?
diff --git a/lib/puppet-debugserver/puppet_monkey_patches.rb b/lib/puppet-debugserver/puppet_monkey_patches.rb
@@ -57,7 +57,7 @@ def evaluate(target, scope)
           result = original_evaluate(target, scope)
           PuppetDebugServer::PuppetDebugSession.instance.execute_hook(:hook_after_pops_evaluate, [self, target, scope])
           result
-        rescue => e # rubocop:disable Style/RescueStandardError Any error could be thrown here
+        rescue => e # rubocop:disable Style/RescueStandardError -- Any error could be thrown here
           # Emit non-Puppet related errors to the debug log. We shouldn't get any of these!
           PuppetDebugServer.log_message(:debug, "Error in Puppet::Pops::Evaluator::EvaluatorImpl.evaluate #{e}: #{e.backtrace}") unless e.is_a?(Puppet::Error)
           raise
diff --git a/lib/puppet-languageserver-sidecar/puppet_helper.rb b/lib/puppet-languageserver-sidecar/puppet_helper.rb
@@ -85,7 +85,7 @@ def self.retrieve_via_puppet_strings(cache, options = {})
       return result if object_types.empty?
 
       current_env = current_environment
-      for_agent = options[:for_agent].nil? ? true : options[:for_agent]
+      for_agent = options[:for_agent].nil? || options[:for_agent]
       Puppet::Pops::Loaders.new(current_env, for_agent)
 
       finder = PuppetPathFinder.new(current_env, object_types)
@@ -95,19 +95,19 @@ def self.retrieve_via_puppet_strings(cache, options = {})
         file_doc = PuppetLanguageServerSidecar::PuppetStringsHelper.file_documentation(path, finder.puppet_path, cache)
         next if file_doc.nil?
 
-        if object_types.include?(:class) # rubocop:disable Style/IfUnlessModifier   This reads better
+        if object_types.include?(:class) # rubocop:disable Style/IfUnlessModifier -- This reads better
           file_doc.classes.each { |item| result.append!(item) }
         end
-        if object_types.include?(:datatype) # rubocop:disable Style/IfUnlessModifier   This reads better
+        if object_types.include?(:datatype) # rubocop:disable Style/IfUnlessModifier -- This reads better
           file_doc.datatypes.each { |item| result.append!(item) }
         end
-        if object_types.include?(:function) # rubocop:disable Style/IfUnlessModifier   This reads better
+        if object_types.include?(:function) # rubocop:disable Style/IfUnlessModifier -- This reads better
           file_doc.functions.each { |item| result.append!(item) }
         end
         next unless object_types.include?(:type)
 
         file_doc.types.each do |item|
-          result.append!(item) unless name == 'whit' || name == 'component'
+          result.append!(item) unless %w[whit component].include?(name)
           finder.temp_file.unlink if item.key == 'file' && File.exist?(finder.temp_file.path) # Remove the temp_file.rb if it exists
         end
       end
diff --git a/lib/puppet-languageserver-sidecar/puppet_modulepath_monkey_patches.rb b/lib/puppet-languageserver-sidecar/puppet_modulepath_monkey_patches.rb
@@ -118,7 +118,7 @@ class Puppet::Settings::EnvironmentConf
   def modulepath
     result = original_modulepath
 
-    if PuppetLanguageServerSidecar::Workspace.has_module_metadata? # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+    if PuppetLanguageServerSidecar::Workspace.has_module_metadata? # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
       result = result + File::PATH_SEPARATOR + PuppetLanguageServerSidecar::Workspace.root_path
     end
 
diff --git a/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb b/lib/puppet-languageserver-sidecar/puppet_strings_helper.rb
@@ -109,7 +109,7 @@ def populate_from_yard_registry!(puppet_path)
 
     def populate_from_sidecar_cache!(path, cache)
       cached_result = cache.load(path, PuppetLanguageServerSidecar::Cache::PUPPETSTRINGS_SECTION)
-      unless cached_result.nil? # rubocop:disable Style/GuardClause    Reads better this way
+      unless cached_result.nil? # rubocop:disable Style/GuardClause -- Reads better this way
         begin
           obj = FileDocumentation.new.from_json!(cached_result)
           @cache[path] = obj
@@ -381,10 +381,10 @@ def from_json!(json_string)
         case key
         when 'path'
           # Simple deserialised object types
-          self.instance_variable_set(:"@#{key}", obj[key]) # rubocop:disable Style/RedundantSelf   Reads better this way
+          self.instance_variable_set(:"@#{key}", obj[key]) # rubocop:disable Style/RedundantSelf -- Reads better this way
         else
           # Sidecar protocol list object types
-          prop = self.instance_variable_get(:"@#{key}") # rubocop:disable Style/RedundantSelf   Reads better this way
+          prop = self.instance_variable_get(:"@#{key}") # rubocop:disable Style/RedundantSelf -- Reads better this way
 
           obj[key].each do |child_hash|
             child = prop.child_type.new
diff --git a/lib/puppet-languageserver/manifest/completion_provider.rb b/lib/puppet-languageserver/manifest/completion_provider.rb
@@ -275,7 +275,7 @@ def self.resolve(session_state, completion_item)
           snippet = "#{data['name']} { '${1:title}':\n"
           attr_names.each_index do |index|
             name = attr_names[index]
-            value_text = (name == 'ensure') ? 'present' : 'value' # rubocop:disable Style/TernaryParentheses  In this case it's easier to read.
+            value_text = (name == 'ensure') ? 'present' : 'value' # rubocop:disable Style/TernaryParentheses -- In this case it's easier to read.
             snippet += "\t#{name.ljust(max_length, ' ')} => '${#{index + 2}:#{value_text}}'\n"
           end
           snippet += '}'
diff --git a/lib/puppet-languageserver/message_handler.rb b/lib/puppet-languageserver/message_handler.rb
@@ -13,7 +13,7 @@ def initialize(*_)
       @session_state = ClientSessionState.new(self)
     end
 
-    def session_state # rubocop:disable Style/TrivialAccessors During the refactor, this is fine.
+    def session_state # rubocop:disable Style/TrivialAccessors -- During the refactor, this is fine.
       @session_state
     end
 
diff --git a/lib/puppet-languageserver/puppet_parser_helper.rb b/lib/puppet-languageserver/puppet_parser_helper.rb
@@ -100,14 +100,14 @@ def self.object_under_cursor(content, line_num, char_num, options)
           # Perhaps try inserting double quotes.  Useful in empty arrays or during variable assignment
           # Grab the line up to the cursor character + 1
           line = get_line_at(content, line_offsets, line_num).slice!(0, char_num + 1)
-          if line.strip.end_with?('=') || line.end_with?('[]') # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+          if line.strip.end_with?('=') || line.end_with?('[]') # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
             new_content = insert_text_at(content, line_offsets, line_num, char_num, "''")
           end
         when :try_quotes_and_comma
           # Perhaps try inserting double quotes with a comma.  Useful resource properties and parameter assignments
           # Grab the line up to the cursor character + 1
           line = get_line_at(content, line_offsets, line_num).slice!(0, char_num + 1)
-          if line.strip.end_with?('=>') # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+          if line.strip.end_with?('=>') # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
             new_content = insert_text_at(content, line_offsets, line_num, char_num, "'',")
           end
         else
@@ -159,7 +159,7 @@ def self.object_under_cursor(content, line_num, char_num, options)
       else
         path = []
         result.model._pcore_all_contents(path) do |item|
-          if check_for_valid_item(item, abs_offset, options[:disallowed_classes]) # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+          if check_for_valid_item(item, abs_offset, options[:disallowed_classes]) # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
             valid_models.push(model_path_locator_struct.new(item, path.dup))
           end
         end
@@ -171,7 +171,7 @@ def self.object_under_cursor(content, line_num, char_num, options)
 
       response = valid_models[0]
 
-      if response.respond_to? :eAllContents # rubocop:disable Style/IfUnlessModifier  Nicer to read like this
+      if response.respond_to? :eAllContents # rubocop:disable Style/IfUnlessModifier -- Nicer to read like this
         response = model_path_locator_struct.new(response, construct_path(response))
       end
 
diff --git a/lib/puppet-languageserver/session_state/language_client.rb b/lib/puppet-languageserver/session_state/language_client.rb
@@ -151,7 +151,7 @@ def parse_register_capability_response!(response, original_request)
           original_request.params.registrations.each do |reg|
             # Mark the registration as completed and failed
             @registrations[reg.method__lsp] = [] if @registrations[reg.method__lsp].nil?
-            @registrations[reg.method__lsp].select { |i| i[:id] == reg.id }.each { |i| i[:registered] = false; i[:state] = :complete } # rubocop:disable Style/Semicolon This is fine
+            @registrations[reg.method__lsp].select { |i| i[:id] == reg.id }.each { |i| i[:registered] = false; i[:state] = :complete } # rubocop:disable Style/Semicolon -- This is fine
           end
           return true
         end
@@ -161,7 +161,7 @@ def parse_register_capability_response!(response, original_request)
 
           # Mark the registration as completed and succesful
           @registrations[reg.method__lsp] = [] if @registrations[reg.method__lsp].nil?
-          @registrations[reg.method__lsp].select { |i| i[:id] == reg.id }.each { |i| i[:registered] = true; i[:state] = :complete } # rubocop:disable Style/Semicolon This is fine
+          @registrations[reg.method__lsp].select { |i| i[:id] == reg.id }.each { |i| i[:registered] = true; i[:state] = :complete } # rubocop:disable Style/Semicolon -- This is fine
 
           # If we just registered the workspace/didChangeConfiguration method then
           # also trigger a configuration request to get the initial state
diff --git a/lib/puppet_editor_services/connection/stdio.rb b/lib/puppet_editor_services/connection/stdio.rb
@@ -6,12 +6,12 @@ module PuppetEditorServices
   module Connection
     class Stdio < ::PuppetEditorServices::Connection::Base
       def send_data(data)
-        $editor_services_stdout.write(data) # rubocop:disable Style/GlobalVars  We need this global var
+        $editor_services_stdout.write(data) # rubocop:disable Style/GlobalVars -- We need this global var
         true
       end
 
       def close_after_writing
-        $editor_services_stdout.flush # rubocop:disable Style/GlobalVars  We need this global var
+        $editor_services_stdout.flush # rubocop:disable Style/GlobalVars -- We need this global var
         server.close_connection
         true
       end
diff --git a/lib/puppet_editor_services/handler/json_rpc.rb b/lib/puppet_editor_services/handler/json_rpc.rb
@@ -107,7 +107,7 @@ def handle_response(response_message, context)
         original_request = context[:request]
         return false if original_request.nil?
 
-        unless response_message.is_successful # rubocop:disable Style/IfUnlessModifier Line is too long otherwise
+        unless response_message.is_successful # rubocop:disable Style/IfUnlessModifier -- Line is too long otherwise
           PuppetEditorServices.log_message(:error, "Response for method '#{original_request.rpc_method}' with id '#{original_request.id}' failed with #{response_message.error}")
         end
         method_name = rpc_name_to_ruby_method_name('response', original_request.rpc_method)
diff --git a/lib/puppet_editor_services/protocol/json_rpc.rb b/lib/puppet_editor_services/protocol/json_rpc.rb
@@ -167,7 +167,7 @@ def receive_json_message_as_hash(json_obj)
         end
 
         # Requests and Notifications must have a method
-        if (is_request || is_notification) && !((json_obj[KEY_METHOD]).is_a? String)
+        if (is_request || is_notification) && !(json_obj[KEY_METHOD].is_a? String)
           reply_error id, CODE_INVALID_REQUEST, MSG_INVALID_REQ_METHOD
           return false
         end
diff --git a/lib/puppet_editor_services/server.rb b/lib/puppet_editor_services/server.rb
@@ -9,7 +9,7 @@ def self.current_server
     end
 
     def self.current_server=(value)
-      @@current_server = value # rubocop:disable Style/ClassVars  This is fine
+      @@current_server = value # rubocop:disable Style/ClassVars -- This is fine
     end
   end
 end
diff --git a/lib/puppet_editor_services/server/stdio.rb b/lib/puppet_editor_services/server/stdio.rb
@@ -24,12 +24,12 @@ def start
         $VERBOSE = nil
         # Some libraries use $stdout to write to the console. Suppress all of that too!
         # Copy the existing $stdout variable and then reassign to NUL to suppress it
-        $editor_services_stdout = $stdout # rubocop:disable Style/GlobalVars  We need this global var
+        $editor_services_stdout = $stdout # rubocop:disable Style/GlobalVars -- We need this global var
         $stdout = File.open(File::NULL, 'w')
 
-        $editor_services_stdout.sync = true # rubocop:disable Style/GlobalVars  We need this global var
+        $editor_services_stdout.sync = true # rubocop:disable Style/GlobalVars -- We need this global var
         # Stop the stupid CRLF injection when on Windows
-        $editor_services_stdout.binmode unless $editor_services_stdout.binmode # rubocop:disable Style/GlobalVars  We need this global var
+        $editor_services_stdout.binmode unless $editor_services_stdout.binmode # rubocop:disable Style/GlobalVars -- We need this global var
 
         @client_connection = PuppetEditorServices::Connection::Stdio.new(self)
 
diff --git a/lib/puppet_editor_services/server/tcp.rb b/lib/puppet_editor_services/server/tcp.rb
@@ -294,7 +294,7 @@ def add_connection(io, service_object)
           end
         end
         callback(conn, :post_init)
-      rescue Exception => e # rubocop:disable Lint/RescueException  Need to swallow all errors here
+      rescue Exception => e # rubocop:disable Lint/RescueException -- Need to swallow all errors here
         callback(self, :log, "Error creating connection #{e.inspect}\n#{e.backtrace}")
       end
 

Original file line number	Diff line number	Diff line change
`@@ -81,7 +81,7 @@ def request_evaluate(_connection_id, request_message)`
`81`	`81`	`request_message,`
`82`	`82`	`'result' => debug_session.evaluate_string(obj.arguments), 'variablesReference' => 0`
`83`	`83`	`)`
`84`		`- rescue => e # rubocop:disable Style/RescueStandardError Anything could be thrown here. Catch 'em all`
	`84`	`+ rescue => e # rubocop:disable Style/RescueStandardError -- Anything could be thrown here. Catch 'em all`
`85`	`85`	`PuppetEditorServices::Protocol::DebugAdapterMessages.reply_error(`
`86`	`86`	`request_message,`
`87`	`87`	`e.to_s`