Skip to content

Fix formatting schema and schema extensions directives in FormatSchema() and FormatSchemaDocument() #365

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

Jump to bottom
Merged
merged 3 commits into from
Apr 17, 2025
Merged

Fix formatting schema and schema extensions directives in FormatSchema() and FormatSchemaDocument() #365

merged 3 commits into from
Apr 17, 2025

Conversation

lorf
Copy link
Contributor

@lorf lorf commented Apr 17, 2025 •
edited
Loading

This merge request hopefully fixes two bugs in formatter, described below. Also added tests for these cases.

Input

GraphQL schema document, used as input:

schema @schemaDirective {
	query: Dummy
}
extend schema @schemaExtensionDirective1
extend schema @schemaExtensionDirective2 @schemaExtensionDirective3
extend schema {
	subscription: Dummy
}
directive @schemaDirective on SCHEMA
directive @schemaExtensionDirective1 on SCHEMA
directive @schemaExtensionDirective2 on SCHEMA
directive @schemaExtensionDirective3 on SCHEMA
type Dummy {
	id: ID
}

Bug in formatter.FormatSchema()

The following code

package main

import (
    "bytes"
    "os"

    "github.com/vektah/gqlparser/v2"
    "github.com/vektah/gqlparser/v2/ast"
    "github.com/vektah/gqlparser/v2/formatter"
)

func main() {
    if len(os.Args) < 2 {
        panic("need graphql file as argument")
    }

    input, _ := os.ReadFile(os.Args[1])
    src := &ast.Source{
        Name: os.Args[1],
        Input: string(input),
    }

    schema, err := gqlparser.LoadSchema(src)
    if err != nil {
        panic(err)
    }

    var buf bytes.Buffer
    f := formatter.NewFormatter(&buf)
    f.FormatSchema(schema)

    buf.WriteTo(os.Stdout)
}

produces this output

schema {
	query: Dummy
	subscription: Dummy
}
[... insignificant part omitted]

omitting all schema and schema extension directives.

Bug in formatter.FormatSchemaDocument()

Following code

package main

import (
    "bytes"
    "os"

    "github.com/vektah/gqlparser/v2/ast"
    "github.com/vektah/gqlparser/v2/formatter"
    "github.com/vektah/gqlparser/v2/parser"
)

func main() {
    if len(os.Args) < 2 {
        panic("need graphql file as argument")
    }

    input, _ := os.ReadFile(os.Args[1])
    src := &ast.Source{
        Name: os.Args[1],
        Input: string(input),
    }

    sd, err := parser.ParseSchemas(src)
    if err != nil {
        panic(err)
    }

    var buf bytes.Buffer
    f := formatter.NewFormatter(&buf)
    f.FormatSchemaDocument(sd)

    buf.WriteTo(os.Stdout)
}

produces this output:

schema {
	@schemaDirective query: Dummy
}
extend schema {
	@schemaExtensionDirective1 @schemaExtensionDirective2 @schemaExtensionDirective3 subscription: Dummy
}
[... insignificant part omitted]

Here schema directives appear inside schema definition block, which contradicts the spec. Directives should appear before definition block, correct output would be:

schema @schemaDirective {
	query: Dummy
}
extend schema @schemaExtensionDirective1 @schemaExtensionDirective2 @schemaExtensionDirective3 {
	subscription: Dummy
}
[... insignificant part omitted]

I have:

  • Added tests covering the bug / feature
  • Updated any relevant documentation

@coveralls
Copy link

Coverage Status

coverage: 87.623% (+0.03%) from 87.597%
when pulling a6c8c8c on lorf:fix/schema-directives
into bcc3a70 on vektah:master.

@StevenACoffman
Copy link
Collaborator

@git-hulk FYI you should be aware of this change for your work.

@StevenACoffman
Copy link
Collaborator

StevenACoffman commented Apr 17, 2025 •
edited
Loading

@lorf Thanks! I'm not sure how this didn't get noticed before, but I really appreciate the fix (and test coverage)!

@StevenACoffman StevenACoffman merged commit f794790 into vektah:master Apr 17, 2025
7 checks passed
@git-hulk
Copy link
Contributor

@git-hulk FYI you should be aware of this change for your work.

@StevenACoffman Great thanks for your review and your great open-source volunteer work. Looking forward to contributing back more to gqlgen and gqlparser.

@lorf lorf deleted the fix/schema-directives branch April 17, 2025 12:47
@lorf
Copy link
Contributor Author

lorf commented Apr 17, 2025

That was fast! Thank You!

@StevenACoffman StevenACoffman mentioned this pull request Apr 17, 2025
Merged
@renovate renovate bot mentioned this pull request Apr 17, 2025
Closed
@lorf lorf mentioned this pull request Apr 17, 2025
Merged
2 tasks
@red-hat-konflux red-hat-konflux bot mentioned this pull request Jun 29, 2025
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lorf @coveralls @StevenACoffman @git-hulk