Also verify if extending would override a class methodoverride-consider-extend

Since extending a class means including on the singleton class of the
class, this should now complain this:

``` ruby
module M
  extend Gitlab::Utils::Override

  override :f
  def f
    super.succ
  end
end

class C
  extend M

  def self.f
    0
  end
end
```

It should complain because `C.f` wasn't calling `M#f`.
This should pass verification:

``` ruby
module M
  extend Gitlab::Utils::Override

  override :f
  def f
    super.succ
  end
end

class B
  def self.f
    0
  end
end

class C < B
  extend M
end
```

Because `C.f` would now call `M#f`, and `M#f` does override something.

2 files changed, 121 insertions, 65 deletions

diff --git a/lib/gitlab/utils/override.rb b/lib/gitlab/utils/override.rb
index 8bf6bcb1fe2..7b2a62fed48 100644
--- a/lib/gitlab/utils/override.rb
+++ b/lib/gitlab/utils/override.rb
@@ -87,18 +87,28 @@ module Gitlab
       end
 
       def included(base = nil)
-        return super if base.nil? # Rails concern, ignoring it
+        super
+
+        queue_verification(base)
+      end
 
+      alias_method :prepended, :included
+
+      def extended(mod)
         super
 
+        queue_verification(mod.singleton_class)
+      end
+
+      def queue_verification(base)
+        return unless ENV['STATIC_VERIFICATION']
+
         if base.is_a?(Class) # We could check for Class in `override`
           # This could be `nil` if `override` was never called
           Override.extensions[self]&.add_class(base)
         end
       end
 
-      alias_method :prepended, :included
-
       def self.extensions
         @extensions ||= {}
       end
diff --git a/spec/lib/gitlab/utils/override_spec.rb b/spec/lib/gitlab/utils/override_spec.rb
index 7c97cee982a..fc08ebcfc6d 100644
--- a/spec/lib/gitlab/utils/override_spec.rb
+++ b/spec/lib/gitlab/utils/override_spec.rb
@@ -1,7 +1,13 @@
-require 'spec_helper'
+require 'fast_spec_helper'
 
 describe Gitlab::Utils::Override do
-  let(:base) { Struct.new(:good) }
+  let(:base) do
+    Struct.new(:good) do
+      def self.good
+        0
+      end
+    end
+  end
 
   let(:derived) { Class.new(base).tap { |m| m.extend described_class } }
   let(:extension) { Module.new.tap { |m| m.extend described_class } }
@@ -9,6 +15,14 @@ describe Gitlab::Utils::Override do
   let(:prepending_class) { base.tap { |m| m.prepend extension } }
   let(:including_class) { base.tap { |m| m.include extension } }
 
+  let(:prepending_class_methods) do
+    base.tap { |m| m.singleton_class.prepend extension }
+  end
+
+  let(:extending_class_methods) do
+    base.tap { |m| m.extend extension }
+  end
+
   let(:klass) { subject }
 
   def good(mod)
@@ -36,7 +50,7 @@ describe Gitlab::Utils::Override do
   shared_examples 'checking as intended' do
     it 'checks ok for overriding method' do
       good(subject)
-      result = klass.new(0).good
+      result = instance.good
 
       expect(result).to eq(1)
       described_class.verify!
@@ -45,7 +59,25 @@ describe Gitlab::Utils::Override do
     it 'raises NotImplementedError when it is not overriding anything' do
       expect do
         bad(subject)
-        klass.new(0).bad
+        instance.bad
+        described_class.verify!
+      end.to raise_error(NotImplementedError)
+    end
+  end
+
+  shared_examples 'checking as intended, nothing was overridden' do
+    it 'raises NotImplementedError because it is not overriding it' do
+      expect do
+        good(subject)
+        instance.good
+        described_class.verify!
+      end.to raise_error(NotImplementedError)
+    end
+
+    it 'raises NotImplementedError when it is not overriding anything' do
+      expect do
+        bad(subject)
+        instance.bad
         described_class.verify!
       end.to raise_error(NotImplementedError)
     end
@@ -54,7 +86,7 @@ describe Gitlab::Utils::Override do
   shared_examples 'nothing happened' do
     it 'does not complain when it is overriding something' do
       good(subject)
-      result = klass.new(0).good
+      result = instance.good
 
       expect(result).to eq(1)
       described_class.verify!
@@ -62,7 +94,7 @@ describe Gitlab::Utils::Override do
 
     it 'does not complain when it is not overriding anything' do
       bad(subject)
-      result = klass.new(0).bad
+      result = instance.bad
 
       expect(result).to eq(true)
       described_class.verify!
@@ -75,83 +107,97 @@ describe Gitlab::Utils::Override do
   end
 
   describe '#override' do
-    context 'when STATIC_VERIFICATION is set' do
-      before do
-        stub_env('STATIC_VERIFICATION', 'true')
-      end
+    context 'when instance is klass.new(0)' do
+      let(:instance) { klass.new(0) }
 
-      context 'when subject is a class' do
-        subject { derived }
+      context 'when STATIC_VERIFICATION is set' do
+        before do
+          stub_env('STATIC_VERIFICATION', 'true')
+        end
 
-        it_behaves_like 'checking as intended'
-      end
+        context 'when subject is a class' do
+          subject { derived }
+
+          it_behaves_like 'checking as intended'
+        end
+
+        context 'when subject is a module, and class is prepending it' do
+          subject { extension }
+          let(:klass) { prepending_class }
+
+          it_behaves_like 'checking as intended'
+        end
 
-      context 'when subject is a module, and class is prepending it' do
-        subject { extension }
-        let(:klass) { prepending_class }
+        context 'when subject is a module, and class is including it' do
+          subject { extension }
+          let(:klass) { including_class }
 
-        it_behaves_like 'checking as intended'
+          it_behaves_like 'checking as intended, nothing was overridden'
+        end
       end
 
-      context 'when subject is a module, and class is including it' do
-        subject { extension }
-        let(:klass) { including_class }
+      context 'when STATIC_VERIFICATION is not set' do
+        before do
+          stub_env('STATIC_VERIFICATION', nil)
+        end
 
-        it 'raises NotImplementedError because it is not overriding it' do
-          expect do
-            good(subject)
-            klass.new(0).good
-            described_class.verify!
-          end.to raise_error(NotImplementedError)
+        context 'when subject is a class' do
+          subject { derived }
+
+          it_behaves_like 'nothing happened'
         end
 
-        it 'raises NotImplementedError when it is not overriding anything' do
-          expect do
-            bad(subject)
-            klass.new(0).bad
-            described_class.verify!
-          end.to raise_error(NotImplementedError)
+        context 'when subject is a module, and class is prepending it' do
+          subject { extension }
+          let(:klass) { prepending_class }
+
+          it_behaves_like 'nothing happened'
         end
-      end
-    end
-  end
 
-  context 'when STATIC_VERIFICATION is not set' do
-    before do
-      stub_env('STATIC_VERIFICATION', nil)
-    end
+        context 'when subject is a module, and class is including it' do
+          subject { extension }
+          let(:klass) { including_class }
 
-    context 'when subject is a class' do
-      subject { derived }
+          it 'does not complain when it is overriding something' do
+            good(subject)
+            result = instance.good
 
-      it_behaves_like 'nothing happened'
-    end
+            expect(result).to eq(0)
+            described_class.verify!
+          end
 
-    context 'when subject is a module, and class is prepending it' do
-      subject { extension }
-      let(:klass) { prepending_class }
+          it 'does not complain when it is not overriding anything' do
+            bad(subject)
+            result = instance.bad
 
-      it_behaves_like 'nothing happened'
+            expect(result).to eq(true)
+            described_class.verify!
+          end
+        end
+      end
     end
 
-    context 'when subject is a module, and class is including it' do
-      subject { extension }
-      let(:klass) { including_class }
+    context 'when instance is klass' do
+      let(:instance) { klass }
 
-      it 'does not complain when it is overriding something' do
-        good(subject)
-        result = klass.new(0).good
+      context 'when STATIC_VERIFICATION is set' do
+        before do
+          stub_env('STATIC_VERIFICATION', 'true')
+        end
 
-        expect(result).to eq(0)
-        described_class.verify!
-      end
+        context 'when subject is a module, and class is prepending it' do
+          subject { extension }
+          let(:klass) { prepending_class_methods }
 
-      it 'does not complain when it is not overriding anything' do
-        bad(subject)
-        result = klass.new(0).bad
+          it_behaves_like 'checking as intended'
+        end
 
-        expect(result).to eq(true)
-        described_class.verify!
+        context 'when subject is a module, and class is extending it' do
+          subject { extension }
+          let(:klass) { extending_class_methods }
+
+          it_behaves_like 'checking as intended, nothing was overridden'
+        end
       end
     end
   end