schema printer adds unnecessary comma for arguments with description

[dariuszkuc]

Description

Schema printer adds unnecessary comma for arguments with description. See repro below.

Steps to reproduce

#[test]
    fn arguments() {
        let mut foo = ObjectType {
            name: Name::new("Foo"),
            description: None,
            directives: vec![],
            fields: IndexMap::new(),
            implements_interfaces: IndexMap::new(),
        };
        foo.fields.insert(
            Name::new("bar"),
            Component::new(FieldDefinition {
                name: Name::new("bar"),
                description: None,
                directives: vec![],
                arguments: vec![Node::new(InputValueDefinition {
                    name: Name::new("arg"),
                    description: Some(NodeStr::new("foobar")),
                    // description: None,
                    directives: vec![],
                    ty: Type::Named(NodeStr::new("String")),
                    default_value: None,
                })],
                ty: Type::Named(NodeStr::new("String")),
            }),
        );

        let mut schema = Schema::new();
        schema
            .types
            .insert(Name::new("Foo"), ExtendedType::Object(Node::new(foo)));
        println!("{}", schema);
    }

Expected result

No comma after single argument with description

type Foo {
  bar(
    "foobar"
    arg: String
  ): String
}

Actual result

Extra comma after single argument

type Foo {
  bar(
    "foobar"
    arg: String,
  ): String
}

Environment

• Operating system and version: MacOS Ventura
• Shell (bash/zsh/powershell): zsh
• apollo-rs crate: mut branch
• Crate version: 1.72.0

[goto-bus-stop]

All multi-line lists get a trailing comma, and description forces an argument decl list to be multi-line. So this is definitely intentional, though perhaps it could be configurable in the future.

[SimonSapin]

I confirm this was intentional, but it’s a stylistic choice that we can change. I’m not sure it’s worth configuration. It could be a heuristic based on the presence of descriptions (on the argument before and/or after the potential comma)

[lrlna]

Do we do what graphql-js does? I think with printing we should just stick to their format

[SimonSapin]

Looking for ', ' in https://github.com/graphql/graphql-js/blob/main/src/language/printer.ts seems like the best we have, to find out where graphql-js uses commas.

In at least one place it chooses either commas OR newlines:

      let argsLine = prefix + wrap('(', join(args, ', '), ')');

      if (argsLine.length > MAX_LINE_LENGTH) {
        argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)');
      }