Skip to content

Draft: Fix `Override` check on classes

Lin Jen-Shin requested to merge fix-override-on-classes into master

What does this MR do and why?

Fix Override check on classes

If we want to detect on classes, we need to define override on the singleton class of the class, otherwise it's on a different level.

Override now stops using method_added hook and it should increase its accuracy. This fixes a bug in Override where override is defined but the actual method was never defined.

Previously the following will not raise anything:

class Base
  extend ::Gitlab::Utils::Override

  override :nothing
end

Because we're relying on method_added to trigger the check. Now it'll raise NotImplementedError because as soon as override is called, we queue the verification anyway.

This is inspired by !69823 (comment 679710783) where I realized that method_added is not really needed, and this leads me to figure out that there were a few cases we're using it wrongly.

How to set up and validate locally

  • Take the tests in spec/lib/gitlab/utils/override_spec.rb and run it on master to see the failures.
  • Or: Take lib/gitlab/utils/override.rb and run bundle exec rake lint:static_verification on master to see verification failures.
Edited by Lin Jen-Shin

Merge request reports

Loading