Recommend calling super in initialize

[headius]

Many users forget to call super from initialize, which can lead to tricky bugs if the superclass needs to initialize its own state.

Perhaps this should be in the style guide?

I'm not sure if it should be a hard failure, since classes that extend Object or BasicObject or a few other core types don't technically need to invoke super, but the safest pattern would be to always do so.

[bbatsov]

@headius Sounds good to me. Would you like to formulate the guideline yourself?

[headius]

Sure I'll see if I can put together a PR.

[marcandre]

👍 Please extend this rule to self.inherited and similar callbacks.

[marcandre]

👍 Please extend this rule to self.inherited and similar callbacks.

And maybe a note that typically included, prepended and extended are not needed, because writing one doesn't interrupt the a potential non-trivial call chain.

[jcoyne]

What about when you explicitly are not wanting to invoke the superclass's behavior?

[marcandre]

What about when you explicitly are not wanting to invoke the superclass's behavior?

Do you have a good example where that would be the case? As with all rules, some can be bent or broken, but as a general principle I would imagine this would be due to a weakness of the superclass...

[jcoyne]

@marcandre I would agree that using inheritance improperly could be one such reason not to call the parent initializer. Brevity is also a reason. There is no sense in calling super on a subclass of ViewComponent::Base because it doesn't do anything: https://github.com/github/view_component/blob/master/lib/view_component/base.rb#L98

[marcandre]

@jcoyne Indeed, this method has no body. Yet. By not wanting to invoke super, you are preventing that class to refactor it's initializer without breaking compatibility. Moreover I'll note that there is an initialize method and it is there only to swallow any arguments. I'm not the author, but the only reason I can see the author defined this initialize is to allow subclasses to call super instead of super().

I still think it's a good rule of thumb (and especially in external dependencies like ViewComponent::Base), but maybe you will want to chime in when we get an actual proposal for the style guide about how to word this. I'd say something "You should", maybe @headius will propose "It's recommended" and maybe you can get it down to "Some feel it's a good idea to ... so don't be surprised if you see a call to super to a method that doesn't (yet) do anything" 😆

[silva96]

there is a super common pattern that breaks this purpose:

# frozen_string_literal: true

class ApplicationService
  def self.call(*args, **kwargs, &block)
    new(*args, **kwargs, &block).__send__ :perform
  end
end

# frozen_string_literal: true

class SomeService < ApplicationService
    def initialize(a_param:)
      @a_param = a_param
    end

    private

    def perform
      puts "I'm an useless service that puts the param: #{@a_param}"
    end
  end

SomeService.call(a_param: 'something')

Why would I like to call super in SomeService initializer?

EDIT:

fortunately all my services objects are inside app/services so I added this to rubocop.yml

Lint/MissingSuper:
  Exclude:
    - 'app/services/**/*'