Skip to content

Also verify if extending would override a class method

Lin Jen-Shin requested to merge override-consider-extend into master

What does this MR do?

Also verify if extending would override a class method

Since extending a class means including on the singleton class of the class, this should now complain this: (before this patch, it doesn't)

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:

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.

Are there points in the code the reviewer needs to double check?

I didn't write tests for all the combinations due to laziness. There are infinite ways to not override something, so we should focus on what would override instead.

Why was this MR needed?

Make sure extend is not left out.

Does this MR meet the acceptance criteria?

  • Tests added for this feature/bug
  • Conform by the code review guidelines
    • Has been reviewed by a Backend maintainer
Edited by Nick Thomas

Merge request reports