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.

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

Validate operation names start with upper case characters #109

Merged
merged 2 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

## v0.33.0

### Changed

- Validate operation names start with upper case characters

## v0.32.2

### Changed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
namespace Spawnia\Sailor\PhpKeywords\Operations;

/**
* @extends \Spawnia\Sailor\Operation<\Spawnia\Sailor\PhpKeywords\Operations\_catch\_catchResult>
* @extends \Spawnia\Sailor\Operation<\Spawnia\Sailor\PhpKeywords\Operations\_Catch\_CatchResult>
*/
class _catch extends \Spawnia\Sailor\Operation
class _Catch extends \Spawnia\Sailor\Operation
{
public static function execute(): _catch\_catchResult
public static function execute(): _Catch\_CatchResult
{
return self::executeOperation(
);
Expand All @@ -23,7 +23,7 @@ protected static function converters(): array

public static function document(): string
{
return /* @lang GraphQL */ 'query catch {
return /* @lang GraphQL */ 'query Catch {
__typename
print {
__typename
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
<?php declare(strict_types=1);

namespace Spawnia\Sailor\PhpKeywords\Operations\_catch;
namespace Spawnia\Sailor\PhpKeywords\Operations\_Catch;

/**
* @property string $__typename
* @property \Spawnia\Sailor\PhpKeywords\Operations\_catch\_Print\_Switch|null $print
* @property \Spawnia\Sailor\PhpKeywords\Operations\_Catch\_Print\_Switch|null $print
*/
class _catch extends \Spawnia\Sailor\ObjectLike
class _Catch extends \Spawnia\Sailor\ObjectLike
{
/**
* @param \Spawnia\Sailor\PhpKeywords\Operations\_catch\_Print\_Switch|null $print
* @param \Spawnia\Sailor\PhpKeywords\Operations\_Catch\_Print\_Switch|null $print
*/
public static function make($print = 'Special default value that allows Sailor to differentiate between explicitly passing null and not passing a value at all.'): self
{
Expand All @@ -30,7 +30,7 @@ protected function converters(): array
return $converters ??= [
'__typename' => new \Spawnia\Sailor\Convert\NonNullConverter(new \Spawnia\Sailor\Convert\StringConverter),
'print' => new \Spawnia\Sailor\Convert\NullConverter(new \Spawnia\Sailor\Convert\PolymorphicConverter([
'Switch' => '\\Spawnia\\Sailor\\PhpKeywords\\Operations\\_catch\\_Print\\_Switch',
'Switch' => '\\Spawnia\\Sailor\\PhpKeywords\\Operations\\_Catch\\_Print\\_Switch',
])),
];
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<?php declare(strict_types=1);

namespace Spawnia\Sailor\PhpKeywords\Operations\_catch;
namespace Spawnia\Sailor\PhpKeywords\Operations\_Catch;

class _catchErrorFreeResult extends \Spawnia\Sailor\ErrorFreeResult
class _CatchErrorFreeResult extends \Spawnia\Sailor\ErrorFreeResult
{
public _catch $data;
public _Catch $data;

public static function endpoint(): string
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
<?php declare(strict_types=1);

namespace Spawnia\Sailor\PhpKeywords\Operations\_catch;
namespace Spawnia\Sailor\PhpKeywords\Operations\_Catch;

class _catchResult extends \Spawnia\Sailor\Result
class _CatchResult extends \Spawnia\Sailor\Result
{
public ?_catch $data = null;
public ?_Catch $data = null;

protected function setData(\stdClass $data): void
{
$this->data = _catch::fromStdClass($data);
$this->data = _Catch::fromStdClass($data);
}

/**
* Useful for instantiation of successful mocked results.
*
* @return static
*/
public static function fromData(_catch $data): self
public static function fromData(_Catch $data): self
{
$instance = new static;
$instance->data = $data;

return $instance;
}

public function errorFree(): _catchErrorFreeResult
public function errorFree(): _CatchErrorFreeResult
{
return _catchErrorFreeResult::fromResult($this);
return _CatchErrorFreeResult::fromResult($this);
}

public static function endpoint(): string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php declare(strict_types=1);

namespace Spawnia\Sailor\PhpKeywords\Operations\_catch\_Print;
namespace Spawnia\Sailor\PhpKeywords\Operations\_Catch\_Print;

/**
* @property string $__typename
Expand Down
2 changes: 1 addition & 1 deletion examples/php-keywords/src/ReservedKeywords.graphql
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
query catch {
query Catch {
print {
int
... on Switch {
Expand Down
6 changes: 3 additions & 3 deletions examples/php-keywords/src/test.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

require __DIR__ . '/../vendor/autoload.php';

use Spawnia\Sailor\PhpKeywords\Operations\_catch;
use Spawnia\Sailor\PhpKeywords\Operations\_catch\_Print\_Switch;
use Spawnia\Sailor\PhpKeywords\Operations\_Catch;
use Spawnia\Sailor\PhpKeywords\Operations\_Catch\_Print\_Switch;
use Spawnia\Sailor\PhpKeywords\Types\_abstract;

$result = _catch::execute();
$result = _Catch::execute();

$switch = $result->data->print;
assert($switch instanceof _Switch);
Expand Down
19 changes: 3 additions & 16 deletions src/Codegen/Generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use GraphQL\Error\Error;
use GraphQL\Error\SyntaxError;
use GraphQL\Language\AST\OperationDefinitionNode;
use GraphQL\Language\Parser;
use GraphQL\Type\Schema;
use GraphQL\Utils\BuildSchema;
Expand Down Expand Up @@ -45,13 +44,13 @@ public function generate(): iterable

// Validate the document as defined by the user to give them an error
// message that is more closely related to their source code
Validator::validate($schema, $document);
Validator::validateDocumentWithSchema($schema, $document);

$document = (new FoldFragments($document))->modify();
AddTypename::modify($document);

// Validate again to ensure the modifications we made were safe
Validator::validate($schema, $document);
Validator::validateDocumentWithSchema($schema, $document);

foreach ((new OperationGenerator($schema, $document, $this->endpointConfig))->generate() as $class) {
yield $this->makeFile($class);
Expand Down Expand Up @@ -185,18 +184,6 @@ public static function parseDocuments(array $documents): array
return $parsed;
}

/** @param array<string, \GraphQL\Language\AST\DocumentNode> $parsed */
public static function validateDocuments(array $parsed): void
{
foreach ($parsed as $path => $documentNode) {
foreach ($documentNode->definitions as $definition) {
if ($definition instanceof OperationDefinitionNode && $definition->name === null) {
throw new Error("Found unnamed operation definition in {$path}.", $definition);
}
}
}
}

protected function schema(): Schema
{
$schemaString = \Safe\file_get_contents(
Expand All @@ -218,7 +205,7 @@ protected function parsedDocuments(): array

$parsed = static::parseDocuments($documents);

static::validateDocuments($parsed);
Validator::validateDocuments($parsed);

return $parsed;
}
Expand Down
24 changes: 23 additions & 1 deletion src/Codegen/Validator.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,34 @@
use GraphQL\Error\Error;
use GraphQL\Error\FormattedError;
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Language\AST\OperationDefinitionNode;
use GraphQL\Type\Schema;
use GraphQL\Validator\DocumentValidator;

class Validator
{
public static function validate(Schema $schema, DocumentNode $document): void
/** @param array<string, \GraphQL\Language\AST\DocumentNode> $parsed */
public static function validateDocuments(array $parsed): void
{
foreach ($parsed as $path => $documentNode) {
foreach ($documentNode->definitions as $definition) {
if ($definition instanceof OperationDefinitionNode) {
$nameNode = $definition->name;
if ($nameNode === null) {
throw new Error("Found unnamed operation definition in {$path}.", $definition);
}

$name = $nameNode->value;
$firstChar = $name[0];
if (strtoupper($firstChar) !== $firstChar) {
throw new Error("Operation names must be PascalCase, found {$name} in {$path}.", $definition);
}
}
}
}
}

public static function validateDocumentWithSchema(Schema $schema, DocumentNode $document): void
{
try {
$errors = DocumentValidator::validate($schema, $document);
Expand Down
42 changes: 6 additions & 36 deletions tests/Unit/Codegen/GeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use GraphQL\Language\AST\FragmentDefinitionNode;
use GraphQL\Language\AST\NameNode;
use GraphQL\Language\AST\OperationDefinitionNode;
use GraphQL\Language\Parser;
use Spawnia\Sailor\Codegen\Generator;
use Spawnia\Sailor\Tests\TestCase;

Expand All @@ -15,11 +14,11 @@ public function testParseNamedOperationSuccessfully(): void
{
$somePath = 'path';
$documents = [
$somePath => /* @lang GraphQL */ '
$somePath => /* @lang GraphQL */ <<<'GRAPHQL'
query MyScalarQuery {
simple
}
',
GRAPHQL,
];

$parsed = Generator::parseDocuments($documents);
Expand All @@ -40,11 +39,11 @@ public function testParseFragmentSuccessfully(): void
{
$somePath = 'path';
$documents = [
$somePath => /* @lang GraphQL */ '
$somePath => /* @lang GraphQL */ <<<'GRAPHQL'
fragment Foo on Bar {
simple
}
',
GRAPHQL,
];

$parsed = Generator::parseDocuments($documents);
Expand All @@ -59,15 +58,15 @@ public function testParseOperationsAndFragmentsSuccessfully(): void
{
$somePath = 'path';
$documents = [
$somePath => /* @lang GraphQL */ '
$somePath => /* @lang GraphQL */ <<<'GRAPHQL'
query FooQuery {
...Foo
}

fragment Foo on Bar {
simple
}
',
GRAPHQL,
];

$parsed = Generator::parseDocuments($documents);
Expand Down Expand Up @@ -101,33 +100,4 @@ public function testParseDocumentsThrowsErrorWithPath(): void
self::expectExceptionMessageMatches("/{$path}/");
Generator::parseDocuments($documents);
}

public function testEnsureOperationsAreNamedPasses(): void
{
self::expectNotToPerformAssertions();
$documents = [
'simple' => Parser::parse(/* @lang GraphQL */ '
query Name {
simple
}
'),
];

Generator::validateDocuments($documents);
}

public function testEnsureOperationsAreNamedThrowsErrorWithPath(): void
{
$path = 'thisShouldBeInTheMessage';
$documents = [
$path => Parser::parse(/* @lang GraphQL */ '
{
unnamedQuery
}
'),
];

self::expectExceptionMessageMatches("/{$path}/");
Generator::validateDocuments($documents);
}
}
48 changes: 46 additions & 2 deletions tests/Unit/Codegen/ValidatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public function testValidateSuccess(): void
simple
}
');
Validator::validate($schema, $document);
Validator::validateDocumentWithSchema($schema, $document);
}

public function testValidateFailure(): void
Expand All @@ -42,6 +42,50 @@ public function testValidateFailure(): void
');

$this->expectException(\Exception::class);
Validator::validate($schema, $document);
Validator::validateDocumentWithSchema($schema, $document);
}

public function testValidateDocumentsPasses(): void
{
self::expectNotToPerformAssertions();

Validator::validateDocuments([
'simple' => Parser::parse(/* @lang GraphQL */ <<<'GRAPHQL'
query Name {
simple
}
GRAPHQL),
]);
}

public function testValidateDocumentsUnnamedOperation(): void
{
$path = 'thisShouldBeInTheMessage';
$document = Parser::parse(/* @lang GraphQL */ <<<'GRAPHQL'
{
unnamedQuery
}
GRAPHQL);

self::expectExceptionMessage("Found unnamed operation definition in {$path}.");
Validator::validateDocuments([
$path => $document,
]);
}

public function testValidateDocumentsLowercaseOperation(): void
{
$path = 'thisShouldBeInTheMessage';
$name = 'camelCase';
$document = Parser::parse(/* @lang GraphQL */ <<<GRAPHQL
query {$name} {
field
}
GRAPHQL);

self::expectExceptionMessage("Operation names must be PascalCase, found {$name} in {$path}.");
Validator::validateDocuments([
$path => $document,
]);
}
}
Loading