Merge pull request #4429 from rmosolgo/fix-selections-on-scalars

Fix crash when selections and inline fragments are made on scalars

Author: rmosolgo

lib/graphql/static_validation/rules/fields_have_appropriate_selections.rb

@@ -26,11 +26,19 @@ def validate_field_selections(ast_node, resolved_type)
         msg = if resolved_type.nil?
           nil
         elsif resolved_type.kind.scalar? && ast_node.selections.any?
-          if ast_node.selections.first.is_a?(GraphQL::Language::Nodes::InlineFragment)
-            "Selections can't be made on scalars (%{node_name} returns #{resolved_type.graphql_name} but has inline fragments [#{ast_node.selections.map(&:type).map(&:name).join(", ")}])"
-          else
-            "Selections can't be made on scalars (%{node_name} returns #{resolved_type.graphql_name} but has selections [#{ast_node.selections.map(&:name).join(", ")}])"
+          selection_strs = ast_node.selections.map do |n|
+            case n
+            when GraphQL::Language::Nodes::InlineFragment
+              "\"... on #{n.type.name} { ... }\""
+            when GraphQL::Language::Nodes::Field
+              "\"#{n.name}\""
+            when GraphQL::Language::Nodes::FragmentSpread
+              "\"#{n.name}\""
+            else
+              raise "Invariant: unexpected selection node: #{n}"
+            end
           end
+          "Selections can't be made on scalars (%{node_name} returns #{resolved_type.graphql_name} but has selections [#{selection_strs.join(", ")}])"
         elsif resolved_type.kind.fields? && ast_node.selections.empty?
           "Field must have selections (%{node_name} returns #{resolved_type.graphql_name} but has no selections. Did you mean '#{ast_node.name} { ... }'?)"
         else

spec/graphql/static_validation/rules/fields_have_appropriate_selections_spec.rb

@@ -17,7 +17,7 @@
     assert_equal(4, errors.length)
 
     illegal_selection_error = {
-      "message"=>"Selections can't be made on scalars (field 'id' returns Int but has selections [something, someFields])",
+      "message"=>"Selections can't be made on scalars (field 'id' returns Int but has selections [\"something\", \"someFields\"])",
       "locations"=>[{"line"=>6, "column"=>47}],
       "path"=>["query getCheese", "illegalSelectionCheese", "id"],
       "extensions"=>{"code"=>"selectionMismatch", "nodeName"=>"field 'id'", "typeName"=>"Int"}
@@ -41,7 +41,7 @@
     assert_includes(errors, interfaces_selection_required_error, "finds interfaces without selections")
 
     incorrect_fragment_error = {
-      "message"=>"Selections can't be made on scalars (field 'flavor' returns String but has inline fragments [String])",
+      "message"=>"Selections can't be made on scalars (field 'flavor' returns String but has selections [\"... on String { ... }\"])",
       "locations"=>[{"line"=>7, "column"=>48}],
       "path"=>["query getCheese", "incorrectFragmentSpread", "flavor"],
       "extensions"=>{"code"=>"selectionMismatch", "nodeName"=>"field 'flavor'", "typeName"=>"String"}
@@ -63,4 +63,28 @@
       assert_includes(errors, selections_required_error)
     end
   end
+
+  describe "selections and inline fragments on scalars" do
+    let(:query_string) {"
+    {
+      cheese(id: 1) {
+        fatContent {
+          name
+          ... on User {
+            id
+          }
+          ... F
+        }
+      }
+    }
+
+    fragment F on Cheese {
+      id
+    }
+    "}
+    it "returns an error" do
+      expected_err = "Selections can't be made on scalars (field 'fatContent' returns Float but has selections [\"name\", \"... on User { ... }\", \"F\"])"
+      assert_includes(errors.map { |e| e["message"] }, expected_err)
+    end
+  end
 end