Add a cop that prevents putting patch_gem_version in a conditional

.changeset/unconditional-patch-gem-version.md

@@ -0,0 +1,10 @@
+---
+"@openproject/rubocop-openproject": minor
+---
+
+Add `OpenProject/NoConditionalPatchGemVersion` cop to catch `patch_gem_version` calls wrapped in a
+condition. The version check inside `patch_gem_version` is what makes the application fail to boot
+on CI when a patched gem is upgraded. The OpenTelemetry grape patch was guarded by
+`defined?(OpenTelemetry::Instrumentation::Grape::EventHandler)`, which is only defined when
+OpenTelemetry is enabled, so CI skipped the check and the gem upgrade went unnoticed. Conditions
+belong inside the block, around the patch itself.

config/default.yml

@@ -8,6 +8,11 @@ OpenProject/NoDoEndBlockWithRSpecCapybaraMatcherInExpect:
   Enabled: true
   VersionAdded: '0.1.0'
 
+OpenProject/NoConditionalPatchGemVersion:
+  Description: 'Call `patch_gem_version` unconditionally so its version check runs on CI.'
+  Enabled: true
+  VersionAdded: '0.8.0'
+
 OpenProject/NoNotImplementedError:
   Description: 'Do not raise `NotImplementedError` to signal an unimplemented abstract method.'
   Enabled: true

lib/rubocop/cop/open_project/no_conditional_patch_gem_version.rb

@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module OpenProject
+      # Checks that `patch_gem_version` is not wrapped in a condition.
+      #
+      # `patch_gem_version` raises when the patched gem is newer than the version the
+      # patch was written for, which makes the application fail to boot on CI after a
+      # gem upgrade. A condition around the call (e.g. `defined?` on a constant of a
+      # gem that is only required in production) skips that version check as well,
+      # so an outdated patch goes unnoticed. Move the condition inside the block.
+      #
+      # @example
+      #   # bad
+      #   if defined?(SomeGem::Thing)
+      #     OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+      #       SomeGem::Thing.prepend Patch
+      #     end
+      #   end
+      #
+      #   # bad
+      #   OpenProject::Patches.patch_gem_version("some-gem", "1.2.3") { apply } if Rails.env.local?
+      #
+      #   # good
+      #   OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+      #     SomeGem::Thing.prepend Patch if defined?(SomeGem::Thing)
+      #   end
+      #
+      class NoConditionalPatchGemVersion < Base
+        MSG = "`patch_gem_version` must run unconditionally so CI catches gem upgrades. " \
+              "Move the condition inside the block."
+
+        RESTRICT_ON_SEND = %i[patch_gem_version].freeze
+
+        def on_send(node)
+          return unless node.each_ancestor(:if, :case, :and, :or).any?
+
+          add_offense(node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/open_project_cops.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require_relative "open_project/add_preview_for_view_component"
+require_relative "open_project/no_conditional_patch_gem_version"
 require_relative "open_project/no_not_implemented_error"
 require_relative "open_project/no_params_in_work_package_where_id"
 require_relative "open_project/use_service_result_factory_methods"

spec/rubocop/cop/open_project/no_conditional_patch_gem_version_spec.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::OpenProject::NoConditionalPatchGemVersion, :config do
+  let(:config) { RuboCop::Config.new }
+
+  it "registers an offense when wrapped in an if block" do
+    expect_offense(<<~RUBY)
+      if defined?(SomeGem::Thing)
+        OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoConditionalPatchGemVersion: #{described_class::MSG}
+          SomeGem::Thing.prepend Patch
+        end
+      end
+    RUBY
+  end
+
+  it "registers an offense when wrapped in an unless block" do
+    expect_offense(<<~RUBY)
+      unless Rails.env.production?
+        OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoConditionalPatchGemVersion: #{described_class::MSG}
+          SomeGem::Thing.prepend Patch
+        end
+      end
+    RUBY
+  end
+
+  it "registers an offense when guarded by a modifier if" do
+    expect_offense(<<~RUBY)
+      OpenProject::Patches.patch_gem_version("some-gem", "1.2.3") { apply } if Rails.env.local?
+      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoConditionalPatchGemVersion: #{described_class::MSG}
+    RUBY
+  end
+
+  it "registers an offense when guarded by a logical operator" do
+    expect_offense(<<~RUBY)
+      defined?(SomeGem::Thing) && OpenProject::Patches.patch_gem_version("some-gem", "1.2.3") { apply }
+                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoConditionalPatchGemVersion: #{described_class::MSG}
+    RUBY
+  end
+
+  it "registers an offense when the condition is further up the tree" do
+    expect_offense(<<~RUBY)
+      if defined?(SomeGem)
+        Rails.application.config.to_prepare do
+          OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OpenProject/NoConditionalPatchGemVersion: #{described_class::MSG}
+            SomeGem::Thing.prepend Patch
+          end
+        end
+      end
+    RUBY
+  end
+
+  it "does not register an offense for a top level call" do
+    expect_no_offenses(<<~RUBY)
+      OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+        SomeGem::Thing.prepend Patch
+      end
+    RUBY
+  end
+
+  it "does not register an offense when called inside an unconditional block" do
+    expect_no_offenses(<<~RUBY)
+      initializer "some_engine.load_patches" do
+        OpenProject::Patches.patch_gem_version("some-gem", "1.2.3") do
+          SomeGem::Thing.prepend Patch
+        end
+      end
+    RUBY
+  end
+
+  it "does not register an offense when the condition is inside the block" do
+    expect_no_offenses(<<~RUBY)
+      OpenProject::Patches.patch_gem_version "some-gem", "1.2.3" do
+        SomeGem::Thing.prepend Patch if defined?(SomeGem::Thing)
+      end
+    RUBY
+  end
+end