Rust Apollo signature generation fixes (second try) (#5054)

To try and increase confidence that nothing is wrong with Nick's PR.

Replacement for #5049 which (for unknown reasons) isn't building
cleanly.

Fixes a few issues with the router implementation of the signature
generation. Mostly related to commas being inserted instead of spaces or
vice-versa, but also includes a significant issue where fragments were
not being included in the signature.

Co-authored-by: [bonnici](https://github.com/bonnici)

apollo-router/src/apollo_studio_interop/mod.rs

@@ -32,6 +32,7 @@ pub(crate) struct ComparableUsageReporting {
 }
 
 /// Enum specifying the result of a comparison.
+#[derive(Debug)]
 pub(crate) enum UsageReportingComparisonResult {
     /// The UsageReporting instances are the same
     Equal,
@@ -167,6 +168,7 @@ impl UsageReportingGenerator<'_> {
                             .get(&fragment_node.fragment_name)
                         {
                             e.insert(fragment.clone());
+                            self.extract_signature_fragments(&fragment.selection_set);
                         }
                     }
                 }
@@ -369,7 +371,7 @@ fn format_selection_set(selection_set: &SelectionSet, f: &mut fmt::Formatter) ->
                 && field_str
                     .chars()
                     .last()
-                    .map_or(false, |c| c.is_alphanumeric())
+                    .map_or(false, |c| c.is_alphanumeric() || c == '_')
             {
                 f.write_str(" ")?;
             }
@@ -414,17 +416,25 @@ fn format_field(field: &Node<Field>, f: &mut fmt::Formatter) -> fmt::Result {
 
         // The graphql-js implementation will use newlines and indentation instead of commas if the length of the "arg line" is
         // over 80 characters. This "arg line" includes the alias followed by ": " if the field has an alias (which is never
-        // the case for now), followed by all argument names and values separated by ": ", surrounded with brackets. Our usage
-        // reporting plugin replaces all newlines + indentation with a single space, so we have to replace commas with spaces if
-        // the line length is too long.
+        // the case for any signatures that the JS implementation formatted), followed by the field name, followed by all argument
+        // names and values separated by ": ", surrounded with brackets. Our usage reporting plugin replaces all newlines +
+        // indentation with a single space, so we have to replace commas with spaces if the line length is too long.
         let arg_strings: Vec<String> = sorted_args
             .iter()
             .map(|a| ApolloReportingSignatureFormatter::Argument(a).to_string())
             .collect();
-        // Adjust for incorrect spacing generated by the argument formatter - 2 extra characters for the surrounding brackets, plus
-        // 2 extra characters per argument for the separating space and the space between the argument name and type.
-        let original_line_length =
-            2 + arg_strings.iter().map(|s| s.len()).sum::<usize>() + (arg_strings.len() * 2);
+        // Adjust for incorrect spacing generated by the argument formatter. We end summing up:
+        // * the length of field name
+        // * 2 extra characters for the surrounding brackets
+        // * the length of all formatted arguments
+        // * one extra character per argument since the JS implementation inserts a space between the argument name and value
+        // * two extra character per argument except the last one since the JS implementation inserts a separating comma and space
+        //   between arguments (but not the last one)
+        let original_line_length = field.name.len()
+            + 2
+            + arg_strings.iter().map(|s| s.len()).sum::<usize>()
+            + arg_strings.len()
+            + ((arg_strings.len() - 1) * 2);
         let separator = if original_line_length > 80 { " " } else { "," };
 
         for (index, arg_string) in arg_strings.iter().enumerate() {
@@ -437,7 +447,7 @@ fn format_field(field: &Node<Field>, f: &mut fmt::Formatter) -> fmt::Result {
                     || arg_string
                         .chars()
                         .last()
-                        .map_or(true, |c| c.is_alphanumeric()))
+                        .map_or(true, |c| c.is_alphanumeric() || c == '_'))
             {
                 f.write_str(separator)?;
             }

apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql

@@ -146,6 +146,10 @@ type Mutation
   @join__type(graph: MAIN)
 {
   noInputMutation: EverythingResponse!
+  graph(id: ID!): TestGraphResponse!
+  mutation2(id: ID!): TestGraphResponse2!
+  mutation3(id: ID!): TestGraphResponse3!
+  mutation4(id1: ID!, id2: ID!): ID!
 }
 
 input NestedEnumInputType
@@ -183,6 +187,8 @@ type Query
   defaultArgQuery(stringInput: String! = "default", inputType: AnotherInputType = {anotherInput: "inputDefault"}): BasicResponse!
   inputTypeDefaultQuery(input: InputTypeWithDefault): BasicResponse!
   sortQuery(listInput: [String!]!, stringInput: String!, nullableStringInput: String, INTInput: Int!, floatInput: Float!, boolInput: Boolean!, enumInput: SomeEnum, idInput: ID!): SortResponse!
+  manyArgsQuery(arg1: String, arg2: String, arg3: String, arg4: String, arg5: String, arg6: String, arg7: String): EverythingResponse!
+  underscoreQuery(arg_: String, _arg2: String, _arg3_: String): UnderscoreResponse
 }
 
 enum SomeEnum
@@ -203,6 +209,33 @@ type SortResponse
   CCC: Int
 }
 
+type TestGraphResponse
+  @join__type(graph: MAIN)
+{
+  updateCheckConfiguration(downgradeDefaultValueChange: Boolean, downgradeStaticChecks: Boolean): Int!
+}
+
+type TestGraphResponse2
+  @join__type(graph: MAIN)
+{
+  updateCheckConfiguration(arg1: Boolean, arg2: Boolean): Int!
+}
+
+type TestGraphResponse3
+  @join__type(graph: MAIN)
+{
+  updateCheckConfiguration(arg1: String, arg2: String, arg3: String): Int!
+}
+
+type UnderscoreResponse
+  @join__type(graph: MAIN)
+{
+  _: String
+  _name: String
+  _name_: String
+  name_: String
+}
+
 type Subscription
   @join__type(graph: MAIN)
 {