Only generate connection objects if connection is annotated with Obje…

…ctType
slackhq · Feb 28, 2022 · d27dcf9 · d27dcf9
1 parent e51361d
commit d27dcf9
Show file tree

Hide file tree

Showing 12 changed files with 48 additions and 53 deletions.
diff --git a/src/Codegen/Builders/ObjectBuilder.hack b/src/Codegen/Builders/ObjectBuilder.hack
@@ -64,17 +64,12 @@ class ObjectBuilder extends CompositeBuilder {
 
     public static function forConnection(
         string $name,
+        \Slack\GraphQL\ObjectType $object_type,
         string $edge_name,
         vec<FieldBuilder> $additional_fields,
     ): ObjectBuilder {
-        // Remove namespace to generate a sane GQL name
-        // This means that connections in different namespaces can collide with each other;
-        // we could eventually fix that by merging the namespace and GQL name when
-        // generating the connection but doesn't seem worth it currently.
-        $gql_name = Str\split($name, '\\')
-            |> C\lastx($$);
         return new ObjectBuilder(
-            new \Slack\GraphQL\ObjectType($gql_name, $gql_name), // TODO: Description
+            $object_type,
             $name, // hack type
             Vec\concat(
                 vec[ // fields

diff --git a/src/Codegen/Generator.hack b/src/Codegen/Generator.hack
@@ -154,11 +154,8 @@ final class Generator {
     private static function classnameDict(dict<string, string> $dict): string {
         $lines = vec['dict['];
         foreach (Dict\sort_by_key($dict) as $key => $value) {
-            $lines[] = Str\format(
-                "  %s => %s::class,",
-                \var_export($key, true),
-                Str\strip_prefix($value, 'Slack\\GraphQL\\'),
-            );
+            $lines[] =
+                Str\format("  %s => %s::class,", \var_export($key, true), Str\strip_prefix($value, 'Slack\\GraphQL\\'));
         }
         $lines[] = ']';
         return Str\join($lines, "\n");
@@ -245,14 +242,21 @@ final class Generator {
             $rc = new \ReflectionClass($class->getName());
             if (
                 \is_subclass_of($class->getName(), \Slack\GraphQL\Pagination\Connection::class) &&
-                $rc->getAttributeClass(\Slack\GraphQL\ObjectType::class)
+                $rc->getAttributeClass(\Slack\GraphQL\ObjectType::class) is nonnull
             ) {
                 invariant(
                     Str\ends_with($class->getName(), 'Connection'),
                     "All connection types must have names ending with `Connection`. `%s` does not.",
                     $class->getName(),
                 );
-                $objects = Vec\concat($objects, $this->getConnectionObjects($class, $class_fields[$class->getName()]));
+                $objects = Vec\concat(
+                    $objects,
+                    $this->getConnectionObjects(
+                        $class,
+                        $rc->getAttributeClass(\Slack\GraphQL\ObjectType::class) as nonnull,
+                        $class_fields[$class->getName()],
+                    ),
+                );
             } elseif (C\contains_key($class_fields, $class->getName())) {
                 $rc = new \ReflectionClass($class->getName());
                 $fields = $class_fields[$class->getName()];
@@ -271,12 +275,8 @@ final class Generator {
                         $hack_class_to_graphql_object,
                     );
                 } else if ($graphql_object is nonnull) {
-                    $objects[] = new ObjectBuilder(
-                        $graphql_object,
-                        $rc->getName(),
-                        $fields,
-                        $hack_class_to_graphql_interface,
-                    );
+                    $objects[] =
+                        new ObjectBuilder($graphql_object, $rc->getName(), $fields, $hack_class_to_graphql_interface);
                 }
             }
 
@@ -298,10 +298,8 @@ final class Generator {
                 if ($mutation_root_field is nonnull) {
                     $this->has_mutations = true;
 
-                    $mutation_fields[$mutation_root_field->getName()] = FieldBuilder::forRootField(
-                        $mutation_root_field,
-                        $rm,
-                    );
+                    $mutation_fields[$mutation_root_field->getName()] =
+                        FieldBuilder::forRootField($mutation_root_field, $rm);
                 }
             }
         }
@@ -335,6 +333,7 @@ final class Generator {
 
     private function getConnectionObjects(
         DefinitionFinder\ScannedClassish $class,
+        \Slack\GraphQL\ObjectType $object_type,
         vec<FieldBuilder> $additional_fields,
     ): vec<ObjectBuilder> {
         $rc = new \ReflectionClass($class->getName());
@@ -349,7 +348,12 @@ final class Generator {
             $rc->getName(),
         );
         return vec[
-            ObjectBuilder::forConnection($class->getName(), $type_info['gql_type'].'Edge', $additional_fields),
+            ObjectBuilder::forConnection(
+                $class->getName(),
+                $object_type,
+                $type_info['gql_type'].'Edge',
+                $additional_fields,
+            ),
             ObjectBuilder::forEdge($type_info['gql_type'], $type_info['hack_type'], $type_info['output_type']),
         ];
     }

diff --git a/src/Codegen/Types.hack b/src/Codegen/Types.hack
@@ -156,10 +156,8 @@ function unwrap_type(IO $io, string $hack_type, bool $nullable = false): (string
         return unwrap_type($io, Str\strip_prefix($hack_type, '?'), true);
     }
     if (Str\starts_with($hack_type, 'HH\vec<')) {
-        list($unwrapped, $suffix) = unwrap_type(
-            $io,
-            Str\strip_prefix($hack_type, 'HH\vec<') |> Str\strip_suffix($$, '>'),
-        );
+        list($unwrapped, $suffix) =
+            unwrap_type($io, Str\strip_prefix($hack_type, 'HH\vec<') |> Str\strip_suffix($$, '>'));
         return tuple($unwrapped, $suffix.($nullable ? '->nullable'.$io.'ListOf()' : '->nonNullable'.$io.'ListOf()'));
     }
     return tuple($hack_type, $nullable ? '::nullable'.$io.'()' : '::nonNullable()');

diff --git a/src/FieldCollector.hack b/src/FieldCollector.hack
@@ -106,10 +106,8 @@ final class FieldCollector {
             }
 
             try {
-                $arg_is_true = Types\BooleanType::nonNullable()->coerceNode(
-                    $arg->getValue(),
-                    $this->context->getVariableValues(),
-                );
+                $arg_is_true =
+                    Types\BooleanType::nonNullable()->coerceNode($arg->getValue(), $this->context->getVariableValues());
             } catch (UserFacingError $_) {
                 // This can only happen if the query wasn't validated or if variable values weren't coerced to the
                 // correct type at the beginning of execution.

diff --git a/src/Graphpinator/Parser/Parser.hack b/src/Graphpinator/Parser/Parser.hack
@@ -69,9 +69,8 @@ final class Parser {
             default:
                 foreach ($operations as $operation) {
                     if ($operation->getName() === null) {
-                        throw new \Graphpinator\Parser\Exception\OperationWithoutName(
-                            $locations[$operation->getName()],
-                        );
+                        throw
+                            new \Graphpinator\Parser\Exception\OperationWithoutName($locations[$operation->getName()]);
                     }
                 }
         }
@@ -352,13 +351,8 @@ final class Parser {
                 $default = $this->parseValue(true);
             }
 
-            $variables[$name] = new \Graphpinator\Parser\Variable\Variable(
-                $location,
-                $name,
-                $type,
-                $default,
-                $this->parseDirectives(),
-            );
+            $variables[$name] =
+                new \Graphpinator\Parser\Variable\Variable($location, $name, $type, $default, $this->parseDirectives());
         }
 
         $this->tokenizer->getNext();

diff --git a/src/Graphpinator/Tokenizer/Tokenizer.hack b/src/Graphpinator/Tokenizer/Tokenizer.hack
@@ -391,9 +391,8 @@ final class Tokenizer implements \HH\KeyedIterator<int, ?Token> {
 
         if ($this->source->getChar() === '-') {
             if (!$negative) {
-                throw new \Graphpinator\Exception\Tokenizer\NumericLiteralNegativeFraction(
-                    $this->source->getLocation(),
-                );
+                throw
+                    new \Graphpinator\Exception\Tokenizer\NumericLiteralNegativeFraction($this->source->getLocation());
             }
 
             $sign = '-';

diff --git a/src/Pagination/Connection.hack b/src/Pagination/Connection.hack
@@ -1,5 +1,6 @@
 
 
+
 namespace Slack\GraphQL\Pagination;
 
 use namespace HH\Lib\{C, Vec};
@@ -57,7 +58,6 @@ abstract class Connection {
      *
      * This should be the Hack class over which you want to paginate.
      */
-     <<__Enforceable>>
     abstract const type TNode;
 
     /**
@@ -177,7 +177,7 @@ abstract class Connection {
      * method with the pagination args.
      */
     <<__Memoize>>
-    final private async function paginate(): Awaitable<shape(
+    private async function paginate(): Awaitable<shape(
         'edges' => vec<Edge<this::TNode>>,
         'pageInfo' => PageInfo,
     )> {

diff --git a/src/Types/NamedType.hack b/src/Types/NamedType.hack
@@ -11,7 +11,7 @@ abstract class NamedType extends BaseType implements INonNullableType {
     abstract const type THackType as nonnull;
     abstract const string NAME;
 
-    final private function __construct() {}
+    private function __construct() {}
 
     <<__Override>>
     final public function getName(): string {

diff --git a/src/Types/Output/NullableOutputType.hack b/src/Types/Output/NullableOutputType.hack
@@ -33,9 +33,8 @@ final class NullableOutputType<TInner as nonnull, TResolved> extends BaseType {
             return new GraphQL\ValidFieldResult(null);
         }
         $result = await $this->inner_type->resolveAsync($value, $parent_nodes, $context);
-        return $result is GraphQL\ValidFieldResult<_>
-            ? $result
-            : new GraphQL\ValidFieldResult(null, $result->getErrors());
+        return
+            $result is GraphQL\ValidFieldResult<_> ? $result : new GraphQL\ValidFieldResult(null, $result->getErrors());
     }
 
     public function resolveError(GraphQL\UserFacingError $error): GraphQL\ValidFieldResult<null> {

diff --git a/tests/Fixtures/NamespaceTestObjects.hack b/tests/Fixtures/NamespaceTestObjects.hack
@@ -16,6 +16,7 @@ namespace Foo {
         }
     }
 
+    <<GraphQL\ObjectType('FooConnection', 'FooConnection')>>
     final class FooConnection extends GraphQL\Pagination\Connection {
         const type TNode = FooObject;
 

diff --git a/tests/Fixtures/Playground.hack b/tests/Fixtures/Playground.hack
@@ -1,5 +1,6 @@
 
 
+
 use namespace Slack\GraphQL;
 use namespace HH\Lib\{Math, Str, Vec};
 
@@ -245,6 +246,7 @@ abstract final class UserMutationAttributes {
 }
 
 
+<<GraphQL\ObjectType('AlphabetConnection', 'AlphabetConnection')>>
 final class AlphabetConnection extends GraphQL\Pagination\ListConnection {
     const type TNode = string;
 

diff --git a/tests/Fixtures/UserConnection.hack b/tests/Fixtures/UserConnection.hack
@@ -4,7 +4,12 @@
 use namespace Slack\GraphQL;
 use namespace HH\Lib\{Math, Str};
 
-final class UserConnection extends GraphQL\Pagination\Connection {
+// Assert that we skip generating type for connections which aren't
+// annotated with `ObjectType`.
+abstract class MyConnection extends GraphQL\Pagination\Connection {}
+
+<<GraphQL\ObjectType('UserConnection', 'Connection providing access to User objects')>>
+final class UserConnection extends MyConnection {
     const type TNode = User;
 
     public function __construct(private string $name_prefix = 'User') {}
-Original file line number
+Diff line change
@@ Expand Up / @@ -16,6 +16,7 @@ namespace Foo { @@
             }
         }
+        <<GraphQL\ObjectType('FooConnection', 'FooConnection')>>
         final class FooConnection extends GraphQL\Pagination\Connection {
             const type TNode = FooObject;
@@ Expand Down @@