Skip to content

Explicitly give `override` for `EE::Project#feature_available?`

Lin Jen-Shin requested to merge be-explicit-about-override into master

What does this MR do?

Explicitly give override for EE::Project#feature_available?.

This is my follow up from #224443 (comment 369917854) which will not prevent the issue, but it might give some rooms for the future improvement.

This will not help because the current arity verification is very relax when there's optional arguments:

def arity_compatible?(sub_method_arity, super_method_arity)
  if sub_method_arity >= 0 && super_method_arity >= 0
    # Regular arguments
    sub_method_arity == super_method_arity
  else
    # It's too complex to check this case, just allow sub-method having negative arity
    # But we don't allow sub_method_arity > 0 yet super_method_arity < 0
    sub_method_arity < 0
  end
end

In the future we may improve this part, but given the natural of dynamics of Ruby, it might never be perfect because we wouldn't know if a variable is supposed to be a superclass interface or the subclass interface, therefore even if it can pass the check correctly we may still misuse it.

But I guess this will still somehow help by "hinting" it. It's also odd that EE::Project#feature_available? is overriding Featurable#feature_available? but that's what's done. I'll suggest we merge Featurable into ProjectFeature because that's the only place we're using it, and move EE::Project#feature_available? to EE::ProjectFeature#feature_available? but it'll require more works...

Here's it's just a simple change.

Does this MR meet the acceptance criteria?

Conformity

Merge request reports