Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/rubocop/cop/type_toolkit/dont_expect_unexpected_nil.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
# typed: true
# frozen_string_literal: true

require "type_toolkit/ext/nil_assertions"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


module RuboCop
module Cop
module TypeToolkit
# This cop detects attempts to raise, rescue, or otherwise use the `UnexpectedNilError` class.
class DontExpectUnexpectedNil < Base
RESTRICT_ON_SEND = [:assert_raises, :raise].freeze

UNEXPECTED_NIL_ERROR_NAME = ::TypeToolkit::UnexpectedNilError.name.not_nil!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's intentional duplication, to make sure this is working correctly.

.split("::").last.not_nil!
.to_sym #: Symbol
private_constant :UNEXPECTED_NIL_ERROR_NAME
Comment on lines +13 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un-hard-coding this, to make sure it stays in sync with the actual constant we care about.


#: (RuboCop::AST::SendNode) -> void
def on_send(node)
case node.method_name
Expand Down Expand Up @@ -55,7 +62,12 @@ def on_investigation_end

#: (RuboCop::AST::ConstNode) -> bool
def unexpected_nil_error?(node)
node.short_name == :UnexpectedNilError && (node.namespace.nil? || node.namespace.cbase_type?)
return false unless node.short_name == UNEXPECTED_NIL_ERROR_NAME

ns = node.namespace
return true if ns.nil? || ns.cbase_type?

ns.const_type? && ns.short_name == :TypeToolkit && (ns.namespace.nil? || ns.namespace.cbase_type?)
end

# Check for `raise UnexpectedNilError`
Expand Down
20 changes: 11 additions & 9 deletions lib/type_toolkit/ext/nil_assertions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@ class NilClass
# @override
#: -> bot
def not_nil!
raise UnexpectedNilError
raise TypeToolkit::UnexpectedNilError
end
end

# An error raised when calling `#not_nil!` on a `nil` value.
#
# `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
def initialize(message = "Called `not_nil!` on nil.")
super(message)
module TypeToolkit
# An error raised when calling `#not_nil!` on a `nil` value.
#
# `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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The tests got really duplicative, so I parameterized them. See the two commits separately

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, tests are much cleaner now. LGTM, there are some type checking errors though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

def initialize(message = "Called `not_nil!` on nil.")
super
end
end
end
1 change: 1 addition & 0 deletions sorbet/config
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
--ignore=vendor/
--enable-experimental-rbs-comments
--suppress-payload-superclass-redefinition-for=RDoc::Markup::Heading
--disable-watchman
39 changes: 38 additions & 1 deletion sorbet/rbi/shims/minitest.rbi
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,42 @@
# frozen_string_literal: true

module Minitest
class Spec < Minitest::Test; end
class Spec < Minitest::Test
extend Minitest::Spec::DSL

include RuboCop::Minitest::AssertOffense

sig do
params(
desc: T.anything,
block: T.proc.bind(T.self_type).void,
).void
end
def it(desc = T.unsafe(nil), &block); end

module DSL
has_attached_class!(:out)

sig do
params(
desc: T.anything,
block: T.proc.bind(T.attached_class).void,
).void
end
def describe(desc, &block); end

sig do
params(
desc: T.anything,
block: T.proc.bind(T.attached_class).void,
).void
end
def it(desc = T.unsafe(nil), &block); end

sig do
params(block: T.proc.bind(T.attached_class).void).void
end
def before(&block); end
end
end
end
Loading
Loading