Rubocops compatible with Ruby 2.4+

.rubocop.yml

@@ -66,7 +66,7 @@ Style/RedundantEach: # new in 1.38
   Enabled: true
 Style/RedundantInitialize: # new in 1.27
   Enabled: true
-Style/RedundantStringEscape: # new in 1.37
+Style/RedundantStringEscape: # new in 1.37, 'pending' by default so enabled to make sure it's applied
   Enabled: true
 Style/YodaExpression: # new in 1.42
   Enabled: true
@@ -140,7 +140,8 @@ Performance/StringIdentifierArgument: # new in 1.13
   Enabled: true
 Performance/StringInclude: # new in 1.7
   Enabled: true
-
+Performance/Sum: # new in 1.8, pending so left enabled explicitly until 2.0
+  Enabled: true
 
 # Old cops
 
@@ -1318,6 +1319,9 @@ Style/ExplicitBlockArgument:
 Style/ExponentialNotation:
   Enabled: false
 
+Style/FetchEnvVar:
+  Enabled: false
+
 Style/FloatDivision:
   Enabled: false
 

.rubocop_todo.yml

@@ -36,6 +36,7 @@ Minitest/DuplicateTestRun:
 Minitest/EmptyLineBeforeAssertionMethods:
   Exclude:
     - 'test/new_relic/agent_test.rb'
+    - 'test/new_relic/cli/commands/deployments_test.rb'
 
 # Offense count: 269
 Minitest/MultipleAssertions:
@@ -57,44 +58,3 @@ Style/ConcatArrayLiterals:
 Minitest/RefuteRespondTo:
   Exclude:
     - 'test/new_relic/cli/commands/deployments_test.rb'
-
-Minitest/EmptyLineBeforeAssertionMethods:
-  Exclude:
-    - 'test/new_relic/agent_test.rb'
-    - 'test/new_relic/cli/commands/deployments_test.rb'
-
-# Offense count: 23
-# This cop supports safe autocorrection (--autocorrect).
-Performance/RegexpMatch:
-  Enabled: false
-
-# Offense count: 1
-# This cop supports unsafe autocorrection (--autocorrect-all).
-# Configuration parameters: OnlySumOrWithInitialValue.
-Performance/Sum:
-  Exclude:
-    - 'lib/new_relic/agent/system_info.rb'
-
-# Offense count: 72
-# This cop supports unsafe autocorrection (--autocorrect-all).
-Performance/UnfreezeString:
-  Enabled: false
-
-# Offense count: 21
-# This cop supports safe autocorrection (--autocorrect).
-# Configuration parameters: AllowedVars.
-Style/FetchEnvVar:
-  Enabled: false
-
-# Offense count: 1
-# This cop supports safe autocorrection (--autocorrect).
-Style/RedundantStringEscape:
-  Exclude:
-    - 'test/new_relic/agent/logging_test.rb'
-
-# Offense count: 115
-# This cop supports unsafe autocorrection (--autocorrect-all).
-# Configuration parameters: ConvertCodeThatCanStartToReturnNil, AllowedMethods, MaxChainLength.
-# AllowedMethods: present?, blank?, presence, try, try!
-Style/SafeNavigation:
-  Enabled: false

bin/nrdebug

@@ -216,7 +216,7 @@ class ProcessReport
 
       section(f) do
         c_backtraces, ruby_backtraces = @target.gather_backtraces
-        if c_backtraces =~ /could not attach/i
+        if /could not attach/i.match?(c_backtraces)
           fail("Failed to attach to target process. Please try again with sudo.")
         end
 

infinite_tracing/lib/infinite_tracing/agent_integrations/agent.rb

@@ -23,7 +23,7 @@ def new_infinite_tracer
     def handle_force_restart(error)
       ::NewRelic::Agent.logger.debug(error.message)
       drop_buffered_data
-      @service.force_restart if @service
+      @service&.force_restart
       @connect_state = :pending
       close_infinite_tracer
       sleep(30)

infinite_tracing/lib/infinite_tracing/config.rb

@@ -64,7 +64,7 @@ def trace_observer_host
       # is overridden by the presence of the port on the host entry.
       def port_from_host_entry
         port_str = NewRelic::Agent.config[:'infinite_tracing.trace_observer.host'].scan(%r{:(\d+)$}).flatten
-        if port = (port_str[0] and port_str[0].to_i)
+        if port = port_str[0]&.to_i
           NewRelic::Agent.logger.warn(":'infinite_tracing.trace_observer.port' is ignored if present because :'infinite_tracing.trace_observer.host' specifies the port")
           return port
         end

infinite_tracing/test/support/fake_trace_observer_helpers.rb

@@ -52,7 +52,7 @@ def assert_only_one_subscription_notifier
           end
 
           def teardown
-            @mock_thread.kill if @mock_thread
+            @mock_thread&.kill
             @mock_thread = nil
             @server_response_enum = nil
             reset_buffers_and_caches
@@ -170,7 +170,7 @@ def emulate_streaming_with_tracer(tracer_class, count, max_buffer_size, &block)
               end
             end
           ensure
-            client.stop unless client.nil?
+            client&.stop
           end
 
           # when the server responds with an error that should stop the server
@@ -215,7 +215,7 @@ def create_grpc_mock(simulate_broken_server: false, expect_mock: true, expect_ba
           end
 
           def join_grpc_mock
-            @mock_thread.join if @mock_thread
+            @mock_thread&.join
           end
 
           # Simulates a Messages seen response from the mock grpc server

lib/new_relic/agent.rb

@@ -386,7 +386,8 @@ def manual_start(options = {})
     #
     def after_fork(options = {})
       record_api_supportability_metric(:after_fork)
-      agent.after_fork(options) if agent
+      # the following line needs else branch coverage
+      agent.after_fork(options) if agent # rubocop:disable Style/SafeNavigation
     end
 
     # Shutdown the agent.  Call this before exiting.  Sends any queued data
@@ -398,15 +399,16 @@ def after_fork(options = {})
     #
     def shutdown(options = {})
       record_api_supportability_metric(:shutdown)
-      agent.shutdown if agent
+      agent&.shutdown
     end
 
     # Clear out any data the agent has buffered but has not yet transmitted
     # to the collector.
     #
     # @api public
     def drop_buffered_data
-      agent.drop_buffered_data if agent
+      # the following line needs else branch coverage
+      agent.drop_buffered_data if agent # rubocop:disable Style/SafeNavigation
       record_api_supportability_metric(:drop_buffered_data)
     end
 
@@ -465,8 +467,7 @@ def set_sql_obfuscator(type = :replace, &block)
     #
     def ignore_transaction
       record_api_supportability_metric(:ignore_transaction)
-      txn = NewRelic::Agent::Transaction.tl_current
-      txn.ignore! if txn
+      NewRelic::Agent::Transaction.tl_current&.ignore!
     end
 
     # This method disables the recording of Apdex metrics in the current
@@ -476,8 +477,7 @@ def ignore_transaction
     #
     def ignore_apdex
       record_api_supportability_metric(:ignore_apdex)
-      txn = NewRelic::Agent::Transaction.tl_current
-      txn.ignore_apdex! if txn
+      NewRelic::Agent::Transaction.tl_current&.ignore_apdex!
     end
 
     # This method disables browser monitoring javascript injection in the
@@ -487,8 +487,7 @@ def ignore_apdex
     #
     def ignore_enduser
       record_api_supportability_metric(:ignore_enduser)
-      txn = NewRelic::Agent::Transaction.tl_current
-      txn.ignore_enduser! if txn
+      NewRelic::Agent::Transaction.tl_current&.ignore_enduser!
     end
 
     # Yield to the block without collecting any metrics or traces in
@@ -564,8 +563,7 @@ def add_custom_attributes(params) # THREAD_LOCAL_ACCESS
       record_api_supportability_metric(:add_custom_attributes)
 
       if params.is_a?(Hash)
-        txn = Transaction.tl_current
-        txn.add_custom_attributes(params) if txn
+        Transaction.tl_current&.add_custom_attributes(params)
 
         segment = ::NewRelic::Agent::Tracer.current_segment
         if segment

lib/new_relic/agent/agent_helpers/connect.rb

@@ -115,7 +115,7 @@ def log_connection(config_data)
           ::NewRelic::Agent.logger.debug("Connected to NewRelic Service at #{@service.collector.name}")
           ::NewRelic::Agent.logger.debug("Agent Run       = #{@service.agent_id}.")
           ::NewRelic::Agent.logger.debug("Connection data = #{config_data.inspect}")
-          if config_data['messages'] && config_data['messages'].any?
+          if config_data['messages']&.any?
             log_collector_messages(config_data['messages'])
           end
         end

lib/new_relic/agent/agent_helpers/start_worker_thread.rb

@@ -42,7 +42,7 @@ def create_event_loop
         # The use-case where this typically arises is in cronjob scheduled rake tasks where there's
         # also some network stability/latency issues happening.
         def stop_event_loop
-          @event_loop.stop if @event_loop
+          @event_loop&.stop
           # Wait the end of the event loop thread.
           if @worker_thread
             unless @worker_thread.join(3)
@@ -100,7 +100,7 @@ def create_and_run_event_loop
         def handle_force_restart(error)
           ::NewRelic::Agent.logger.debug(error.message)
           drop_buffered_data
-          @service.force_restart if @service
+          @service&.force_restart
           @connect_state = :pending
           sleep(30)
         end

lib/new_relic/agent/commands/agent_command_router.rb

@@ -29,7 +29,8 @@ def initialize(event_listener = nil)
           @handlers['start_profiler'] = proc { |cmd| thread_profiler_session.handle_start_command(cmd) }
           @handlers['stop_profiler'] = proc { |cmd| thread_profiler_session.handle_stop_command(cmd) }
 
-          if event_listener
+          # the following statement needs else branch coverage
+          if event_listener # rubocop:disable Style/SafeNavigation
             event_listener.subscribe(:before_shutdown, &method(:on_before_shutdown))
           end
         end

lib/new_relic/agent/commands/thread_profiler_session.rb

@@ -51,7 +51,7 @@ def stop(report_data)
 
         def harvest
           NewRelic::Agent.logger.debug(
-            "Harvesting from Thread Profiler #{@finished_profile.to_log_description unless @finished_profile.nil?}"
+            "Harvesting from Thread Profiler #{@finished_profile&.to_log_description}"
           )
           profile = @finished_profile
           @backtrace_service.profile_agent_code = false

lib/new_relic/agent/configuration/default_source.rb

@@ -104,7 +104,8 @@ def self.config_search_paths
             end
 
             # If we're packaged for warbler, we can tell from GEM_HOME
-            if ENV["GEM_HOME"] && ENV["GEM_HOME"].end_with?(".jar!")
+            # the following line needs else branch coverage
+            if ENV["GEM_HOME"] && ENV["GEM_HOME"].end_with?(".jar!") # rubocop:disable Style/SafeNavigation
               app_name = File.basename(ENV["GEM_HOME"], ".jar!")
               paths << File.join(ENV["GEM_HOME"], app_name, "config", "newrelic.yml")
               paths << File.join(ENV["GEM_HOME"], app_name, "config", "newrelic.yml.erb")

lib/new_relic/agent/configuration/environment_source.rb

@@ -94,7 +94,7 @@ def set_key_by_type(config_key, environment_key)
           elsif type == Array
             self[config_key] = value.split(/\s*,\s*/)
           elsif type == NewRelic::Agent::Configuration::Boolean
-            if value =~ /false|off|no/i
+            if /false|off|no/i.match?(value)
               self[config_key] = false
             elsif !value.nil?
               self[config_key] = true

lib/new_relic/agent/configuration/server_source.rb

@@ -70,7 +70,7 @@ def merge_agent_config_hash(merged_settings, connect_reply)
 
         def fix_transaction_threshold(merged_settings)
           # when value is "apdex_f" remove the config and defer to default
-          if merged_settings['transaction_tracer.transaction_threshold'].to_s =~ /apdex_f/i
+          if /apdex_f/i.match?(merged_settings['transaction_tracer.transaction_threshold'].to_s)
             merged_settings.delete('transaction_tracer.transaction_threshold')
           end
         end

lib/new_relic/agent/configuration/yaml_source.rb

@@ -124,7 +124,7 @@ def substitute_transaction_threshold(config)
               config['transaction_tracer']['transaction_threshold'].to_s =~ /apdex_f/i
             # when value is "apdex_f" remove the config and defer to default
             config['transaction_tracer'].delete('transaction_threshold')
-          elsif config['transaction_tracer.transaction_threshold'].to_s =~ /apdex_f/i
+          elsif /apdex_f/i.match?(config['transaction_tracer.transaction_threshold'].to_s)
             config.delete('transaction_tracer.transaction_threshold')
           end
         end

lib/new_relic/agent/connect/request_builder.rb

@@ -48,7 +48,7 @@ def sanitize_environment_report(environment_report)
 
         def environment_metadata
           env_copy = {}
-          ENV.keys.each { |k| env_copy[k] = ENV[k] if k =~ /^NEW_RELIC_METADATA_/ }
+          ENV.keys.each { |k| env_copy[k] = ENV[k] if /^NEW_RELIC_METADATA_/.match?(k) }
           env_copy
         end
 

lib/new_relic/agent/custom_event_aggregator.rb

@@ -27,7 +27,7 @@ def record(type, attributes)
         return unless enabled?
 
         type = @type_strings[type]
-        unless type =~ EVENT_TYPE_REGEX
+        unless EVENT_TYPE_REGEX.match?(type)
           note_dropped_event(type)
           return false
         end

lib/new_relic/agent/database/obfuscation_helpers.rb

@@ -52,7 +52,7 @@ module ObfuscationHelpers
         FAILED_TO_OBFUSCATE_MESSAGE = "Failed to obfuscate SQL query - quote characters remained after obfuscation".freeze
 
         def obfuscate_single_quote_literals(sql)
-          return sql unless sql =~ COMPONENTS_REGEX_MAP[:single_quotes]
+          return sql unless sql&.match?(COMPONENTS_REGEX_MAP[:single_quotes])
 
           sql.gsub(COMPONENTS_REGEX_MAP[:single_quotes], PLACEHOLDER)
         end

lib/new_relic/agent/datastores/redis.rb

@@ -25,7 +25,7 @@ module Redis
 
         def self.format_command(command_with_args)
           if Agent.config[:'transaction_tracer.record_redis_arguments']
-            result = String.new('')
+            result = +''
 
             append_command_with_args(result, command_with_args)
 
@@ -36,7 +36,7 @@ def self.format_command(command_with_args)
         end
 
         def self.format_pipeline_commands(commands_with_args)
-          result = String.new('')
+          result = +''
 
           commands_with_args.each do |command|
             if result.length >= MAXIMUM_COMMAND_LENGTH

lib/new_relic/agent/distributed_tracing/cross_app_tracing.rb

@@ -35,7 +35,7 @@ def is_cross_app?
       end
 
       def cat_trip_id
-        cross_app_payload && cross_app_payload.referring_trip_id || transaction.guid
+        cross_app_payload&.referring_trip_id || transaction.guid
       end
 
       def cross_app_monitor
@@ -73,7 +73,7 @@ def add_message_cat_headers(headers)
       end
 
       def record_cross_app_metrics
-        if (id = cross_app_payload && cross_app_payload.id)
+        if (id = cross_app_payload&.id)
           app_time_in_seconds = [
             Process.clock_gettime(Process::CLOCK_REALTIME) - transaction.start_time,
             0.0
@@ -103,11 +103,11 @@ def record_cat_path_hash(hash)
       end
 
       def cat_referring_path_hash
-        cross_app_payload && cross_app_payload.referring_path_hash
+        cross_app_payload&.referring_path_hash
       end
 
       def append_cat_info(payload)
-        if (referring_guid = cross_app_payload && cross_app_payload.referring_guid)
+        if (referring_guid = cross_app_payload&.referring_guid)
           payload[:referring_transaction_guid] = referring_guid
         end
 

lib/new_relic/agent/distributed_tracing/trace_context.rb

@@ -78,7 +78,7 @@ def parse(format: NewRelic::FORMAT_NON_RACK,
           end
 
           def create_trace_state_entry(entry_key, payload)
-            "#{entry_key}=#{payload}".dup
+            +"#{entry_key}=#{payload}"
           end
 
           private
@@ -138,7 +138,7 @@ def extract_tracestate(format, carrier, trace_state_entry_key)
 
             payload = nil
             trace_state_size = 0
-            trace_state_vendors = String.new
+            trace_state_vendors = +''
             trace_state = header.split(COMMA).map(&:strip)
             trace_state.reject! do |entry|
               if entry == NewRelic::EMPTY_STR
@@ -223,7 +223,7 @@ def join_trace_state(trace_state_entry_size)
             max_size = MAX_TRACE_STATE_SIZE - trace_state_entry_size
             return @trace_state_entries.join(COMMA).prepend(COMMA) if @trace_state_size < max_size
 
-            joined_trace_state = ''.dup
+            joined_trace_state = +''
 
             used_size = 0