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

Validation messages translation #800

Closed
wants to merge 4 commits into from
Closed

Validation messages translation #800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8b23af
Added ability to override methods of `ValuesOfCorrectType`.
LastDragon-ru Mar 25, 2021
1ae2376
Just for case.
LastDragon-ru Mar 25, 2021
477f857
Overwriting test.
LastDragon-ru Mar 25, 2021
4c7f7df
Removed "it('error messages can be overwriten')"
LastDragon-ru Mar 26, 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
2 changes: 1 addition & 1 deletion src/Validator/DocumentValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public static function isValidLiteralValue(Type $type, $valueNode)
$emptyDoc = new DocumentNode(['definitions' => []]);
$typeInfo = new TypeInfo($emptySchema, $type);
$context = new ValidationContext($emptySchema, $emptyDoc, $typeInfo);
$validator = new ValuesOfCorrectType();
$validator = self::getRule(ValuesOfCorrectType::class) ?? new ValuesOfCorrectType();
$visitor = $validator->getVisitor($context);
Visitor::visit($valueNode, Visitor::visitWithTypeInfo($typeInfo, $visitor));

Expand Down
16 changes: 8 additions & 8 deletions src/Validator/Rules/ValuesOfCorrectType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public function getVisitor(ValidationContext $context)

$context->reportError(
new Error(
self::getBadValueMessage((string) $type, Printer::doPrint($node), null, $context, $fieldName),
static::getBadValueMessage((string) $type, Printer::doPrint($node), null, $context, $fieldName),
$node
)
);
Expand Down Expand Up @@ -111,7 +111,7 @@ static function ($field) : string {

$context->reportError(
new Error(
self::requiredFieldMessage($type->name, $fieldName, (string) $fieldType),
static::requiredFieldMessage($type->name, $fieldName, (string) $fieldType),
$node
)
);
Expand All @@ -135,7 +135,7 @@ static function ($field) : string {

$context->reportError(
new Error(
self::unknownFieldMessage($parentType->name, $node->name->value, $didYouMean),
static::unknownFieldMessage($parentType->name, $node->name->value, $didYouMean),
$node
)
);
Expand All @@ -147,7 +147,7 @@ static function ($field) : string {
} elseif (! $type->getValue($node->value)) {
$context->reportError(
new Error(
self::getBadValueMessage(
static::getBadValueMessage(
$type->name,
Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node),
Expand Down Expand Up @@ -198,7 +198,7 @@ private function isValidScalar(ValidationContext $context, ValueNode $node, $fie
if (! $type instanceof ScalarType) {
$context->reportError(
new Error(
self::getBadValueMessage(
static::getBadValueMessage(
(string) $locationType,
Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node),
Expand All @@ -220,7 +220,7 @@ private function isValidScalar(ValidationContext $context, ValueNode $node, $fie
// Ensure a reference to the original error is maintained.
$context->reportError(
new Error(
self::getBadValueMessage(
static::getBadValueMessage(
(string) $locationType,
Printer::doPrint($node),
$error->getMessage(),
Expand Down Expand Up @@ -279,10 +279,10 @@ private static function getBadValueMessage($typeName, $valueName, $message = nul
if ($context) {
$arg = $context->getArgument();
if ($arg) {
return self::badArgumentValueMessage($typeName, $valueName, $fieldName, $arg->name, $message);
return static::badArgumentValueMessage($typeName, $valueName, $fieldName, $arg->name, $message);
}
}

return self::badValueMessage($typeName, $valueName, $message);
return static::badValueMessage($typeName, $valueName, $message);
}
}
36 changes: 36 additions & 0 deletions tests/Validator/ValuesOfCorrectTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use GraphQL\Error\FormattedError;
use GraphQL\Language\SourceLocation;
use GraphQL\Validator\DocumentValidator;
use GraphQL\Validator\Rules\ValuesOfCorrectType;

class ValuesOfCorrectTypeTest extends ValidatorTestCase
Expand Down Expand Up @@ -1649,4 +1650,39 @@ public function testListVariablesWithInvalidItem() : void

self::assertTrue($errors[0]->isClientSafe());
}

public function testOverwriting() : void
{
DocumentValidator::addRule(
new class() extends ValuesOfCorrectType
{
public function getName()
{
return (new ValuesOfCorrectType())->getName();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this function overwritten at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because getName() used by DocumentValidator::addRule() and we want to replace ValuesOfCorrectType completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the default realization of getName() uses static::class

graphql-php/src/Validator/Rules/ValidationRule.php

Lines 16 to 19 in 65428ce

public function getName()
{
return $this->name === '' || $this->name === null ? static::class : $this->name;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems like an annoying gotcha for users that extend this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️ Maybe something like?

class ValuesOfCorrectType extends ValidationRule
{
    // (1)
    /** @var string */
    protected $name = self::class;
    
    // (2) or this (probably a breaking change) 
    public function __construct() { 
        $this->name = self::class;
    }

}

public static function badArgumentValueMessage($typeName, $valueName, $fieldName, $argName, $message = null)
{
return 'overwritten';
}
}
);

$errors = $this->expectInvalid(
self::getTestSchema(),
null,
'
{
complicatedArgs {
stringArgField(stringArg: 1)
}
}
',
[
$this->badValueWithMessage('overwritten', 4, 43),
]
);

self::assertTrue($errors[0]->isClientSafe());
}
}