Move UnexpectedNilError into gem namespace#16
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| UNEXPECTED_NIL_ERROR_NAME = ::TypeToolkit::UnexpectedNilError.name.not_nil! | ||
| .split("::").last.not_nil! | ||
| .to_sym #: Symbol | ||
| private_constant :UNEXPECTED_NIL_ERROR_NAME |
There was a problem hiding this comment.
Un-hard-coding this, to make sure it stays in sync with the actual constant we care about.
| # `UnexpectedNilError` should never occur in well-formed code, so it should never be rescued. | ||
| # This is why it inherits from `Exception` instead of `StandardError`, | ||
| # so that bare rescues clauses (like `rescue => e`) don't rescue it. | ||
| class UnexpectedNilError < Exception # rubocop:disable Lint/InheritException |
There was a problem hiding this comment.
Since it's now TypeToolkit::UnexpectedNilError I think it'll be good to add tests to look for raise usages with that full name too.
There was a problem hiding this comment.
Done. The tests got really duplicative, so I parameterized them. See the two commits separately
There was a problem hiding this comment.
Oh nice, tests are much cleaner now. LGTM, there are some type checking errors though.
| # typed: true | ||
| # frozen_string_literal: true | ||
|
|
||
| require "type_toolkit/ext/nil_assertions" |
There was a problem hiding this comment.
Quick question: does loading this cop now patch NilClass and Kernel as a side effect? Users who configure the cop without wanting the runtime extensions would get them implicitly.
Was this the intended design, or should the constant be defined another way to avoid that side effect?
There was a problem hiding this comment.
Yes it does,
and yeah I could extract out the error class to its own file, but I'll cross that bridge if someone has a really good reason to want that. Was hoping to be opinionated/KISS for now
| class DontExpectUnexpectedNil < Base | ||
| RESTRICT_ON_SEND = [:assert_raises, :raise].freeze | ||
|
|
||
| UNEXPECTED_NIL_ERROR_NAME = ::TypeToolkit::UnexpectedNilError.name.not_nil! |
There was a problem hiding this comment.
There was a problem hiding this comment.
That's intentional duplication, to make sure this is working correctly.
9f868ae to
46be90a
Compare
| # `UnexpectedNilError` should never occur in well-formed code, so it should never be rescued. | ||
| # This is why it inherits from `Exception` instead of `StandardError`, | ||
| # so that bare rescues clauses (like `rescue => e`) don't rescue it. | ||
| class UnexpectedNilError < Exception # rubocop:disable Lint/InheritException |
There was a problem hiding this comment.
Oh nice, tests are much cleaner now. LGTM, there are some type checking errors though.
6526ab8 to
b38c687
Compare
Follow up to #7. Let's namespace the constant to
TypeToolkit::UnexpectedNilError