Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Preserve extended methods from class-based types in SchemaExtender::extend() #934

Merged
merged 13 commits into from
Sep 10, 2021
Merged

Preserve extended methods from class-based types in SchemaExtender::extend() #934

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f1ee6a8
Add class-defined scalar test case for SchemaExtenderTest
abdullahseba Sep 8, 2021
c6aa310
Fix formatting
spawnia Sep 8, 2021
42215cc
Clarify types
spawnia Sep 8, 2021
162919b
Implement possible fix for #933 and rewrite test
abdullahseba Sep 8, 2021
8420b3b
Fix codestyle
spawnia Sep 9, 2021
bcd73c3
Make the test laser-focused on the actual problem
spawnia Sep 9, 2021
a80b4bf
fix codestyle
spawnia Sep 9, 2021
d97d754
Fix other method references
spawnia Sep 9, 2021
c10c605
changelog
spawnia Sep 9, 2021
62cc470
Reference method with ()
spawnia Sep 10, 2021
7b4a03b
baseline over ignore
spawnia Sep 10, 2021
dce75fa
justify @var hints
spawnia Sep 10, 2021
472d0f6
namespace test-specific classes
spawnia Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Language/AST/NodeList.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class NodeList implements ArrayAccess, IteratorAggregate, Countable

/**
* @param array<Node|array<string, mixed>> $nodes
* @phpstan-param array<T|array<string, mixed>> $nodes
*
* @phpstan-param array<T|array<string, mixed>> $nodes
* @phpstan-return self<T>
*/
public static function create(array $nodes): self
Expand All @@ -43,6 +43,7 @@ public static function create(array $nodes): self

/**
* @param array<Node|array> $nodes
*
* @phpstan-param array<T|array<string, mixed>> $nodes
*/
public function __construct(array $nodes)
Expand Down Expand Up @@ -90,6 +91,7 @@ public function offsetGet($offset)// : Node
/**
* @param int|string|null $offset
* @param Node|array<string, mixed> $value
*
* @phpstan-param T|array<string, mixed> $value
*/
#[ReturnTypeWillChange]
Expand Down Expand Up @@ -134,8 +136,8 @@ public function splice(int $offset, int $length, $replacement = null): NodeList

/**
* @param NodeList|array<Node|array<string, mixed>> $list
* @phpstan-param NodeList<T>|array<T> $list
*
* @phpstan-param NodeList<T>|array<T> $list
* @phpstan-return NodeList<T>
*/
public function merge($list): NodeList
Expand Down
3 changes: 1 addition & 2 deletions src/Type/Schema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,9 @@ private function defaultTypeLoader(string $typeName): ?Type

/**
* @param Type|callable $type
* @phpstan-param T|callable():T $type
*
* @phpstan-param T|callable():T $type
* @phpstan-return T
*
* @template T of Type
*/
public static function resolveType($type): Type
Expand Down
122 changes: 65 additions & 57 deletions src/Utils/SchemaExtender.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,16 @@ class SchemaExtender
{
public const SCHEMA_EXTENSION = 'SchemaExtension';

/** @var Type[] */
protected static $extendTypeCache;
/** @var array<string, Type> */
protected static array $extendTypeCache;

/** @var mixed[] */
protected static $typeExtensionsMap;
/** @var array<string, array<TypeExtensionNode>> */
protected static array $typeExtensionsMap;

/** @var ASTDefinitionBuilder */
protected static $astBuilder;
protected static ASTDefinitionBuilder $astBuilder;

/**
* @return TypeExtensionNode[]|null
* @return array<TypeExtensionNode>|null
*/
protected static function getExtensionASTNodes(NamedType $type): ?array
{
Expand Down Expand Up @@ -147,9 +146,7 @@ protected static function extendUnionType(UnionType $type): UnionType
return new UnionType([
'name' => $type->name,
'description' => $type->description,
'types' => static function () use ($type): array {
return static::extendPossibleTypes($type);
},
'types' => static fn (): array => static::extendPossibleTypes($type),
'astNode' => $type->astNode,
'resolveType' => $type->config['resolveType'] ?? null,
'extensionASTNodes' => static::getExtensionASTNodes($type),
Expand All @@ -172,16 +169,14 @@ protected static function extendInputObjectType(InputObjectType $type): InputObj
return new InputObjectType([
'name' => $type->name,
'description' => $type->description,
'fields' => static function () use ($type): array {
return static::extendInputFieldMap($type);
},
'fields' => static fn (): array => static::extendInputFieldMap($type),
'astNode' => $type->astNode,
'extensionASTNodes' => static::getExtensionASTNodes($type),
]);
}

/**
* @return mixed[]
* @return array<string, array<string, mixed>>
*/
protected static function extendInputFieldMap(InputObjectType $type): array
{
Expand All @@ -203,6 +198,7 @@ protected static function extendInputFieldMap(InputObjectType $type): array

$extensions = static::$typeExtensionsMap[$type->name] ?? null;
if ($extensions !== null) {
/** @var InputObjectTypeExtensionNode $extension */
spawnia marked this conversation as resolved.
Show resolved Hide resolved
foreach ($extensions as $extension) {
foreach ($extension->fields as $field) {
$fieldName = $field->name->value;
Expand All @@ -219,7 +215,7 @@ protected static function extendInputFieldMap(InputObjectType $type): array
}

/**
* @return mixed[]
* @return array<string, array<string, mixed>>
*/
protected static function extendValueMap(EnumType $type): array
{
Expand All @@ -242,6 +238,7 @@ protected static function extendValueMap(EnumType $type): array

$extensions = static::$typeExtensionsMap[$type->name] ?? null;
if ($extensions !== null) {
/** @var EnumTypeExtensionNode $extension */
foreach ($extensions as $extension) {
foreach ($extension->values as $value) {
$valueName = $value->name->value;
Expand All @@ -258,7 +255,7 @@ protected static function extendValueMap(EnumType $type): array
}

/**
* @return ObjectType[]
* @return array<int, Type> Should be ObjectType, will be caught in schema validation
*/
protected static function extendPossibleTypes(UnionType $type): array
{
Expand All @@ -269,6 +266,7 @@ protected static function extendPossibleTypes(UnionType $type): array

$extensions = static::$typeExtensionsMap[$type->name] ?? null;
if ($extensions !== null) {
/** @var UnionTypeExtensionNode $extension */
foreach ($extensions as $extension) {
foreach ($extension->types as $namedType) {
$possibleTypes[] = static::$astBuilder->buildType($namedType);
Expand All @@ -282,7 +280,7 @@ protected static function extendPossibleTypes(UnionType $type): array
/**
* @param ObjectType|InterfaceType $type
*
* @return array<int, InterfaceType>
* @return array<int, Type> Should be InterfaceType, will be caught in schema validation
*/
protected static function extendImplementedInterfaces(ImplementingType $type): array
{
Expand All @@ -296,15 +294,22 @@ protected static function extendImplementedInterfaces(ImplementingType $type): a
/** @var ObjectTypeExtensionNode|InterfaceTypeExtensionNode $extension */
foreach ($extensions as $extension) {
foreach ($extension->interfaces as $namedType) {
$interfaces[] = static::$astBuilder->buildType($namedType);
/** @var InterfaceType $interface */
spawnia marked this conversation as resolved.
Show resolved Hide resolved
$interface = static::$astBuilder->buildType($namedType);
$interfaces[] = $interface;
}
}
}

return $interfaces;
}

protected static function extendType($typeDef)
/**
* @param ListOfType|NonNull|(Type &NamedType) $typeDef
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param ListOfType|NonNull|(Type &NamedType) $typeDef
* @param ListOfType|NonNull|(Type&NamedType) $typeDef

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that, PHP_CS keeps messing it up. I don't like that tool, php-cs-fixer does a much better job in my experience.

*
* @return ListOfType|NonNull|(Type&NamedType)
*/
protected static function extendType(Type $typeDef): Type
{
if ($typeDef instanceof ListOfType) {
return Type::listOf(static::extendType($typeDef->getOfType()));
Expand All @@ -314,45 +319,43 @@ protected static function extendType($typeDef)
return Type::nonNull(static::extendType($typeDef->getWrappedType()));
}

// @phpstan-ignore-next-line generic is not properly caught
spawnia marked this conversation as resolved.
Show resolved Hide resolved
return static::extendNamedType($typeDef);
}

/**
* @param FieldArgument[] $args
* @param array<FieldArgument> $args
*
* @return mixed[]
* @return array<string, array<string, mixed>>
*/
protected static function extendArgs(array $args): array
{
return Utils::keyValMap(
$args,
static function (FieldArgument $arg): string {
return $arg->name;
},
static function (FieldArgument $arg): array {
$def = [
'type' => static::extendType($arg->getType()),
'description' => $arg->description,
'astNode' => $arg->astNode,
];

if ($arg->defaultValueExists()) {
$def['defaultValue'] = $arg->defaultValue;
}
$extended = [];
foreach ($args as $arg) {
$def = [
'type' => static::extendType($arg->getType()),
'description' => $arg->description,
'astNode' => $arg->astNode,
];

return $def;
if ($arg->defaultValueExists()) {
$def['defaultValue'] = $arg->defaultValue;
}
);

$extended[$arg->name] = $def;
}

return $extended;
}

/**
* @param InterfaceType|ObjectType $type
*
* @return mixed[]
* @return array<string, array<string, mixed>>
*
* @throws Error
*/
protected static function extendFieldMap($type): array
protected static function extendFieldMap(Type $type): array
{
$newFieldMap = [];
$oldFieldMap = $type->getFields();
Expand All @@ -373,6 +376,7 @@ protected static function extendFieldMap($type): array

$extensions = static::$typeExtensionsMap[$type->name] ?? null;
if ($extensions !== null) {
/** @var ObjectTypeExtensionNode|InputObjectTypeExtensionNode $extension */
spawnia marked this conversation as resolved.
Show resolved Hide resolved
foreach ($extensions as $extension) {
foreach ($extension->fields as $field) {
$fieldName = $field->name->value;
Expand All @@ -393,12 +397,8 @@ protected static function extendObjectType(ObjectType $type): ObjectType
return new ObjectType([
'name' => $type->name,
'description' => $type->description,
'interfaces' => static function () use ($type): array {
return static::extendImplementedInterfaces($type);
},
'fields' => static function () use ($type): array {
return static::extendFieldMap($type);
},
'interfaces' => static fn (): array => static::extendImplementedInterfaces($type),
'fields' => static fn (): array => static::extendFieldMap($type),
'astNode' => $type->astNode,
'extensionASTNodes' => static::getExtensionASTNodes($type),
'isTypeOf' => $type->config['isTypeOf'] ?? null,
Expand All @@ -411,12 +411,8 @@ protected static function extendInterfaceType(InterfaceType $type): InterfaceTyp
return new InterfaceType([
'name' => $type->name,
'description' => $type->description,
'interfaces' => static function () use ($type): array {
return static::extendImplementedInterfaces($type);
},
'fields' => static function () use ($type): array {
return static::extendFieldMap($type);
},
'interfaces' => static fn (): array => static::extendImplementedInterfaces($type),
'fields' => static fn (): array => static::extendFieldMap($type),
'astNode' => $type->astNode,
'extensionASTNodes' => static::getExtensionASTNodes($type),
'resolveType' => $type->config['resolveType'] ?? null,
Expand All @@ -435,7 +431,14 @@ protected static function isSpecifiedScalarType(Type $type): bool
);
}

protected static function extendNamedType(Type $type)
/**
* @param T $type
*
* @return T
*
* @template T of Type
*/
protected static function extendNamedType($type)
{
if (Introspection::isIntrospectionType($type) || static::isSpecifiedScalarType($type)) {
return $type;
Expand All @@ -458,13 +461,18 @@ protected static function extendNamedType(Type $type)
}
}

// @phpstan-ignore-next-line the lines above ensure only matching subtypes get in here
return static::$extendTypeCache[$name];
}

/**
* @return mixed|null
* @param T|null $type
*
* @return T|null
*
* @template T of Type
*/
protected static function extendMaybeNamedType(?NamedType $type = null)
protected static function extendMaybeNamedType(?Type $type = null): ?Type
{
if ($type !== null) {
return static::extendNamedType($type);
Expand All @@ -474,9 +482,9 @@ protected static function extendMaybeNamedType(?NamedType $type = null)
}

/**
* @param DirectiveDefinitionNode[] $directiveDefinitions
* @param array<DirectiveDefinitionNode> $directiveDefinitions
*
* @return Directive[]
* @return array<int, Directive>
*/
protected static function getMergedDirectives(Schema $schema, array $directiveDefinitions): array
{
Expand Down
Loading