Verify for modules with Override.verify!

lib/gitlab/utils/override.rb

 module Gitlab
   module Utils
     module Override
+      class Extension
+        def self.verify_class!(klass, method_name)
+          instance_method_defined?(klass, method_name) ||
+            raise(
+              NotImplementedError.new(
+                "#{klass}\##{method_name} doesn't exist!"))
+        end
+
+        def self.instance_method_defined?(klass, name, include_super: true)
+          klass.instance_methods(include_super).include?(name) ||
+            klass.private_instance_methods(include_super).include?(name)
+        end
+
+        attr_reader :subject, :method_names
+
+        def initialize(subject)
+          @subject = subject
+        end
+
+        def add_method_name(method_name)
+          method_names << method_name
+        end
+
+        def add_class(klass)
+          classes << klass
+        end
+
+        def verify!
+          classes.each do |klass|
+            index = klass.ancestors.index(subject)
+            parents = klass.ancestors.drop(index + 1)
+
+            method_names.each do |method_name|
+              parents.any? do |parent|
+                self.class.instance_method_defined?(
+                  parent, method_name, include_super: false)
+              end ||
+                raise(
+                  NotImplementedError.new(
+                    "#{klass}\##{method_name} doesn't exist!"))
+            end
+          end
+        end
+
+        private
+
+        def method_names
+          @method_names ||= []
+        end
+
+        def classes
+          @classes ||= []
+        end
+      end
+
       # Instead of writing patterns like this:
       #
       #     def f
@@ -11,7 +66,7 @@ module Gitlab
       #
       # We could write it like:
       #
-      #     include ::Gitlab::Utils::Override
+      #     extend ::Gitlab::Utils::Override
       #
       #     override :f
       #     def f
@@ -22,9 +77,32 @@ module Gitlab
       # https://gitlab.com/gitlab-org/gitlab-ee/issues/1819
       def override(method_name)
         return unless ENV['STATIC_VERIFICATION']
-        return if instance_methods.include?(method_name)
 
-        raise NotImplementedError.new("Method #{method_name} doesn't exist")
+        if kind_of?(Class)
+          Extension.verify_class!(self, method_name)
+        else # We delay the check for modules
+          Override.extensions[self] ||= Extension.new(self)
+          Override.extensions[self].add_method_name(method_name)
+        end
+      end
+
+      def included(base)
+        super
+
+        if base.kind_of?(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
+
+      def self.verify!
+        extensions.values.each(&:verify!)
       end
     end
   end

lib/tasks/lint.rake

 unless Rails.env.production?
   namespace :lint do
+    task :static_verification_env do
+      ENV['STATIC_VERIFICATION'] = 'true'
+    end
+
+    desc "GitLab | lint | Static verification"
+    task static_verification: %w[
+      lint:static_verification_env
+      dev:load
+    ] do
+      Gitlab::Utils::Override.verify!
+    end
+
     desc "GitLab | lint | Lint JavaScript files using ESLint"
     task :javascript do
       Rake::Task['eslint'].invoke

scripts/static-analysis

@@ -13,7 +13,7 @@ tasks = [
   %w[yarn run eslint],
   %w[bundle exec rubocop --parallel],
   %w[bundle exec rake gettext:lint],
-  %w[env STATIC_VERIFICATION=true bundle exec rake dev:load],
+  %w[bundle exec rake lint:static_verification],
   %w[scripts/lint-changelog-yaml],
   %w[scripts/lint-conflicts.sh]
 ]

spec/lib/gitlab/utils/override_spec.rb

@@ -3,32 +3,75 @@ require 'spec_helper'
 describe Gitlab::Utils::Override do
   let(:base) { Struct.new(:good) }
 
-  let(:derived) do
-    Class.new(base) do
-      extend Gitlab::Utils::Override # rubocop:disable RSpec/DescribedClass
-    end
-  end
+  let(:derived) { Class.new(base).tap { |m| m.extend described_class } }
+  let(:extension) { Module.new.tap { |m| m.extend described_class } }
+
+  let(:prepending_class) { base.tap { |m| m.prepend extension } }
+  let(:including_class) { base.tap { |m| m.include extension } }
 
-  def good
-    derived.module_eval do
+  let(:klass) { subject }
+
+  def good(mod)
+    mod.module_eval do
       override :good
       def good
         super.succ
       end
     end
 
-    derived
+    mod
   end
 
-  def bad
-    derived.module_eval do
+  def bad(mod)
+    mod.module_eval do
       override :bad
       def bad
         true
       end
     end
 
-    derived
+    mod
+  end
+
+  shared_examples 'checking as intended' do
+    it 'checks ok for overriding method' do
+      good(subject)
+      result = klass.new(0).good
+
+      expect(result).to eq(1)
+      described_class.verify!
+    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)
+    end
+  end
+
+  shared_examples 'nothing happened' do
+    it 'does not complain when it is overriding something' do
+      good(subject)
+      result = klass.new(0).good
+
+      expect(result).to eq(1)
+      described_class.verify!
+    end
+
+    it 'does not complain when it is not overriding anything' do
+      bad(subject)
+      result = klass.new(0).bad
+
+      expect(result).to eq(true)
+      described_class.verify!
+    end
+  end
+
+  before do
+    # Make sure we're not touching the internal cache
+    allow(described_class).to receive(:extensions).and_return({})
   end
 
   describe '#override' do
@@ -37,28 +80,78 @@ describe Gitlab::Utils::Override do
         stub_env('STATIC_VERIFICATION', 'true')
       end
 
-      it 'checks ok for overriding method' do
-        result = good.new(0).good
+      context 'when subject is a class' do
+        subject { derived }
 
-        expect(result).to eq(1)
+        it_behaves_like 'checking as intended'
       end
 
-      it 'raises NotImplementedError when it is not overriding anything' do
-        expect { bad }.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 'checking as intended'
+      end
+
+      context 'when subject is a module, and class is including it' do
+        subject { extension }
+        let(:klass) { including_class }
+
+        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)
+        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)
+        end
       end
     end
+  end
 
   context 'when STATIC_VERIFICATION is not set' do
-      it 'does not complain when it is overriding anything' do
-        result = good.new(0).good
+    before do
+      stub_env('STATIC_VERIFICATION', nil)
+    end
+
+    context 'when subject is a class' do
+      subject { derived }
+
+      it_behaves_like 'nothing happened'
+    end
+
+    context 'when subject is a module, and class is prepending it' do
+      subject { extension }
+      let(:klass) { prepending_class }
+
+      it_behaves_like 'nothing happened'
+    end
+
+    context 'when subject is a module, and class is including it' do
+      subject { extension }
+      let(:klass) { including_class }
+
+      it 'does not complain when it is overriding something' do
+        good(subject)
+        result = klass.new(0).good
 
-        expect(result).to eq(1)
+        expect(result).to eq(0)
+        described_class.verify!
       end
 
       it 'does not complain when it is not overriding anything' do
-        result = bad.new(0).bad
+        bad(subject)
+        result = klass.new(0).bad
 
         expect(result).to eq(true)
+        described_class.verify!
       end
     end
   end