Merge pull request #849 from graphql-java-kickstart/build/stricter-verify

Fail the build on compiler warnings and check the release bundle in CI

Author: oryan-block

.github/workflows/pull-request.yml

@@ -24,3 +24,23 @@ jobs:
           restore-keys: ${{ runner.os }}-m2
       - name: Build with Maven
         run: mvn --batch-mode --update-snapshots verify
+
+  # The same jar has to work with the oldest supported graphql-java, so build and test against it too
+  graphql-java-25:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v7
+      - name: Setup java
+        uses: actions/setup-java@v6
+        with:
+          java-version: '11'
+          distribution: 'temurin'
+      - name: Cache Maven packages
+        uses: actions/cache@v6
+        with:
+          path: ~/.m2
+          key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
+          restore-keys: ${{ runner.os }}-m2
+      - name: Build with Maven
+        run: mvn --batch-mode --update-snapshots verify -Dgraphql-java.version=25.1

pom.xml

@@ -262,6 +262,11 @@
                 <artifactId>kotlin-maven-plugin</artifactId>
                 <groupId>org.jetbrains.kotlin</groupId>
                 <version>${kotlin.version}</version>
+                <configuration>
+                    <args>
+                        <arg>-Werror</arg>
+                    </args>
+                </configuration>
                 <executions>
                     <execution>
                         <id>compile</id>
@@ -286,8 +291,8 @@
                 <artifactId>maven-compiler-plugin</artifactId>
                 <version>3.16.0</version>
                 <configuration>
-                    <source>11</source>
-                    <target>11</target>
+                    <release>11</release>
+                    <failOnWarning>true</failOnWarning>
                 </configuration>
             </plugin>
 

src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt

@@ -234,8 +234,8 @@ internal class SchemaClassScanner(
                 dictionary
                     .filter {
                         it.value.javaType != null
-                            && it.value.typeClass() != java.lang.Object::class.java
-                            && !java.util.Map::class.java.isAssignableFrom(it.value.typeClass())
+                            && it.value.typeClass() != Any::class.java
+                            && !Map::class.java.isAssignableFrom(it.value.typeClass())
                             && it.key !is InputObjectTypeDefinition
                             && it.key !is UnionTypeDefinition
                     }.mapValuesTo(it) { it.value.javaType }
@@ -355,7 +355,7 @@ internal class SchemaClassScanner(
      * Scan a new object for types that haven't been mapped yet.
      */
     private fun scanQueueItemForPotentialMatches(item: QueueItem) {
-        val resolverInfo = if (item.clazz == Object::class.java) {
+        val resolverInfo = if (item.clazz == Any::class.java) {
             getResolverInfoFromTypeDictionary(item.type.name)
                 ?: throw SchemaClassScannerError("The GraphQL schema type '${item.type.name}' maps to a field of type java.lang.Object however there is no matching entry for this type in the type dictionary. You may need to add this type to the dictionary before building the schema.")
         } else {
@@ -391,7 +391,7 @@ internal class SchemaClassScanner(
     }
 
     private fun isResolverForSupertype(resolverInfo: NormalResolverInfo, dataClass: Class<*>) =
-        resolverInfo.dataClassType != Object::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
+        resolverInfo.dataClassType != Any::class.java && resolverInfo.dataClassType.isAssignableFrom(dataClass)
 
     private fun scanResolverInfoForPotentialMatches(type: ObjectTypeDefinition, resolverInfo: ResolverInfo) {
         type.getExtendedFieldDefinitions(extensionDefinitions).forEach { field ->
@@ -428,7 +428,7 @@ internal class SchemaClassScanner(
                 if (options.preferGraphQLResolver && realEntry.hasResolverRef()) {
                     log.warn("The real entry ${realEntry.joinReferences()} is a GraphQLResolver so ignoring this one ${javaType.unwrap()} $reference")
                 } else {
-                    if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) {
+                    if (Map::class.java.isAssignableFrom(javaType.unwrap())) {
                         throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n|   ${reference.getDescription()}")
                     }
                     throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n|   ${reference.getDescription()}")

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -12,7 +12,7 @@ import graphql.language.*
 import graphql.schema.*
 import graphql.schema.idl.RuntimeWiring
 import graphql.schema.idl.ScalarInfo
-import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility
+import graphql.schema.visibility.BlockedFields
 import org.slf4j.LoggerFactory
 import kotlin.reflect.KClass
 
@@ -69,7 +69,10 @@ class SchemaParser internal constructor(
      */
     fun parseSchemaObjects(): SchemaObjects {
         if (!options.introspectionEnabled) {
-            codeRegistryBuilder.fieldVisibility(NoIntrospectionGraphqlFieldVisibility.NO_INTROSPECTION_FIELD_VISIBILITY)
+            // What the deprecated NoIntrospectionGraphqlFieldVisibility does. Its suggested replacement,
+            // Introspection.enabledJvmWide, would also turn introspection off for every other schema in the JVM.
+            // TODO deprecate introspectionEnabled and remove this in 15.0.0 (#858)
+            codeRegistryBuilder.fieldVisibility(BlockedFields.newBlock().addPattern("__.*").build())
         }
         // this overrides the above introspection enabled setting obviously... todo: add documentation
         options.fieldVisibility?.let { codeRegistryBuilder.fieldVisibility(it) }
@@ -433,6 +436,7 @@ class SchemaParser internal constructor(
     }
 
     // TODO remove this once directives are fully replaced with applied directives
+    @Suppress("DEPRECATION") // legacy directive argument values
     private fun buildDirectives(
         directives: List<Directive>,
         directiveLocation: Introspection.DirectiveLocation
@@ -535,7 +539,7 @@ class SchemaParser internal constructor(
             is NonNullType -> GraphQLNonNull(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects))
             is InputObjectTypeDefinition -> {
                 log.info("Create input object")
-                createInputObject(typeDefinition, inputObjects, referencingInputObjects as MutableSet<String>)
+                createInputObject(typeDefinition, inputObjects, referencingInputObjects)
             }
             is TypeName -> {
                 val scalarType = customScalars[typeDefinition.name]

src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt

@@ -10,7 +10,6 @@ import graphql.schema.DataFetcher
 import graphql.schema.DataFetchingEnvironment
 import graphql.schema.visibility.GraphqlFieldVisibility
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.ReceiveChannel
 import kotlinx.coroutines.reactive.publish
 import org.reactivestreams.Publisher
@@ -45,7 +44,6 @@ data class SchemaParserOptions internal constructor(
         fun newOptions() = Builder()
 
         @JvmStatic
-        @ExperimentalCoroutinesApi
         fun defaultOptions() = Builder().build()
     }
 
@@ -164,7 +162,6 @@ data class SchemaParserOptions internal constructor(
             this.allowUndeclaredDirectives = allowUndeclaredDirectives
         }
 
-        @ExperimentalCoroutinesApi
         fun build(): SchemaParserOptions {
             val coroutineContextProvider = coroutineContextProvider
                 ?: DefaultCoroutineContextProvider(Dispatchers.Default)

src/main/kotlin/graphql/kickstart/tools/TypeClassMatcher.kt

@@ -68,7 +68,7 @@ internal class TypeClassMatcher(private val definitionsByName: Map<String, TypeD
                 realType is Class<*> && realType.isArray ->
                     match(potentialMatch, graphQLType.type, realType.componentType)
                 // e.g. a property map value: the element type is unknown, so match it as Object too
-                realType == Object::class.java ->
+                realType == Any::class.java ->
                     match(potentialMatch, graphQLType.type, realType)
                 else ->
                     throw error(potentialMatch, "Java class is not a List or generic type information was lost: $realType")

src/main/kotlin/graphql/kickstart/tools/resolver/FieldResolverScanner.kt

@@ -56,7 +56,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
             }
         }
 
-        if (java.util.Map::class.java.isAssignableFrom(search.type.unwrap())) {
+        if (Map::class.java.isAssignableFrom(search.type.unwrap())) {
             return MapFieldResolver(field, search, options, search.type.unwrap())
         }
 
@@ -128,7 +128,7 @@ internal class FieldResolverScanner(val options: SchemaParserOptions) {
             .filter { !Modifier.isPrivate(it.modifiers) }
             // discard any methods that are coming off the root of the class hierarchy
             // to avoid issues with duplicate method declarations
-            .filter { it.declaringClass != Object::class.java }
+            .filter { it.declaringClass != Any::class.java }
             // subscription resolvers must return a publisher
             .filter { !search.isSubscription || resolverMethodReturnsPublisher(it) }
             .toList()

src/main/kotlin/graphql/kickstart/tools/resolver/MapFieldResolver.kt

@@ -36,7 +36,7 @@ internal class MapFieldResolver(
         val resolvedType = TypeResolver().resolve(mapClass)
         val typeParameters = resolvedType.typeParametersFor(Map::class.java)
 
-        return typeParameters.elementAtOrElse(1) { Object::class.java }
+        return typeParameters.elementAtOrElse(1) { Any::class.java }
     }
 
     override fun createDataFetcher(): DataFetcher<*> {

src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt

@@ -157,7 +157,7 @@ internal class MethodFieldResolver(
             is ListType -> List::class.java.isAssignableFrom(this.typeResolver.getRawClass(genericParameterType))
                 && isConcreteScalarType(environment, type.type, this.typeResolver.resolve(genericParameterType))
 
-            is TypeName -> environment.graphQLSchema?.getType(type.name!!)?.let { isScalar(it) && type.name != "ID" }
+            is TypeName -> environment.graphQLSchema.getType(type.name!!)?.let { isScalar(it) && type.name != "ID" }
                 ?: false
 
             is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType)

src/main/kotlin/module-info.java

@@ -5,6 +5,8 @@
     requires kotlinx.coroutines.reactive;
 
     requires com.graphqljava;
+    // graphql-java's nullability annotations, only needed at compile time
+    requires static org.jspecify;
 
     requires com.fasterxml.jackson.core;
     requires com.fasterxml.jackson.kotlin;