Merge pull request #1533 from appsignal/rubocop-cleanup

Remove the RuboCop todo file and prune lint disables

Author: unflxw

.rubocop.yml

@@ -1,5 +1,3 @@
-inherit_from: .rubocop_todo.yml
-
 AllCops:
   TargetRubyVersion: 2.7
   NewCops: enable
@@ -73,6 +71,14 @@ Style/RedundantConstantBase:
 Style/RedundantLineContinuation:
   AutoCorrect: false
 
+# We rescue Exception only in deliberate, reviewed places: integrations that
+# record the error and re-raise it, and a spec helper that retries on RSpec
+# expectation failures (ExpectationNotMetError subclasses Exception). The cop
+# can't tell these from a careless rescue, so we disable it rather than scatter
+# inline disables.
+Lint/RescueException:
+  Enabled: false
+
 Lint/ConstantDefinitionInBlock:
   Exclude:
     - "spec/**/*.rb"
@@ -116,26 +122,48 @@ Naming/FileName:
   Exclude:
     - "ext/Rakefile"
 
+# Our API mirrors the agent's set_*/get_* method names, a convention used
+# throughout the gem and the extension. We follow it deliberately, so disable
+# the cop instead of scattering excludes and inline disables for it.
 Naming/AccessorMethodName:
-  Exclude:
-    - "lib/appsignal/helpers/instrumentation.rb"
-    - "lib/appsignal/transaction.rb"
+  Enabled: false
+
+# Several command methods return a boolean status (abort_installation,
+# Demo.transmit). The cop wants them renamed with a `?`, which would wrongly
+# read them as queries, so we don't run it.
+Naming/PredicateMethod:
+  Enabled: false
 
 Naming/RescuedExceptionsVariableName:
   Enabled: false
 
 Naming/VariableNumber:
   Enabled: false
 
-Metrics/ModuleLength:
+# Complexity and length metrics were never enforced here: their thresholds
+# floated for years through `rubocop --auto-gen-config` and the remaining hits
+# were silenced inline. They flag a style we deliberately accept, so instead of
+# tracking an ever-changing Max we don't run them.
+Metrics/AbcSize:
+  Enabled: false
+
+Metrics/BlockLength:
   Enabled: false
 
 Metrics/ClassLength:
   Enabled: false
 
-Metrics/BlockLength:
-  Exclude:
-    - "Rakefile"
+Metrics/CyclomaticComplexity:
+  Enabled: false
+
+Metrics/MethodLength:
+  Enabled: false
+
+Metrics/ModuleLength:
+  Enabled: false
+
+Metrics/PerceivedComplexity:
+  Enabled: false
 
 Gemspec/DevelopmentDependencies:
   Enabled: false

.rubocop_todo.yml

@@ -21,7 +21,7 @@ IGNORED_PATHS = [
   ".rubocop_todo.yml"
 ].freeze
 
-Gem::Specification.new do |gem| # rubocop:disable Metrics/BlockLength
+Gem::Specification.new do |gem|
   gem.authors = [
     "Robert Beekman",
     "Thijs Cadier",

appsignal.gemspec

@@ -136,9 +136,9 @@ def download_archive(type)
           :proxy => proxy }
       ]
       if URI.respond_to?(:open) # rubocop:disable Style/GuardClause
-        return URI.open(*args)
+        return URI.open(*args) # rubocop:disable Security/Open
       else
-        return open(*args)
+        return open(*args) # rubocop:disable Security/Open
       end
     rescue => error
       backtrace = error.backtrace.join("\n")

ext/base.rb

@@ -77,7 +77,7 @@ def link_libraries
   end
 end
 
-def have_required_function(library, func)
+def have_required_function(library, func) # rubocop:disable Naming/PredicatePrefix
   if have_func(func)
     report["build"]["dependencies"][library] = "linked"
     return

ext/extconf.rb

@@ -105,7 +105,7 @@ def testing?
     #
     # @return [void]
     # @since 0.7.0
-    def start # rubocop:disable Metrics/AbcSize
+    def start
       if ENV.fetch("_APPSIGNAL_DIAGNOSE", false)
         internal_logger.info("Skipping start in diagnose context")
         return
@@ -169,7 +169,7 @@ def start # rubocop:disable Metrics/AbcSize
     # @param block [Proc] Optional block to configure the config object.
     # @return [void]
     # @!visibility private
-    def _load_config!(env_param = nil, &block) # rubocop:disable Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
+    def _load_config!(env_param = nil, &block)
       # Ensure it's not an empty string if it's a value
       proper_env_param = env_param&.to_s&.strip
       # Unset it if it's an empty string

lib/appsignal.rb

@@ -70,7 +70,7 @@ def describe(events)
           end
         end
 
-        def deduplicate_cron!(events) # rubocop:disable Metrics/AbcSize
+        def deduplicate_cron!(events)
           # Remove redundant cron check-in events from the given list of events.
           # This is done by removing redundant *pairs* of events -- that is,
           # for each identifier, only send one complete pair of start and

lib/appsignal/check_in/event.rb

@@ -350,7 +350,7 @@ def fetch_installation_report
           path = File.expand_path("../../../ext/install.report", __dir__)
           raw_report = File.read(path)
           JSON.parse(raw_report)
-        rescue StandardError, JSON::ParserError => e # rubocop:disable Lint/ShadowedException
+        rescue StandardError => e
           {
             "parsing_error" => {
               "error" => "#{e.class}: #{e}",

lib/appsignal/cli/diagnose.rb

@@ -12,7 +12,7 @@ class Install
       EXCLUDED_ENVIRONMENTS = ["test"].freeze
 
       class << self
-        def run(push_api_key, options) # rubocop:disable Metrics/AbcSize
+        def run(push_api_key, options)
           self.coloring = options.delete(:color) { true }
           $stdout.sync = true
 
@@ -228,7 +228,7 @@ def install_for_unknown_framework(config)
           done_notice
         end
 
-        def configure(config, environments, name_overwritten) # rubocop:disable Metrics/AbcSize
+        def configure(config, environments, name_overwritten)
           install_for_capistrano
 
           ENV["APPSIGNAL_APP_ENV"] = "development"
@@ -241,7 +241,7 @@ def configure(config, environments, name_overwritten) # rubocop:disable Metrics/
           puts "  See our docs for information on the different configuration methods: "
           puts "  https://docs.appsignal.com/ruby/configuration.html"
           puts
-          loop do # rubocop:disable Metrics/BlockLength
+          loop do
             print "  Choose (1-3): "
             case ask_for_input
             when "1"

lib/appsignal/cli/install.rb

@@ -432,7 +432,7 @@ def active?
     end
 
     # @!visibility private
-    def write_to_environment # rubocop:disable Metrics/AbcSize
+    def write_to_environment
       ENV["_APPSIGNAL_ACTIVE"]                       = active?.to_s
       ENV["_APPSIGNAL_AGENT_PATH"]                   = File.expand_path("../../ext", __dir__).to_s
       ENV["_APPSIGNAL_APP_NAME"]                     = config_hash[:name]
@@ -591,7 +591,9 @@ def load_from_disk
       return unless yml_config_file?
 
       read_options = YAML::VERSION >= "4.0.0" ? { :aliases => true } : {}
+      # rubocop:disable Security/YAMLLoad
       configurations = YAML.load(ERB.new(File.read(yml_config_file)).result, **read_options)
+      # rubocop:enable Security/YAMLLoad
       config_for_this_env = configurations[env]
       if config_for_this_env
         config_for_this_env.transform_keys(&:to_sym)