small refactors

danadajian · danadajian · commit 8b5a1c7ca306 · 2024-04-23T15:37:55.000-05:00
diff --git a/bun.lockb b/bun.lockb
diff --git a/eslint.config.js b/eslint.config.js
@@ -13,6 +13,7 @@ export default [
       },
     },
     rules: {
+      "no-console": "error",
       "@typescript-eslint/no-non-null-assertion": "error",
       "@typescript-eslint/no-unsafe-argument": "error",
       "@typescript-eslint/no-unsafe-call": "error",
diff --git a/package.json b/package.json
@@ -28,7 +28,7 @@
     "@graphql-codegen/cli": "5.0.2",
     "@total-typescript/ts-reset": "0.5.1",
     "bun-types": "1.1.4",
-    "eslint": "9.1.0",
+    "eslint": "9.1.1",
     "husky": "9.0.11",
     "prettier": "3.2.5",
     "tsup": "8.0.2",
@@ -39,7 +39,7 @@
     "build": "tsup src/plugin.ts --clean --dts --external graphql",
     "format": "prettier --write .",
     "format-check": "prettier --check .",
-    "integration": "bun run build && graphql-codegen && ./gradlew compileTestKotlin",
+    "integration": "bun run build && graphql-codegen && ./gradlew build",
     "lint": "eslint .",
     "prepack": "bun run build",
     "prepare": "husky",
diff --git a/src/config.ts b/src/config.ts
@@ -46,11 +46,11 @@ export const configSchema = object({
   dependentTypesInScope: optional(array(string())),
   /**
    * Denotes Kotlin classes representing union types to be treated as interfaces rather than annotation classes.
-   * This should be used for types outside `dependentTypesInScope` that are not generated by the plugin.
+   * @description This should be used for types outside `dependentTypesInScope` that are not generated by the plugin. This also should only be used when unionGeneration is set to `ANNOTATION_CLASS`.
    */
   externalUnionsAsInterfaces: optional(array(string())),
   /**
-   * Additional imports to add to the generated file.
+   * Additional imports to add to the generated file. GraphQL Kotlin annotations are always imported.
    * @example ["com.example.additional.import.*"]
    */
   extraImports: optional(array(string())),
@@ -133,6 +133,7 @@ export const configSchema = object({
   ),
   /**
    * Denotes the generation strategy for union types. Defaults to `MARKER_INTERFACE`.
+   *
    * https://opensource.expediagroup.com/graphql-kotlin/docs/schema-generator/writing-schemas/unions/
    */
   unionGeneration: optional(
diff --git a/src/definitions/input.ts b/src/definitions/input.ts
@@ -47,7 +47,6 @@ export function buildInputObjectDefinition(
 
   const annotations = buildAnnotations({
     config,
-    inputDescription: node.description?.value,
     definitionNode: node,
   });
 
diff --git a/src/definitions/object.ts b/src/definitions/object.ts
@@ -49,20 +49,20 @@ export function buildObjectTypeDefinition(
 
   if (isResolverType(node, config)) {
     return `${annotations}@GraphQLIgnore\ninterface ${name}${interfaceInheritance} {
-${getClassMembers({ node, schema, config })}
+${getDataClassMembers({ node, schema, config })}
 }
 
 ${annotations}@GraphQLIgnore\ninterface ${name}CompletableFuture {
-${getClassMembers({ node, schema, config, completableFuture: true })}
+${getDataClassMembers({ node, schema, config, completableFuture: true })}
 }`;
   }
 
   return `${annotations}data class ${name}(
-${getClassMembers({ node, schema, config })}
+${getDataClassMembers({ node, schema, config })}
 )${interfaceInheritance}`;
 }
 
-function getClassMembers({
+function getDataClassMembers({
   node,
   schema,
   config,
diff --git a/src/definitions/union.ts b/src/definitions/union.ts
@@ -13,7 +13,6 @@ limitations under the License.
 
 import { UnionTypeDefinitionNode } from "graphql";
 import { shouldIncludeTypeDefinition } from "../helpers/should-include-type-definition";
-import { buildDirectiveAnnotations } from "../helpers/build-directive-annotations";
 import { CodegenConfigWithDefaults } from "../config";
 import {
   buildAnnotations,
@@ -32,10 +31,9 @@ export function buildUnionTypeDefinition(
     definitionNode: node,
   });
   if (config.unionGeneration === "ANNOTATION_CLASS") {
-    const directiveAnnotations = buildDirectiveAnnotations(node, config);
     const possibleTypes =
       node.types?.map((type) => `${type.name.value}::class`).join(", ") || "";
-    return `${directiveAnnotations}@GraphQLUnion(
+    return `${annotations}@GraphQLUnion(
     name = "${node.name.value}",
     possibleTypes = [${possibleTypes}],
     description = "${trimDescription(node.description?.value)}"
diff --git a/src/helpers/add-dependent-types-to-only-types.ts b/src/helpers/add-dependent-types-to-only-types.ts
@@ -13,24 +13,24 @@ limitations under the License.
 
 import { CodegenConfigWithDefaults } from "../config";
 import { getDependentTypeNames } from "./get-dependent-type-names";
-import { dependentTypeIsInScope } from "./dependent-type-is-in-scope";
 import { GraphQLSchema } from "graphql";
 
-export function addDependentTypes(
+export function addDependentTypesToOnlyTypes(
   config: CodegenConfigWithDefaults,
   schema: GraphQLSchema,
 ) {
   if (!config.onlyTypes) {
-    throw new Error(`config.onlyTypes is required to add dependent types`);
+    throw new Error(`onlyTypes config is required to add dependent types`);
   }
   const onlyTypesNodes = config.onlyTypes
     .map((typeName) => schema.getType(typeName)?.astNode)
     .filter(Boolean);
   const dependentTypeNames = onlyTypesNodes.flatMap((node) =>
     getDependentTypeNames(schema, node, config),
   );
-  const dependentTypesInScope = dependentTypeNames.filter((typeName) =>
-    dependentTypeIsInScope(typeName, config),
-  );
+  const typesInScope = config.dependentTypesInScope;
+  const dependentTypesInScope = typesInScope
+    ? dependentTypeNames.filter((typeName) => typesInScope.includes(typeName))
+    : dependentTypeNames;
   config.onlyTypes.push(...dependentTypesInScope);
 }
diff --git a/src/helpers/build-annotations.ts b/src/helpers/build-annotations.ts
@@ -18,6 +18,7 @@ import {
   InputValueDefinitionNode,
   TypeDefinitionNode,
 } from "graphql";
+import { buildDescriptionAnnotation } from "./build-description-annotation";
 import { buildDirectiveAnnotations } from "./build-directive-annotations";
 import { CodegenConfigWithDefaults } from "../config";
 import { TypeMetadata } from "./build-type-metadata";
@@ -30,37 +31,24 @@ export type DefinitionNode =
 
 export function buildAnnotations({
   config,
-  inputDescription,
   definitionNode,
   resolvedType,
 }: {
   config: CodegenConfigWithDefaults;
-  inputDescription?: string;
-  definitionNode?: DefinitionNode;
+  definitionNode: DefinitionNode;
   resolvedType?: TypeMetadata;
 }) {
-  const description =
-    inputDescription ?? definitionNode?.description?.value ?? "";
-  const isDeprecated = isDeprecatedDescription(description, resolvedType);
-  const descriptionAnnotator = isDeprecated
-    ? "@Deprecated"
-    : "@GraphQLDescription";
-  const descriptionValue = isDeprecated
-    ? description.replace("DEPRECATED: ", "")
-    : description;
-  const trimmedDescription = trimDescription(descriptionValue);
-  const descriptionAnnotation = description
-    ? `${descriptionAnnotator}("${trimmedDescription}")\n`
-    : "";
-
-  const directiveAnnotations = definitionNode
-    ? buildDirectiveAnnotations(
-        definitionNode,
-        config,
-        description,
-        resolvedType,
-      )
-    : "";
+  const description = definitionNode?.description?.value ?? "";
+  const descriptionAnnotation = buildDescriptionAnnotation(
+    description,
+    definitionNode,
+    config,
+    resolvedType,
+  );
+  const directiveAnnotations = buildDirectiveAnnotations(
+    definitionNode,
+    config,
+  );
   const unionAnnotation = resolvedType?.unionAnnotation
     ? `@${resolvedType.unionAnnotation}\n`
     : "";
@@ -84,15 +72,6 @@ export function buildAnnotations({
   );
 }
 
-export function isDeprecatedDescription(
-  description?: string,
-  resolvedType?: TypeMetadata,
-) {
-  return (
-    description?.startsWith("DEPRECATED: ") && !resolvedType?.unionAnnotation
-  );
-}
-
 export function trimDescription(description?: string) {
   return (
     description
diff --git a/src/helpers/build-config-with-defaults.ts b/src/helpers/build-config-with-defaults.ts
@@ -13,7 +13,7 @@ export function buildConfigWithDefaults(
     ...config,
     extraImports: [
       "com.expediagroup.graphql.generator.annotations.*",
-      ...(config.extraImports ?? ([] satisfies string[])),
+      ...(config.extraImports ?? []),
     ],
   } as const;
 }
diff --git a/src/helpers/build-description-annotation.ts b/src/helpers/build-description-annotation.ts
@@ -0,0 +1,53 @@
+import { CodegenConfigWithDefaults } from "../config";
+import { TypeMetadata } from "./build-type-metadata";
+import { indent } from "@graphql-codegen/visitor-plugin-common";
+import { Kind } from "graphql/index";
+import { DefinitionNode, trimDescription } from "./build-annotations";
+
+export function buildDescriptionAnnotation(
+  description: string,
+  definitionNode: DefinitionNode,
+  config: CodegenConfigWithDefaults,
+  resolvedType?: TypeMetadata,
+) {
+  const trimmedDescription = trimDescription(description);
+  const isDeprecatedDescription = descriptionIsDeprecated(trimmedDescription);
+  if (isDeprecatedDescription && resolvedType?.unionAnnotation) {
+    return `@GraphQLDescription("${trimmedDescription}")\n`;
+  } else if (isDeprecatedDescription) {
+    const descriptionValue = description.replace("DEPRECATED: ", "");
+    return `@Deprecated("${trimDescription(descriptionValue)}")\n`;
+  }
+
+  const deprecatedDirective = definitionNode.directives?.find(
+    (directive) => directive.name.value === "deprecated",
+  );
+  const deprecatedReasonNode = deprecatedDirective?.arguments?.find(
+    (arg) => arg.name.value === "reason",
+  )?.value;
+  const deprecatedReason =
+    deprecatedReasonNode?.kind === "StringValue"
+      ? deprecatedReasonNode.value
+      : "";
+  const trimmedDeprecatedReason = trimDescription(deprecatedReason);
+
+  if (deprecatedDirective && resolvedType?.unionAnnotation) {
+    return `@GraphQLDescription("${trimmedDeprecatedReason}")\n`;
+  } else if (deprecatedDirective) {
+    const graphqlDescription = trimmedDescription
+      ? `@GraphQLDescription("${trimmedDescription}")\n`
+      : "";
+    const deprecatedDescription = `@Deprecated("${trimmedDeprecatedReason}")\n`;
+    return `${graphqlDescription}${graphqlDescription ? indent(deprecatedDescription, 2) : deprecatedDescription}`;
+  }
+
+  return trimmedDescription &&
+    (config.unionGeneration === "MARKER_INTERFACE" ||
+      definitionNode?.kind !== Kind.UNION_TYPE_DEFINITION)
+    ? `@GraphQLDescription("${trimmedDescription}")\n`
+    : "";
+}
+
+function descriptionIsDeprecated(description: string) {
+  return description.startsWith("DEPRECATED: ");
+}
diff --git a/src/helpers/build-directive-annotations.ts b/src/helpers/build-directive-annotations.ts
@@ -12,55 +12,26 @@ limitations under the License.
 */
 
 import { CodegenConfigWithDefaults } from "../config";
-import {
-  DefinitionNode,
-  isDeprecatedDescription,
-  trimDescription,
-} from "./build-annotations";
+import { DefinitionNode } from "./build-annotations";
 import { getFederationDirectiveReplacement } from "./get-federation-directive-replacement";
-import { TypeMetadata } from "./build-type-metadata";
 import { ConstDirectiveNode } from "graphql/language";
 
 export function buildDirectiveAnnotations(
-  incomingNode: DefinitionNode,
+  definitionNode: DefinitionNode,
   config: CodegenConfigWithDefaults,
-  description?: string,
-  resolvedType?: TypeMetadata,
 ) {
-  const kind = incomingNode.kind;
-  const directives = incomingNode.directives ?? [];
-
+  const directives = definitionNode.directives ?? [];
   return directives
     .map((directive) => {
       const directiveName = directive.name.value;
-      if (
-        directiveName === "deprecated" &&
-        !isDeprecatedDescription(description)
-      ) {
-        const deprecatedReasonNode = directive.arguments?.find(
-          (arg) => arg.name.value === "reason",
-        )?.value;
-        const deprecatedReason =
-          deprecatedReasonNode?.kind === "StringValue"
-            ? deprecatedReasonNode.value
-            : "";
-        if (incomingNode.description?.value && resolvedType?.unionAnnotation) {
-          return "";
-        }
-        const descriptionAnnotator = resolvedType?.unionAnnotation
-          ? "@GraphQLDescription"
-          : "@Deprecated";
-        const trimmedDeprecatedReason = trimDescription(deprecatedReason);
-        return `${descriptionAnnotator}("${trimmedDeprecatedReason}")\n`;
-      }
       const federationReplacement =
         getFederationDirectiveReplacement(directive);
       if (federationReplacement) return federationReplacement + "\n";
 
       const directiveReplacementFromConfig = config.directiveReplacements?.find(
         ({ directive, definitionType }) =>
           directive === directiveName &&
-          (!definitionType || definitionType === kind),
+          (!definitionType || definitionType === definitionNode.kind),
       );
       if (!directiveReplacementFromConfig) return "";
       const kotlinAnnotations = buildKotlinAnnotations(
diff --git a/src/helpers/dependent-type-is-in-scope.ts b/src/helpers/dependent-type-is-in-scope.ts
diff --git a/src/plugin.ts b/src/plugin.ts
@@ -22,7 +22,7 @@ import {
 } from "@graphql-codegen/visitor-plugin-common";
 import { Input, safeParse } from "valibot";
 import { configSchema } from "./config";
-import { addDependentTypes } from "./helpers/add-dependent-types";
+import { addDependentTypesToOnlyTypes } from "./helpers/add-dependent-types-to-only-types";
 import { visit } from "graphql";
 import { buildConfigWithDefaults } from "./helpers/build-config-with-defaults";
 
@@ -51,12 +51,11 @@ export const plugin: PluginFunction<GraphQLKotlinCodegenConfig> = (
   }
 
   const configWithDefaults = buildConfigWithDefaults(config, info.outputFile);
-
   if (
     configWithDefaults.onlyTypes &&
     configWithDefaults.includeDependentTypes
   ) {
-    addDependentTypes(configWithDefaults, schema);
+    addDependentTypesToOnlyTypes(configWithDefaults, schema);
   }
   const visitor = new KotlinVisitor(configWithDefaults, schema);
   const astNode = getCachedDocumentNodeFromSchema(schema);
@@ -67,7 +66,10 @@ export const plugin: PluginFunction<GraphQLKotlinCodegenConfig> = (
       .map((annotation) => `import ${annotation}`)
       .join("\n") + "\n";
   const typeDefinitions = definitions
-    .filter((d: unknown) => typeof d === "string" && d.length)
+    .filter(
+      (definition: unknown) =>
+        typeof definition === "string" && definition.length,
+    )
     .join("\n\n");
 
   return [packageName, imports, typeDefinitions].join("\n") + "\n";
diff --git a/test/unit/should_honor_union_generation_config/expected.kt b/test/unit/should_honor_union_generation_config/expected.kt
diff --git a/test/unit/should_honor_union_generation_config/schema.graphql b/test/unit/should_honor_union_generation_config/schema.graphql

Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ export function buildConfigWithDefaults(`
`13`	`13`	`...config,`
`14`	`14`	`extraImports: [`
`15`	`15`	`"com.expediagroup.graphql.generator.annotations.*",`
`16`		`- ...(config.extraImports ?? ([] satisfies string[])),`
	`16`	`+ ...(config.extraImports ?? []),`
`17`	`17`	`],`
`18`	`18`	`} as const;`
`19`	`19`	`}`