Remove clearDirectives and use first-class properties for @specifiedBy in SchemaPrinter

andimarek · claude · andimarek · commit 31ee7c6c3b80 · 2026-01-28T12:43:44.000+10:00
Remove GraphQLSchema.Builder.clearDirectives() as it allows removing spec-required built-in directives, which arguably violates the spec. All 7 built-in directives are now always present. Change SchemaPrinter to synthesize @specifiedBy from the first-class specifiedByUrl property on GraphQLScalarType, matching how @deprecated already uses the first-class deprecationReason property. This ensures both directives rely on their first-class properties as the source of truth rather than applied directives. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java
@@ -692,7 +692,6 @@ public static Builder newSchema(GraphQLSchema existingSchema) {
                 .introspectionSchemaType(existingSchema.getIntrospectionSchemaType())
                 .codeRegistry(existingSchema.getCodeRegistry())
                 .clearAdditionalTypes()
-                .clearDirectives()
                 .additionalDirectives(new LinkedHashSet<>(existingSchema.getDirectives()))
                 .clearSchemaDirectives()
                 .withSchemaDirectives(schemaDirectivesArray(existingSchema))
@@ -862,12 +861,6 @@ public Builder additionalDirective(GraphQLDirective additionalDirective) {
             return this;
         }
 
-        public Builder clearDirectives() {
-            this.additionalDirectives.clear();
-            return this;
-        }
-
-
         public Builder withSchemaDirectives(GraphQLDirective... directives) {
             for (GraphQLDirective directive : directives) {
                 withSchemaDirective(directive);
diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java
@@ -64,6 +64,7 @@
 import java.util.stream.Stream;
 
 import static graphql.Directives.DeprecatedDirective;
+import static graphql.Directives.SpecifiedByDirective;
 import static graphql.Scalars.GraphQLString;
 import static graphql.schema.visibility.DefaultGraphqlFieldVisibility.DEFAULT_FIELD_VISIBILITY;
 import static graphql.util.EscapeUtil.escapeJsonString;
@@ -559,7 +560,8 @@ private SchemaElementPrinter<GraphQLScalarType> scalarPrinter() {
                     printAsAst(out, type.getDefinition(), type.getExtensionDefinitions());
                 } else {
                     printComments(out, type, "");
-                    out.format("scalar %s%s\n\n", type.getName(), directivesString(GraphQLScalarType.class, type));
+                    List<GraphQLAppliedDirective> directives = addOrUpdateSpecifiedByDirectiveIfNeeded(type);
+                    out.format("scalar %s%s\n\n", type.getName(), directivesString(GraphQLScalarType.class, directives));
                 }
             }
         };
@@ -1103,6 +1105,65 @@ private String getDeprecationReason(GraphQLDirectiveContainer directiveContainer
         }
     }
 
+    private boolean isSpecifiedByDirectiveAllowed() {
+        return options.getIncludeDirective().test(SpecifiedByDirective.getName());
+    }
+
+    private boolean isSpecifiedByDirective(GraphQLAppliedDirective directive) {
+        return directive.getName().equals(SpecifiedByDirective.getName());
+    }
+
+    private boolean hasSpecifiedByDirective(List<GraphQLAppliedDirective> directives) {
+        return directives.stream().anyMatch(this::isSpecifiedByDirective);
+    }
+
+    private List<GraphQLAppliedDirective> addOrUpdateSpecifiedByDirectiveIfNeeded(GraphQLScalarType scalarType) {
+        List<GraphQLAppliedDirective> directives = DirectivesUtil.toAppliedDirectives(scalarType);
+        String url = scalarType.getSpecifiedByUrl();
+
+        if (url == null) {
+            // first-class property is not set - remove any @specifiedBy applied directive
+            return directives.stream()
+                    .filter(d -> !isSpecifiedByDirective(d))
+                    .collect(toList());
+        }
+        if (!hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) {
+            directives = new ArrayList<>(directives);
+            directives.add(createSpecifiedByDirective(url));
+        } else if (hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) {
+            // Update URL in case modified by schema transform
+            directives = updateSpecifiedByDirective(directives, url);
+        }
+        return directives;
+    }
+
+    private GraphQLAppliedDirective createSpecifiedByDirective(String url) {
+        GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument()
+                .name("url")
+                .valueProgrammatic(url)
+                .type(GraphQLString)
+                .build();
+        return GraphQLAppliedDirective.newDirective()
+                .name("specifiedBy")
+                .argument(arg)
+                .build();
+    }
+
+    private List<GraphQLAppliedDirective> updateSpecifiedByDirective(List<GraphQLAppliedDirective> directives, String url) {
+        GraphQLAppliedDirectiveArgument newArg = GraphQLAppliedDirectiveArgument.newArgument()
+                .name("url")
+                .valueProgrammatic(url)
+                .type(GraphQLString)
+                .build();
+
+        return directives.stream().map(d -> {
+            if (isSpecifiedByDirective(d)) {
+                return d.transform(builder -> builder.argument(newArg));
+            }
+            return d;
+        }).collect(toList());
+    }
+
     private String directiveDefinition(GraphQLDirective directive) {
         StringBuilder sb = new StringBuilder();
 
diff --git a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy
@@ -148,28 +148,25 @@ class GraphQLSchemaTest extends Specification {
         ((Directive) newSchema.extensionDefinitions.first().getDirectives().first()).name == "pizza"
     }
 
-    def "clear directives works as expected"() {
+    def "all built-in directives are always present"() {
         setup:
         def schemaBuilder = basicSchemaBuilder()
 
-        when: "no additional directives have been specified"
+        when: "a schema is built"
         def schema = schemaBuilder.build()
-        then:
+        then: "all 7 built-in directives are present"
         schema.directives.size() == 7
-
-        when: "clear directives is called"
-        schema = schemaBuilder.clearDirectives().build()
-        then:
-        schema.directives.size() == 5 // @deprecated and @specifiedBy and @oneOf et al is ALWAYS added if missing
-
-        when: "clear directives is called with more directives"
-        schema = schemaBuilder.clearDirectives().additionalDirective(Directives.SkipDirective).build()
-        then:
-        schema.directives.size() == 6
+        schema.getDirective("include") != null
+        schema.getDirective("skip") != null
+        schema.getDirective("deprecated") != null
+        schema.getDirective("specifiedBy") != null
+        schema.getDirective("oneOf") != null
+        schema.getDirective("defer") != null
+        schema.getDirective("experimental_disableErrorPropagation") != null
 
         when: "the schema is transformed, things are copied"
-        schema = schema.transform({ builder -> builder.additionalDirective(Directives.IncludeDirective) })
-        then:
+        schema = schema.transform({ builder -> builder })
+        then: "all 7 built-in directives are still present"
         schema.directives.size() == 7
     }
 
