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

Allow @deprecated directive on field args #243

Closed

Conversation

jonbretman
Copy link

Fixes #204

@jonbretman
Copy link
Author

@tonyghita any thoughts?

@tonyghita
Copy link
Member

Is this part of the specification? I'd like to avoid straying too far from the spec.

@jonbretman
Copy link
Author

I believe so. It’s mentioned in the spec under InputValueDefinition - http://facebook.github.io/graphql/draft/#InputValueDefinition

@tonyghita
Copy link
Member

The specification for InputValueDefinition mentions directives, but the introspection specification leaves out deprecations on __InputValue: http://facebook.github.io/graphql/draft/#sec-Schema-Introspection.

I think without special isDeprecated fields on introspection, this change would not be very useful.
Here's an issue on the GraphQL specification specifically for this functionality: graphql/graphql-spec#197

I'm going to close this change until the issue in the specification is resolved.

@tonyghita tonyghita closed this Aug 22, 2018
@jonbretman
Copy link
Author

Fair enough, thanks for looking into it.

@smitt04
Copy link

smitt04 commented Oct 25, 2018

This is working its way into the spec graphql/graphql-spec#525

@tonyghita
Copy link
Member

We're open to adding it once it is accepted into the specification 👍

@sqs
Copy link
Contributor

sqs commented Dec 18, 2022

It is now added into the specification: graphql/graphql-spec#805 (comment)

@sqs
Copy link
Contributor

sqs commented Dec 18, 2022

And here is the diff that seems to work. Happy to submit another PR.

diff --git a/example/social/introspect.json b/example/social/introspect.json
index 795803f..e474d79 100644
--- a/example/social/introspect.json
+++ b/example/social/introspect.json
@@ -17,7 +17,8 @@
         "description": "Marks an element of a GraphQL schema as no longer supported.",
         "locations": [
           "FIELD_DEFINITION",
-          "ENUM_VALUE"
+          "ENUM_VALUE",
+          "ARGUMENT_DEFINITION"
         ],
         "name": "deprecated"
       },
diff --git a/example/starwars/introspect.json b/example/starwars/introspect.json
index a25eace..009799f 100644
--- a/example/starwars/introspect.json
+++ b/example/starwars/introspect.json
@@ -17,7 +17,8 @@
         "description": "Marks an element of a GraphQL schema as no longer supported.",
         "locations": [
           "FIELD_DEFINITION",
-          "ENUM_VALUE"
+          "ENUM_VALUE",
+          "ARGUMENT_DEFINITION"
         ],
         "name": "deprecated"
       },
diff --git a/graphql_test.go b/graphql_test.go
index 76240d5..cb240d6 100644
--- a/graphql_test.go
+++ b/graphql_test.go
@@ -1180,8 +1180,8 @@ func TestFragments(t *testing.T) {
                        `,
                },
                {
-                       Schema:         starwarsSchema,
-                       Query:          `
+                       Schema: starwarsSchema,
+                       Query: `
                                query {
                                        human(id: "1000") {
                                                id
@@ -1733,6 +1733,24 @@ func TestEnums(t *testing.T) {
        })
 }
 
+type testDeprecatedArgsResolver struct{}
+
+func (r *testDeprecatedArgsResolver) A(ctx context.Context, args struct{ B *string }) int32 {
+       return 0
+}
+
+func TestDeprecatedArgs(t *testing.T) {
+       graphql.MustParseSchema(`
+               schema {
+                       query: Query
+               }
+
+               type Query {
+                       a(b: String @deprecated): Int!
+               }
+       `, &testDeprecatedArgsResolver{})
+}
+
 func TestInlineFragments(t *testing.T) {
        gqltesting.RunTests(t, []*gqltesting.Test{
                {
@@ -2506,7 +2524,8 @@ func TestIntrospection(t *testing.T) {
                                                                        "description": "Marks an element of a GraphQL schema as no longer supported.",
                                                                        "locations": [
                                                                                "FIELD_DEFINITION",
-                                                                               "ENUM_VALUE"
+                                                                               "ENUM_VALUE",
+                                                                               "ARGUMENT_DEFINITION"
                                                                        ],
                                                                        "args": [
                                                                                {
diff --git a/internal/common/values.go b/internal/common/values.go
index 2d6e0b5..33f63d2 100644
--- a/internal/common/values.go
+++ b/internal/common/values.go
@@ -27,9 +27,11 @@ func ParseArgumentList(l *Lexer) types.ArgumentList {
                name := l.ConsumeIdentWithLoc()
                l.ConsumeToken(':')
                value := ParseLiteral(l, false)
+               directives := ParseDirectives(l)
                args = append(args, &types.Argument{
-                       Name:  name,
-                       Value: value,
+                       Name:       name,
+                       Value:      value,
+                       Directives: directives,
                })
        }
        l.ConsumeToken(')')
diff --git a/internal/schema/meta.go b/internal/schema/meta.go
index 154c07c..7941a63 100644
--- a/internal/schema/meta.go
+++ b/internal/schema/meta.go
@@ -57,7 +57,7 @@ var metaSrc = `
                # for how to access supported similar data. Formatted in
                # [Markdown](https://daringfireball.net/projects/markdown/).
                reason: String = "No longer supported"
-       ) on FIELD_DEFINITION | ENUM_VALUE
+       ) on FIELD_DEFINITION | ENUM_VALUE | ARGUMENT_DEFINITION
 
        # Provides a scalar specification URL for specifying the behavior of custom scalar types.
        directive @specifiedBy(
diff --git a/types/argument.go b/types/argument.go
index b2681a2..72329da 100644
--- a/types/argument.go
+++ b/types/argument.go
@@ -4,8 +4,9 @@ package types
 //
 // https://spec.graphql.org/draft/#sec-Language.Arguments
 type Argument struct {
-       Name  Ident
-       Value Value
+       Name       Ident
+       Value      Value
+       Directives DirectiveList
 }
 
 // ArgumentList is a collection of GraphQL Arguments.

sqs pushed a commit to sourcegraph/graphql-go that referenced this pull request Dec 18, 2022
@pavelnikolov
Copy link
Member

@sqs thank you for commenting. Could you, please, open a new PR? I would be happy to review and merge it.

@pavelnikolov
Copy link
Member

@sqs I took the changes and raised a PR myself.

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.

@deprecated directive on input
5 participants