cookpad / global-style-guides

Official style guides for Cookpad Global
67 stars 16 forks source link

Add prefer-custom-error-class-in-abstract-methods #188

Closed jesseclark closed 3 years ago

jesseclark commented 3 years ago

What

Adds a section to the Ruby style guide recommending using a custom error class over a plain raise or raising NotImplementedError in abstract methods.

Why

Per the outcome of this RFC

jesseclark commented 3 years ago

Thank you! 👏🏻

What do you think of adding the Rubocop rule too?

@davidstosik Looking at the docs for that Rubocop rule again, it says:

This cop looks for error classes inheriting from Exception and its standard library subclasses, excluding subclasses of StandardError.

and the list of "illegal classes" to inherit from is:

ILLEGAL_CLASSES =

    %w[
      Exception
      SystemStackError
      NoMemoryError
      SecurityError
      NotImplementedError
      LoadError
      SyntaxError
      ScriptError
      Interrupt
      SignalException
      SystemExit
    ].freeze

So it seems that this cop would flag something like this: class UnimplementedAbstractMethodError < NotImplementedError

but not: raise NotImplementedError which is what we want to discourage.

Also, I checked to make sure that this cop was not explicitly disabled in our rubocop config and tested locally and this cop is enabled by default and will flag the inheritance example above.

We could possibly change the enforced style to standard_error instead of runtime_error if that is what our preference would be, but I do not have a strong opinion on that.

davidstosik commented 3 years ago

@jesseclark You are right, I was mistaken, that cop acts on inheritance, not raise. And there does not seem to be a cop, at the moment, that would achieve what I'm suggesting. Thanks again for the doc addition! 👍🏻