varvet / pundit

Minimal authorization through OO design and pure Ruby classes
MIT License
8.27k stars 629 forks source link

Do not use NotImplementedError #775

Closed hallelujah closed 11 months ago

hallelujah commented 1 year ago

This error is not appropriate for method not implemented.

https://ruby-doc.org/3.2.2/NotImplementedError.html

Raised when a feature is not implemented on the current platform. For example, methods depending on the fsync or fork system calls may raise this exception if the underlying operating system or Ruby runtime does not support them.

Note that if fork raises a NotImplementedError, then respond_to?(:fork) returns false.

This error is not rescued by this syntax as its ancestor is not StandardError but ScriptError

class Foo
  def self.bar
    raise NotImplementedError, "You must define #resolve in #{self.class}"
  rescue => e
    puts e.message
  end
end

# IRB

irb(main):015:0> Foo.bar
(irb):10:in `bar': You must define #resolve in Class (NotImplementedError)
    from (irb):15:in `<main>'
    from /Users/hery/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/irb-1.6.4/exe/irb:9:in `<top (required)>'
    from /Users/hery/.asdf/installs/ruby/3.2.2/bin/irb:25:in `load'
    from /Users/hery/.asdf/installs/ruby/3.2.2/bin/irb:25:in `<main>'
Burgestrand commented 1 year ago

TIL! Yeah, we can remove this from the generators.

I won't be doing this any time soon, but we would accept a PR with this change. Thanks for the report.