From 3a8e3c109d5c177bfd432e436ded8098ec8ff447 Mon Sep 17 00:00:00 2001 From: Dan Adajian Date: Wed, 30 Oct 2024 15:44:11 -0500 Subject: [PATCH] fix: ignore directive replacements for consolidated types when definitionType is input/output (#119) * unit test * make test pass * improve test * fix integration tests * fix integration tests * fix integration tests --- package.json | 2 +- src/annotations/build-annotations.ts | 4 +++ .../build-directive-annotations.ts | 25 +++++++++++++++--- src/definitions/enum.ts | 7 ++++- src/definitions/field.ts | 3 +++ src/definitions/input.ts | 2 ++ src/definitions/interface.ts | 1 + src/definitions/object.ts | 3 ++- src/definitions/union.ts | 4 ++- src/visitor.ts | 4 +-- .../codegen.config.ts | 18 +++++++++++++ .../expected.kt | 26 +++++++++++++++++++ .../schema.graphql | 26 +++++++++++++++++++ 13 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/codegen.config.ts create mode 100644 test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/expected.kt create mode 100644 test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/schema.graphql diff --git a/package.json b/package.json index d454aa9..c89e618 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@expediagroup/graphql-kotlin-codegen", - "packageManager": "bun@1.1.17", + "packageManager": "bun@1.1.32", "main": "dist/plugin.cjs", "types": "dist/plugin.d.cts", "files": [ diff --git a/src/annotations/build-annotations.ts b/src/annotations/build-annotations.ts index ff27031..500120e 100644 --- a/src/annotations/build-annotations.ts +++ b/src/annotations/build-annotations.ts @@ -15,6 +15,7 @@ import { indent } from "@graphql-codegen/visitor-plugin-common"; import { EnumValueDefinitionNode, FieldDefinitionNode, + GraphQLSchema, InputValueDefinitionNode, Kind, TypeDefinitionNode, @@ -31,10 +32,12 @@ export type DefinitionNode = | EnumValueDefinitionNode; export function buildAnnotations({ + schema, config, definitionNode, typeMetadata, }: { + schema: GraphQLSchema; config: CodegenConfigWithDefaults; definitionNode: DefinitionNode; typeMetadata?: TypeMetadata; @@ -49,6 +52,7 @@ export function buildAnnotations({ const directiveAnnotations = buildDirectiveAnnotations( definitionNode, config, + schema, ); const unionAnnotation = typeMetadata?.unionAnnotation ? `@${typeMetadata.unionAnnotation}\n` diff --git a/src/annotations/build-directive-annotations.ts b/src/annotations/build-directive-annotations.ts index a96498d..77d3c44 100644 --- a/src/annotations/build-directive-annotations.ts +++ b/src/annotations/build-directive-annotations.ts @@ -14,12 +14,21 @@ limitations under the License. import { CodegenConfigWithDefaults } from "../config/build-config-with-defaults"; import { DefinitionNode } from "./build-annotations"; import { ConstDirectiveNode } from "graphql/language"; -import { Kind } from "graphql"; +import { GraphQLSchema, isInputObjectType, Kind } from "graphql"; +import { shouldConsolidateTypes } from "../utils/should-consolidate-types"; +import { sanitizeName } from "../utils/sanitize-name"; export function buildDirectiveAnnotations( definitionNode: DefinitionNode, config: CodegenConfigWithDefaults, + schema: GraphQLSchema, ) { + const name = sanitizeName(definitionNode.name.value); + const potentialMatchingInputType = schema.getType(`${name}Input`); + const typeWillBeConsolidated = + isInputObjectType(potentialMatchingInputType) && + potentialMatchingInputType.astNode && + shouldConsolidateTypes(potentialMatchingInputType.astNode, schema, config); const directives = definitionNode.directives ?? []; return directives .map((directive) => { @@ -29,9 +38,17 @@ export function buildDirectiveAnnotations( if (federationReplacement) return federationReplacement + "\n"; const directiveReplacementFromConfig = config.directiveReplacements?.find( - ({ directive, definitionType }) => - directive === directiveName && - (!definitionType || definitionType === definitionNode.kind), + ({ directive, definitionType }) => { + if (directive !== directiveName) return false; + if (!definitionType) return true; + if (definitionType !== definitionNode.kind) return false; + if ( + definitionType !== Kind.INPUT_OBJECT_TYPE_DEFINITION && + definitionType !== Kind.OBJECT_TYPE_DEFINITION + ) + return true; + return !typeWillBeConsolidated; + }, ); if (!directiveReplacementFromConfig) return ""; const kotlinAnnotations = buildKotlinAnnotations( diff --git a/src/definitions/enum.ts b/src/definitions/enum.ts index 9bd94d6..a5c85e1 100644 --- a/src/definitions/enum.ts +++ b/src/definitions/enum.ts @@ -17,9 +17,11 @@ import { buildAnnotations } from "../annotations/build-annotations"; import { shouldExcludeTypeDefinition } from "../config/should-exclude-type-definition"; import { CodegenConfigWithDefaults } from "../config/build-config-with-defaults"; import { sanitizeName } from "../utils/sanitize-name"; +import { GraphQLSchema } from "graphql"; export function buildEnumTypeDefinition( node: EnumTypeDefinitionNode, + schema: GraphQLSchema, config: CodegenConfigWithDefaults, ) { if (shouldExcludeTypeDefinition(node, config)) { @@ -29,10 +31,11 @@ export function buildEnumTypeDefinition( const enumName = sanitizeName(node.name.value); const enumValues = node.values?.map((valueNode) => { - return buildEnumValueDefinition(valueNode, config); + return buildEnumValueDefinition(valueNode, schema, config); }) ?? []; const annotations = buildAnnotations({ + schema, config, definitionNode: node, }); @@ -47,10 +50,12 @@ ${indentMultiline(enumValues.join(",\n") + ";", 2)} function buildEnumValueDefinition( node: EnumValueDefinitionNode, + schema: GraphQLSchema, config: CodegenConfigWithDefaults, ) { const annotations = buildAnnotations({ config, + schema, definitionNode: node, }); if (!config.convert) { diff --git a/src/definitions/field.ts b/src/definitions/field.ts index 39715c5..702e1cc 100644 --- a/src/definitions/field.ts +++ b/src/definitions/field.ts @@ -72,6 +72,7 @@ export function buildObjectFieldDefinition({ typeMetadata, ); const annotations = buildAnnotations({ + schema, config, definitionNode: fieldNode, typeMetadata, @@ -112,6 +113,7 @@ export function buildConstructorFieldDefinition({ typeMetadata, ); const annotations = buildAnnotations({ + schema, config, definitionNode: fieldNode, typeMetadata, @@ -156,6 +158,7 @@ export function buildInterfaceFieldDefinition({ typeMetadata, ); const annotations = buildAnnotations({ + schema, config, definitionNode: fieldNode, typeMetadata, diff --git a/src/definitions/input.ts b/src/definitions/input.ts index b491e03..a027892 100644 --- a/src/definitions/input.ts +++ b/src/definitions/input.ts @@ -40,6 +40,7 @@ export function buildInputObjectDefinition( const initial = typeToUse.isNullable ? " = null" : ""; const annotations = buildAnnotations({ + schema, config, definitionNode: field, }); @@ -53,6 +54,7 @@ export function buildInputObjectDefinition( .join(",\n"); const annotations = buildAnnotations({ + schema, config, definitionNode: node, }); diff --git a/src/definitions/interface.ts b/src/definitions/interface.ts index c15bb31..f37547b 100644 --- a/src/definitions/interface.ts +++ b/src/definitions/interface.ts @@ -40,6 +40,7 @@ export function buildInterfaceDefinition( .join("\n"); const annotations = buildAnnotations({ + schema, config, definitionNode: node, }); diff --git a/src/definitions/object.ts b/src/definitions/object.ts index 3671b69..df57881 100644 --- a/src/definitions/object.ts +++ b/src/definitions/object.ts @@ -42,10 +42,10 @@ export function buildObjectTypeDefinition( } const annotations = buildAnnotations({ + schema, config, definitionNode: node, }); - const name = sanitizeName(node.name.value); const dependentInterfaces = getDependentInterfaceNames(node); const dependentUnions = getDependentUnionsForType(schema, node); const interfacesToInherit = @@ -57,6 +57,7 @@ export function buildObjectTypeDefinition( ); const interfaceInheritance = `${interfacesToInherit.length ? ` : ${sanitizedInterfaceNames.join(", ")}` : ""}`; + const name = sanitizeName(node.name.value); const potentialMatchingInputType = schema.getType(`${name}Input`); const typeWillBeConsolidated = isInputObjectType(potentialMatchingInputType) && diff --git a/src/definitions/union.ts b/src/definitions/union.ts index 039cc79..d0bc32a 100644 --- a/src/definitions/union.ts +++ b/src/definitions/union.ts @@ -11,7 +11,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { UnionTypeDefinitionNode } from "graphql"; +import { GraphQLSchema, UnionTypeDefinitionNode } from "graphql"; import { shouldExcludeTypeDefinition } from "../config/should-exclude-type-definition"; import { CodegenConfigWithDefaults } from "../config/build-config-with-defaults"; import { @@ -22,12 +22,14 @@ import { sanitizeName } from "../utils/sanitize-name"; export function buildUnionTypeDefinition( node: UnionTypeDefinitionNode, + schema: GraphQLSchema, config: CodegenConfigWithDefaults, ) { if (shouldExcludeTypeDefinition(node, config)) { return ""; } const annotations = buildAnnotations({ + schema, config, definitionNode: node, }); diff --git a/src/visitor.ts b/src/visitor.ts index f58dc1b..85e85f1 100644 --- a/src/visitor.ts +++ b/src/visitor.ts @@ -40,7 +40,7 @@ export class KotlinVisitor extends BaseVisitor< } EnumTypeDefinition(node: EnumTypeDefinitionNode): string { - return buildEnumTypeDefinition(node, this.config); + return buildEnumTypeDefinition(node, this._schema, this.config); } InterfaceTypeDefinition(node: InterfaceTypeDefinitionNode): string { @@ -56,6 +56,6 @@ export class KotlinVisitor extends BaseVisitor< } UnionTypeDefinition(node: UnionTypeDefinitionNode): string { - return buildUnionTypeDefinition(node, this.config); + return buildUnionTypeDefinition(node, this._schema, this.config); } } diff --git a/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/codegen.config.ts b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/codegen.config.ts new file mode 100644 index 0000000..f255791 --- /dev/null +++ b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/codegen.config.ts @@ -0,0 +1,18 @@ +import { GraphQLKotlinCodegenConfig } from "../../../src/plugin"; +import { Kind } from "graphql/index"; + +export default { + extraImports: ["should_honor_directiveReplacements_config.*"], + directiveReplacements: [ + { + directive: "directive1", + kotlinAnnotations: ["@SomeAnnotation1"], + definitionType: Kind.OBJECT_TYPE_DEFINITION, + }, + { + directive: "directive2", + kotlinAnnotations: ["@SomeAnnotation2"], + }, + ], + classConsolidationEnabled: true, +} satisfies GraphQLKotlinCodegenConfig; diff --git a/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/expected.kt b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/expected.kt new file mode 100644 index 0000000..0b13219 --- /dev/null +++ b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/expected.kt @@ -0,0 +1,26 @@ +package com.kotlin.generated + +import com.expediagroup.graphql.generator.annotations.* +import should_honor_directiveReplacements_config.* + +@SomeAnnotation1 +@SomeAnnotation2 +@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.OBJECT]) +data class TypeThatShouldGetDirectiveReplacement( + val field: String? = null +) + +@SomeAnnotation2 +@GraphQLValidObjectLocations(locations = [GraphQLValidObjectLocations.Locations.INPUT_OBJECT]) +data class InputTypeThatShouldNotGetDirectiveReplacement( + val field: String? = null +) + +data class MyConsolidatedTypeWithDirectives( + val field: String? = null +) + +@SomeAnnotation2 +data class MyConsolidatedTypeWithDirectives2( + val field: String? = null +) diff --git a/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/schema.graphql b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/schema.graphql new file mode 100644 index 0000000..77acb14 --- /dev/null +++ b/test/unit/should_ignore_filtered_directiveReplacements_for_consolidated_types/schema.graphql @@ -0,0 +1,26 @@ +directive @directive1 on INPUT_OBJECT | OBJECT +directive @directive2 on INPUT_OBJECT | OBJECT + +type TypeThatShouldGetDirectiveReplacement @directive1 @directive2 { + field: String +} + +input InputTypeThatShouldNotGetDirectiveReplacement @directive1 @directive2 { + field: String +} + +type MyConsolidatedTypeWithDirectives @directive1 { + field: String +} + +input MyConsolidatedTypeWithDirectivesInput @directive1 { + field: String +} + +type MyConsolidatedTypeWithDirectives2 @directive2 { + field: String +} + +input MyConsolidatedTypeWithDirectives2Input @directive2 { + field: String +}