Skip to content

Commit

Permalink
Log an error message if a Sorbet signature raises an exception
Browse files Browse the repository at this point in the history
  • Loading branch information
phil-monroe committed Aug 8, 2024
1 parent e0cb492 commit 8ba5ee1
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 12 deletions.
9 changes: 8 additions & 1 deletion lib/tapioca/commands/abstract_gem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,14 @@ def compile_gem_rbi(gem)
) if @file_header

rbi.root = Runtime::Trackers::Autoload.with_disabled_exits do
Tapioca::Gem::Pipeline.new(gem, include_doc: @include_doc, include_loc: @include_loc).compile
Tapioca::Gem::Pipeline.new(
gem,
include_doc: @include_doc,
include_loc: @include_loc,
error_handler: ->(error) {
say_error(error, :bold, :red)
},
).compile
end

merge_with_exported_rbi(gem, rbi) if @include_exported_rbis
Expand Down
14 changes: 13 additions & 1 deletion lib/tapioca/gem/listeners/methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def compile_method(tree, symbol_name, constant, method, visibility = RBI::Public
return unless method_owned_by_constant?(method, constant)
return if @pipeline.symbol_in_payload?(symbol_name) && !@pipeline.method_in_gem?(method)

signature = signature_of(method)
signature = lookup_signature_of(method)
method = T.let(signature.method, UnboundMethod) if signature

method_name = method.name.to_s
Expand Down Expand Up @@ -211,6 +211,18 @@ def initialize_method_for(constant)
def ignore?(event)
event.is_a?(Tapioca::Gem::ForeignScopeNodeAdded)
end

sig { params(method: UnboundMethod).returns(T.untyped) }
def lookup_signature_of(method)
signature_of!(method)
rescue LoadError, StandardError => error
@pipeline.error_handler.call(<<~MSG)
Unable to compile signature for method: #{method.owner}##{method.name}
Exception raised when loading signature: #{error.inspect}
MSG

nil
end
end
end
end
Expand Down
20 changes: 18 additions & 2 deletions lib/tapioca/gem/pipeline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,28 @@ class Pipeline
sig { returns(Gemfile::GemSpec) }
attr_reader :gem

sig { params(gem: Gemfile::GemSpec, include_doc: T::Boolean, include_loc: T::Boolean).void }
def initialize(gem, include_doc: false, include_loc: false)
sig { returns(T.proc.params(error: String).void) }
attr_reader :error_handler

sig do
params(
gem: Gemfile::GemSpec,
error_handler: T.proc.params(error: String).void,
include_doc: T::Boolean,
include_loc: T::Boolean,
).void
end
def initialize(
gem,
error_handler:,
include_doc: false,
include_loc: false
)
@root = T.let(RBI::Tree.new, RBI::Tree)
@gem = gem
@seen = T.let(Set.new, T::Set[String])
@alias_namespace = T.let(Set.new, T::Set[String])
@error_handler = error_handler

@events = T.let([], T::Array[Gem::Event])

Expand Down
2 changes: 1 addition & 1 deletion lib/tapioca/loaders/loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def require_helper(file)
# The `eager_load_paths` method still exists, but doesn't return all paths anymore and causes Tapioca to miss some
# engine paths. The following commit is the change:
# https://github.com/rails/rails/commit/ebfca905db14020589c22e6937382e6f8f687664
sig { params(engine: T.class_of(Rails::Engine)).returns(T::Array[String]) }
T::Sig::WithoutRuntime.sig { params(engine: T.class_of(Rails::Engine)).returns(T::Array[String]) }
def eager_load_paths(engine)
config = engine.config

Expand Down
7 changes: 6 additions & 1 deletion lib/tapioca/runtime/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,13 @@ def qualified_name_of(constant)
end

sig { params(method: T.any(UnboundMethod, Method)).returns(T.untyped) }
def signature_of(method)
def signature_of!(method)
T::Utils.signature_for_method(method)
end

sig { params(method: T.any(UnboundMethod, Method)).returns(T.untyped) }
def signature_of(method)
signature_of!(method)
rescue LoadError, StandardError
nil
end
Expand Down
2 changes: 1 addition & 1 deletion lib/tapioca/static/symbol_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def symbols_from_paths(paths)

private

sig { returns(T::Array[T.class_of(Rails::Engine)]) }
T::Sig::WithoutRuntime.sig { returns(T::Array[T.class_of(Rails::Engine)]) }
def engines
@engines ||= T.let(
if Object.const_defined?("Rails::Engine")
Expand Down
44 changes: 39 additions & 5 deletions spec/tapioca/gem/pipeline_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,24 @@ class Tapioca::Gem::PipelineSpec < Minitest::HooksSpec
include Tapioca::SorbetHelper

DEFAULT_GEM_NAME = T.let("the-default-gem", String)
NO_UNHANDLED_PIPELINE_COMPILATION_ERRORS_MESSAGE = <<~MSG
Tapioca::Gem::Pipeline#compile should not have any unhandled errors reported.
Please either fix your changes to ensure no errors are reported, or explicity pass
in a custom `error_handler: ->(err) { ... }` to the `compile()` method to handle
expected errors.
MSG

private

sig { params(gem_name: String, include_doc: T::Boolean, include_loc: T::Boolean).returns(String) }
def compile(gem_name = DEFAULT_GEM_NAME, include_doc: false, include_loc: false)
sig do
params(
gem_name: String,
include_doc: T::Boolean,
include_loc: T::Boolean,
reported_errors_expected: T::Boolean,
).returns(String)
end
def compile(gem_name = DEFAULT_GEM_NAME, include_doc: false, include_loc: false, reported_errors_expected: false)
mock_gem_path = mock_gems[gem_name]

# If we are compiling for a mock gem, we need to create a fake gemspec
Expand All @@ -36,14 +49,20 @@ def compile(gem_name = DEFAULT_GEM_NAME, include_doc: false, include_loc: false)
gem = Tapioca::Gemfile::GemSpec.new(Gem::Specification.stubs[gem_name].first)

# push it through the pipeline
tree = Tapioca::Gem::Pipeline.new(gem, include_doc: include_doc, include_loc: include_loc).compile
tree = Tapioca::Gem::Pipeline.new(
gem,
include_doc: include_doc,
include_loc: include_loc,
error_handler: ->(error) { reported_errors << error },
).compile

# NOTE: This is not returning a `RBI::File`.
# The following test suite is based on the string output of the `RBI::Tree` rather
# than the, now used, `RBI::File`. The file output includes the sigils, comments, etc.
# We should eventually update these tests to be based on the `RBI::File`.
Tapioca::DEFAULT_RBI_FORMATTER.format_tree(tree)

assert_empty(reported_errors, NO_UNHANDLED_PIPELINE_COMPILATION_ERRORS_MESSAGE) unless reported_errors_expected
tree.string
end

Expand All @@ -65,6 +84,11 @@ def mock_gems
@gems
end

sig { returns(T::Array[String]) }
def reported_errors
@reported_errors ||= T.let([], T.nilable(T::Array[String]))
end

describe Tapioca::Gem::Pipeline do
before do
# We need to undefine and unload `ActiveSupport` so that the test object
Expand Down Expand Up @@ -3861,7 +3885,7 @@ module ActiveSupport::Rescuable::ClassMethods; end
assert_equal(output, compile)
end

it "skips signatures if they raise" do
it "skips signatures and reports the errors if the sig block raises" do
add_ruby_file("foo.rb", <<~RUBY)
class Foo
extend(T::Sig)
Expand Down Expand Up @@ -3900,7 +3924,17 @@ def foo(*args, &blk); end
end
RBI

assert_equal(output, compile)
assert_equal(output, compile(reported_errors_expected: true))
assert_equal(reported_errors, [
<<~ERROR,
Unable to compile signature for method: Foo#bar
Exception raised when loading signature: #<LoadError: LoadError>
ERROR
<<~ERROR,
Unable to compile signature for method: Foo#foo
Exception raised when loading signature: #<ArgumentError: ArgumentError>
ERROR
])
end

it "handles signatures with attached classes" do
Expand Down
57 changes: 57 additions & 0 deletions spec/tapioca/runtime/reflection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,29 @@ def equal?(other)
end
end

class SignatureFoo
extend T::Sig

sig { returns(String) }
def good_method
"Thank you."
end

# NOTE: leveraging eval to avoid actual sorbet typechecking
eval <<~RUBY
sig do
raise ArgumentError
end
def bad_method
"oh no..."
end
RUBY

def unknown_method
' ¯\_(ツ)_/¯ '
end
end

class ReflectionSpec < Minitest::Spec
describe Tapioca::Runtime::Reflection do
it "might return the wrong results without Reflection helpers" do
Expand Down Expand Up @@ -111,6 +134,40 @@ class ReflectionSpec < Minitest::Spec
it "returns top level anchored name for named class" do
assert_equal("::Tapioca::Runtime::LyingFoo", Runtime::Reflection.qualified_name_of(LyingFoo))
end

describe "signature_for" do
it "returns a valid signature" do
method = SignatureFoo.instance_method(:good_method)
refute_nil(Runtime::Reflection.signature_of(method))
end

it "returns nil when a signature is not defined" do
method = SignatureFoo.instance_method(:unknown_method)
assert_nil(Runtime::Reflection.signature_of(method))
end

it "returns nil when a signature block raises an exception" do
method = SignatureFoo.instance_method(:bad_method)
assert_nil(Runtime::Reflection.signature_of(method))
end
end

describe "signature_for!" do
it "returns a valid signature" do
method = SignatureFoo.instance_method(:good_method)
refute_nil(Runtime::Reflection.signature_of!(method))
end

it "returns nil when a signature is not defined" do
method = SignatureFoo.instance_method(:unknown_method)
assert_nil(Runtime::Reflection.signature_of!(method))
end

it "returns nil when a signature block raises an exception" do
method = SignatureFoo.instance_method(:bad_method)
assert_raises(ArgumentError) { Runtime::Reflection.signature_of!(method) }
end
end
end
end
end
Expand Down

0 comments on commit 8ba5ee1

Please sign in to comment.