-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sorbet support (initial) #3
base: master
Are you sure you want to change the base?
Changes from 23 commits
6dd84fb
d22a223
019e9ef
3518df8
12f43a9
f704ff4
a6f93a2
fe22912
46ad16f
c7f6159
3502874
a2615a4
3930a0c
e1201a4
00fc30c
e54a28d
8752f4a
4adac91
cde36ce
ba6de3b
1350dde
18ac177
0b6b714
f8b740f
28f235a
16f2ffd
d1fe510
24e99b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
root = true | ||
|
||
[*] | ||
indent_style = space | ||
indent_size = 2 | ||
end_of_line = lf | ||
charset = utf-8 | ||
trim_trailing_whitespace = true | ||
insert_final_newline = true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# frozen_string_literal: true | ||
|
||
require "sorbet-runtime" | ||
|
||
# Let methods with sig receive double/instance_double arguments | ||
T::Configuration.call_validation_error_handler = lambda do |signature, opts| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It reminds me this RSpec Sorbet integration: https://github.com/samuelgiles/rspec-sorbet/blob/4d0e6479453b3b4ef36d2d6a2df8282ae559368b/lib/rspec/sorbet/doubles.rb#L104 Are we doing something similar here? Maybe, we can rely on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It uses a similar approach and calls the same method The first problem I see here is that they may not work together at the moment. The reason for that is this line: I will try to reproduce the error and write a comment about the result below. Anyway, it should be easy to fix. RSpec Sorbet uses a correct approach to override this method. It should be possible to integrate RSpec Sorbet gem if this is needed I guess. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed this issue in another branch: link to the commit |
||
is_mocked = opts[:value].is_a?(RSpec::Mocks::Double) || opts[:value].is_a?(RSpec::Mocks::VerifyingDouble) | ||
unless is_mocked | ||
return T::Configuration.send(:call_validation_error_handler_default, signature, opts) | ||
end | ||
|
||
# https://github.com/rspec/rspec-mocks/blob/main/lib/rspec/mocks/verifying_double.rb | ||
# https://github.com/rspec/rspec-mocks/blob/v3.12.3/lib/rspec/mocks/test_double.rb | ||
doubled_class = if opts[:value].is_a? RSpec::Mocks::Double | ||
doubled_class_name = opts[:value].instance_variable_get :@name | ||
Kernel.const_get(doubled_class_name) | ||
elsif opts[:value].is_a? RSpec::Mocks::VerifyingDouble | ||
opts[:value].instance_variable_get(:@doubled_module).send(:object) | ||
end | ||
are_related = doubled_class <= opts[:type].raw_type | ||
return if are_related | ||
|
||
return T::Configuration.send(:call_validation_error_handler_default, signature, opts) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# frozen_string_literal: true | ||
|
||
gem "sorbet-runtime", "~> 0.5" | ||
require "sorbet-runtime" | ||
require "set" | ||
require "pathname" | ||
|
||
require "mock_suey/sorbet_rspec" | ||
require "mock_suey/ext/instance_class" | ||
|
||
module MockSuey | ||
module TypeChecks | ||
using Ext::InstanceClass | ||
|
||
class Sorbet | ||
RAISE_ON_MISSING_MESSAGE = "Please, set raise_on_missing_types to false to disable this error. Details: https://github.com/test-prof/mock-suey#raise_on_missing_types" | ||
|
||
def initialize(load_dirs: []) | ||
@load_dirs = Array(load_dirs) | ||
end | ||
|
||
def typecheck!(method_call, raise_on_missing: false) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a room for improvement: the variable |
||
original_method_sig = get_original_method_sig(method_call) | ||
return sig_is_missing(raise_on_missing) unless original_method_sig | ||
|
||
unbound_mocked_method = get_unbound_mocked_method(method_call) | ||
override_mocked_method_to_avoid_recursion(method_call) | ||
|
||
validate_call!( | ||
instance: method_call.mocked_obj, | ||
original_method: unbound_mocked_method, | ||
method_sig: original_method_sig, | ||
args: method_call.arguments, | ||
blk: method_call.block | ||
) | ||
end | ||
|
||
private | ||
|
||
def validate_call!(instance:, original_method:, method_sig:, args:, blk:) | ||
T::Private::Methods::CallValidation.validate_call( | ||
instance, | ||
original_method, | ||
method_sig, | ||
args, | ||
blk | ||
) | ||
end | ||
|
||
def get_unbound_mocked_method(method_call) | ||
method_call.mocked_obj.method(method_call.method_name).unbind | ||
end | ||
|
||
def get_original_method_sig(method_call) | ||
unbound_original_method = get_unbound_original_method(method_call) | ||
T::Utils.signature_for_method(unbound_original_method) | ||
end | ||
|
||
def get_unbound_original_method(method_call) | ||
method_name = method_call.method_name | ||
mocked_obj = method_call.mocked_obj | ||
is_a_class = mocked_obj.is_a? Class | ||
|
||
if is_a_class | ||
method_call.mocked_obj.method(method_name) | ||
else | ||
method_call.receiver_class.instance_method(method_name) | ||
end | ||
end | ||
|
||
def sig_is_missing(raise_on_missing) | ||
raise MissingSignature, RAISE_ON_MISSING_MESSAGE if raise_on_missing | ||
end | ||
|
||
def override_mocked_method_to_avoid_recursion(method_call) | ||
method_name = method_call.method_name | ||
mocked_obj = method_call.mocked_obj | ||
return_value = method_call.return_value | ||
mocked_obj.define_singleton_method(method_name) { |*args, &block| return_value } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Defining a singleton method on a mocked object might cause a problem. It's better to either |
||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# frozen_string_literal: true | ||
|
||
describe "Sorbet integration tests" do | ||
context "RSpec" do | ||
let(:env) { {"TYPED_SORBET" => "true"} } | ||
|
||
it "has no affect on simple double" do | ||
status, output = run_rspec("double_sorbet", env: env) | ||
|
||
expect(status).to be_success | ||
expect(output).to include("6 examples, 0 failures") | ||
end | ||
|
||
context "instance_double_sorbet" do | ||
it "enhances instance_double without extensions" do | ||
status, output = run_rspec("instance_double_sorbet", env: env) | ||
|
||
expect(status).not_to be_success | ||
expect(output).to include("16 examples") | ||
expect(output).to include("5 failures") | ||
expect(output).to include("AccountantSorbet#tax_rate_for") | ||
expect(output).to include("AccountantSorbet#net_pay") | ||
expect(output).to include("AccountantSorbet#tax_for") | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# frozen_string_literal: true | ||
|
||
$LOAD_PATH.unshift File.expand_path("../../../../lib", __FILE__) | ||
|
||
require_relative "./spec_helper" | ||
require_relative "../shared/tax_calculator_sorbet" | ||
require_relative "tax_calculator_sorbet_spec" | ||
|
||
describe AccountantSorbet do | ||
before do | ||
allow(tax_calculator).to receive(:for_income).and_return(42) | ||
allow(tax_calculator).to receive(:tax_rate_for).and_return(10.0) | ||
allow(tax_calculator).to receive(:for_income).with(-10).and_return(TaxCalculator::Result.new(0)) | ||
end | ||
|
||
let(:tax_calculator) { double("TaxCalculatorSorbet") } | ||
|
||
include_examples "accountant", AccountantSorbet do | ||
it "incorrect" do | ||
# NOTE: in fact, sorbet-runtine also checks for type errors for ALL types | ||
expect { subject.net_pay("incorrect") }.to raise_error(TypeError) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, we can somehow distinguish stdlib types from custom types, so we can ignore only missing stdlib types? Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, it should be easy to implement.
I should also mention that I missed an important part of the problem here and I should update the readme to include this information.
If a signature is described in a
.rb
file, it will be used bysorbet-runtime
and type checking will be available. One of the gems that is using sorbet signatures is ShopifyAPI for example.However, many signatures are declared inside
.rbi
files, like 1) signatures for stdlib and core types and 2) signatures for most libraries including rails. Unfortunately, these types cannot be loaded into runtime at the moment. Therefore, it's not possible to type check their mocks yet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also updated readme to make the explanation more understandable.
link to the comment
I can add this commit to the
master
branch too.