Skip to content
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.

Sign up for GitHub

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

Rename #sorbet_type → #__tapioca_type #2125

Merged
merged 1 commit into from
Dec 16, 2024
Merged

Rename #sorbet_type → #__tapioca_type #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions lib/tapioca/dsl/compilers/json_api_client_resource.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,17 @@ def type_for(property)
return "T.untyped" if type.nil?

sorbet_type = if type.respond_to?(:sorbet_type)
line, file = type.method(:sorbet_type).source_location
Copy link
Contributor Author

@amomchilov amomchilov Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This assumes that the source_location is available (i.e. this was defined in Ruby, and not in a C extension). Is that safe to assume, or should I guard against it?

Doing so is a bit uglier:

msg = <<~MESSAGE
  WARNING: `#sorbet_type` is deprecated. Please rename your method to `#__tapioca_type`."
MESSAGE

if (source_location = type.method(:sorbet_type).source_location)
  msg.concat("Defined on line #{source_location[0]} of #{source_location[1]}")
end

msg = $stderr.puts

Copy link
Contributor

Choose a reason for hiding this comment

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

Could also be nil if there's some metaprogramming involved, so let's play it safe. You could extract out the deprecation stuff so as to not pollute the conditional.

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!


$stderr.puts <<~MESSAGE
WARNING: `#sorbet_type` is deprecated. Please rename your method to `#__tapioca_type`."

Defined on line #{line} of #{file}
MESSAGE

type.sorbet_type
elsif type.respond_to?(:__tapioca_type)
type.__tapioca_type
elsif type == ::JsonApiClient::Schema::Types::Integer
"::Integer"
elsif type == ::JsonApiClient::Schema::Types::String
Expand Down
71 changes: 69 additions & 2 deletions spec/tapioca/dsl/compilers/json_api_client_resource_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,10 @@ def name=(name); end
assert_equal(expected, rbi_for(:Post))
end

it "honours types that declare sorbet_type" do
it "honours types that respond to #__tapioca_type" do
add_ruby_file("post.rb", <<~RUBY)
class CustomType
def self.sorbet_type
def self.__tapioca_type
"Integer"
end
end
Expand Down Expand Up @@ -368,6 +368,73 @@ def tag_count=(tag_count); end

assert_equal(expected, rbi_for(:Post))
end

it "prints a warning that #sorbet_type is deprecated" do
add_ruby_file("post.rb", <<~RUBY)
class CustomType
def self.sorbet_type
"Integer"
end
end

class Post < JsonApiClient::Resource
property :comment_count, type: :custom_type
property :tag_count, type: :custom_type, default: 0
end
RUBY

expected = <<~RBI
# typed: strong

class Post
include JsonApiClientResourceGeneratedMethods

module JsonApiClientResourceGeneratedMethods
sig { returns(T.nilable(Integer)) }
def comment_count; end

sig { params(comment_count: T.nilable(Integer)).returns(T.nilable(Integer)) }
def comment_count=(comment_count); end

sig { returns(Integer) }
def tag_count; end

sig { params(tag_count: Integer).returns(Integer) }
def tag_count=(tag_count); end
end
end
RBI

assert_output(nil, /WARNING: `#sorbet_type` is deprecated./) do
assert_equal(expected, rbi_for(:Post))
end
end

it "does not crash if the source location #sorbet_type is unknown" do
add_ruby_file("post.rb", <<~RUBY)
class CustomType
def self.sorbet_type
"Integer"
end

def self.method(name)
m = super
# Fake the source location, as if this was defined in a C extension.
def m.source_location
Copy link
Contributor Author

@amomchilov amomchilov Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's surprisingly hard to make a method with no source_location ahha. Native methods from C extensions would have it, but other than that, every way to define a Ruby method ended up having it defined. I tried:

  1. define_singleton_method
  2. singleton_class.define_method
  3. eval
  4. class_eval
  5. Class.new { ... }

and they all had it defined.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IKR? I could have sworn you needed to explicitly pass in the ___FILE__ and __LINE__ else you got no info at all

nil
end
m
end
end

class Post < JsonApiClient::Resource
property :comment_count, type: :custom_type
property :tag_count, type: :custom_type, default: 0
end
RUBY

rbi_for(:Post) # Should not raise
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have assert_nothing_raised, so a comment will suffice.

end
end
end
end
Expand Down
Loading