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.

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

map anonymous GraphQL InputObjects to T.untyped #1146

Closed
wants to merge 1 commit into from

Conversation

ryanquanz
Copy link

Motivation

When run tapioca dsl against a GraphQLType that used an anonymous class inheriting from InputObject, I was seeing errors like these:

(when sorbet assertions are being enforced)

TypeError: Passed `nil` into T.must
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/_types.rb:218:in `must'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/dsl/helpers/graphql_type_helper.rb:47:in `type_for'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation_2_7.rb:106:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation_2_7.rb:106:in `block in create_validator_method_fast1'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:55:in `block (2 levels) in decorate'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:53:in `each'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:53:in `block in decorate'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:40:in `block in create_class'
            <internal:kernel>:90:in `tap'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:39:in `create_class'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `validate_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/_methods.rb:270:in `block in _on_method_added'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:16:in `create_path'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `validate_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/_methods.rb:270:in `block in _on_method_added'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:52:in `decorate'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `validate_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/_methods.rb:270:in `block in _on_method_added'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/helpers/test/dsl_compiler.rb:96:in `rbi_for'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `validate_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/_methods.rb:270:in `block in _on_method_added'
            /home/spin/src/github.com/Shopify/tapioca/lib/tapioca/helpers/test/dsl_compiler.rb:33:in `rbi_for'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `bind_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/call_validation.rb:157:in `validate_call'
            /home/spin/.bundle/tapioca/gems/sorbet-runtime-0.5.10326/lib/types/private/methods/_methods.rb:270:in `block in _on_method_added'
            /home/spin/src/github.com/Shopify/tapioca/spec/tapioca/dsl/compilers/graphql_input_object_spec.rb:80:in `block (3 levels) in <class:GraphqlInputObjectSpec>'

(when sorbet assertions are not being enforced)

tapioca/lib/tapioca/helpers/rbi_helper.rb:100:in `as_nilable_type': NoMethodError: undefined method `start_with?' for nil:NilClass (Parallel::UndumpableException)

      if type.start_with?("T.nilable(", "::T.nilable(") || type == "T.untyped" || type == "::T.untyped"
             ^^^^^^^^^^^^
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/helpers/graphql_type_helper.rb:54:in `type_for'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:55:in `block (2 levels) in decorate'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:53:in `each'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:53:in `block in decorate'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:40:in `block in create_class'
	from <internal:kernel>:90:in `tap'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:39:in `create_class'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/rbi_ext/model.rb:16:in `create_path'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/compilers/graphql_input_object.rb:52:in `decorate'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/pipeline.rb:160:in `block in rbi_for_constant'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/pipeline.rb:156:in `each'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/pipeline.rb:156:in `rbi_for_constant'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/pipeline.rb:68:in `block in run'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:587:in `call_with_index'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:557:in `process_incoming_jobs'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:537:in `block in worker'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:528:in `fork'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:528:in `worker'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:519:in `block in create_workers'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:518:in `each'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:518:in `each_with_index'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:518:in `create_workers'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:457:in `work_in_processes'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/parallel-1.22.1/lib/parallel.rb:294:in `map'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/executor.rb:31:in `run_in_parallel'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/dsl/pipeline.rb:67:in `run'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/commands/dsl.rb:104:in `execute'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/cli.rb:144:in `block in dsl'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca.rb:33:in `block in silence_warnings'
	from /opt/rubies/3.1.2/lib/ruby/3.1.0/rubygems/user_interaction.rb:46:in `use_ui'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca.rb:32:in `silence_warnings'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `bind_call'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/sorbet-runtime-0.5.10346/lib/types/private/methods/_methods.rb:272:in `block in _on_method_added'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/lib/tapioca/cli.rb:140:in `dsl'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/thor-1.2.1/lib/thor/command.rb:27:in `run'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/thor-1.2.1/lib/thor/invocation.rb:127:in `invoke_command'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/thor-1.2.1/lib/thor.rb:392:in `dispatch'
	from /Users/ryanquanz/.gem/ruby/3.1.2/gems/thor-1.2.1/lib/thor/base.rb:485:in `start'
	from /Users/ryanquanz/src/github.com/Shopify/tapioca/exe/tapioca:23:in `<top (required)>'
	from bin/tapioca:29:in `load'
	from bin/tapioca:29:in `<main>'

The problem appears to be that Tapioca::Dsl::Helpers::GraphqlTypeHelper#type_for when receiving a descendant of GraphQL::Schema::InputObject assumes it will have a name, but with anonymous classes that is not the case:

parsed_type = case unwrapped_type
when GraphQL::Types::Boolean.singleton_class
"T::Boolean"
when GraphQL::Types::Float.singleton_class
qualified_name_of(Float)
when GraphQL::Types::ID.singleton_class
qualified_name_of(String)
when GraphQL::Types::Int.singleton_class
qualified_name_of(Integer)
when GraphQL::Types::ISO8601Date.singleton_class
qualified_name_of(Date)
when GraphQL::Types::ISO8601DateTime.singleton_class
qualified_name_of(DateTime)
when GraphQL::Types::JSON.singleton_class
"T::Hash[::String, T.untyped]"
when GraphQL::Types::String.singleton_class
qualified_name_of(String)
when GraphQL::Schema::Enum.singleton_class
enum_values = T.cast(unwrapped_type.enum_values, T::Array[GraphQL::Schema::EnumValue])
value_types = enum_values.map { |v| qualified_name_of(v.value.class) }.uniq
if value_types.size == 1
value_types.first
else
"T.any(#{value_types.join(", ")})"
end
when GraphQL::Schema::InputObject.singleton_class
qualified_name_of(unwrapped_type)
else
"T.untyped"
end
parsed_type = T.must(parsed_type)

Implementation

I replace the T.must with a guard to return T.untyped, I'm not sure if we could make a more precise type determination?

Tests

I have augmented a test - hopefully it suffices as replication instructions.

GraphQLTypeHelper assumes that all classes have names. Anonymous
classes don't have names and cause a T.must assertion to blow
up. This replaces the T.must, instead returning T.untyped if
an anonymous InputObject is encountered.
@ryanquanz ryanquanz requested a review from a team as a code owner September 2, 2022 16:19
end

parsed_type = T.must(parsed_type)
return "T.untyped" if parsed_type.nil?
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: can we change this to:

Suggested change
return "T.untyped" if parsed_type.nil?
parsed_type = "T.untyped" unless parsed_type

This makes sure that the rest of the code has a chance to run, in case any logic we add want to make use of the type at this point.

For example, the T::Array case 2 lines below might need to use the type here to get T::Array[T.untyped]

@@ -57,6 +57,7 @@ class CreateCommentInput < GraphQL::Schema::InputObject
class CreateCommentInput < GraphQL::Schema::InputObject
argument :body, String, required: true
argument :post_id, ID, required: true
argument :signature, Class.new(GraphQL::Schema::InputObject), required: true
Copy link
Member

Choose a reason for hiding this comment

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

Based on my comment above, can you also add an argument with [Class.new(GraphQL::Schema::InputObject)] type?

@rafaelfranca
Copy link
Member

Sorry @ryanquanz I end up fixing this before looking on this PR in ad74c72.

Really sorry about that.

@rafaelfranca rafaelfranca deleted the rq/handle-anonymous-types-in-graphql-input branch March 29, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants