Merge pull request #377 from yura3d/0.12.x

Argument error handling improvements
This commit is contained in:
Vladimir Razuvaev 2018-11-21 15:07:05 +07:00 committed by GitHub
commit 7d7b693a41
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 108 additions and 94 deletions

View File

@ -20,6 +20,7 @@ use GraphQL\Type\Definition\InputType;
use GraphQL\Type\Definition\InterfaceType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\OutputType;
use GraphQL\Type\Definition\Type;
use GraphQL\Type\Definition\UnionType;
use GraphQL\Type\Definition\WrappingType;

View File

@ -4,6 +4,7 @@ namespace GraphQL\Validator\Rules;
use GraphQL\Error\Error;
use GraphQL\Language\AST\BooleanValueNode;
use GraphQL\Language\AST\EnumValueNode;
use GraphQL\Language\AST\FieldNode;
use GraphQL\Language\AST\FloatValueNode;
use GraphQL\Language\AST\IntValueNode;
use GraphQL\Language\AST\ListValueNode;
@ -17,6 +18,7 @@ use GraphQL\Language\Printer;
use GraphQL\Language\Visitor;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\EnumValueDefinition;
use GraphQL\Type\Definition\FieldArgument;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull;
@ -39,6 +41,12 @@ class ValuesOfCorrectType extends AbstractValidationRule
($message ? "; ${message}" : '.');
}
static function badArgumentValueMessage($typeName, $valueName, $fieldName, $argName, $message = null)
{
return "Field \"{$fieldName}\" argument \"{$argName}\" requires type {$typeName}, found {$valueName}" .
($message ? "; {$message}" : '.');
}
static function requiredFieldMessage($typeName, $fieldName, $fieldTypeName)
{
return "Field {$typeName}.{$fieldName} of required type " .
@ -53,37 +61,53 @@ class ValuesOfCorrectType extends AbstractValidationRule
);
}
private static function getBadValueMessage($typeName, $valueName, $message = null, $context = null, $fieldName = null)
{
if ($context AND $arg = $context->getArgument()) {
return self::badArgumentValueMessage($typeName, $valueName, $fieldName, $arg->name, $message);
} else {
return self::badValueMessage($typeName, $valueName, $message);
}
}
public function getVisitor(ValidationContext $context)
{
$fieldName = '';
return [
NodeKind::NULL => function(NullValueNode $node) use ($context) {
NodeKind::FIELD => [
'enter' => function (FieldNode $node) use (&$fieldName) {
$fieldName = $node->name->value;
}
],
NodeKind::NULL => function(NullValueNode $node) use ($context, &$fieldName) {
$type = $context->getInputType();
if ($type instanceof NonNull) {
$context->reportError(
new Error(
self::badValueMessage((string) $type, Printer::doPrint($node)),
self::getBadValueMessage((string) $type, Printer::doPrint($node), null, $context, $fieldName),
$node
)
);
}
},
NodeKind::LST => function(ListValueNode $node) use ($context) {
NodeKind::LST => function(ListValueNode $node) use ($context, &$fieldName) {
// Note: TypeInfo will traverse into a list's item type, so look to the
// parent input type to check if it is a list.
$type = Type::getNullableType($context->getParentInputType());
if (!$type instanceof ListOfType) {
$this->isValidScalar($context, $node);
$this->isValidScalar($context, $node, $fieldName);
return Visitor::skipNode();
}
},
NodeKind::OBJECT => function(ObjectValueNode $node) use ($context) {
NodeKind::OBJECT => function(ObjectValueNode $node) use ($context, &$fieldName) {
// Note: TypeInfo will traverse into a list's item type, so look to the
// parent input type to check if it is a list.
$type = Type::getNamedType($context->getInputType());
if (!$type instanceof InputObjectType) {
$this->isValidScalar($context, $node);
$this->isValidScalar($context, $node, $fieldName);
return Visitor::skipNode();
}
unset($fieldName);
// Ensure every required field exists.
$inputFields = $type->getFields();
$nodeFields = iterator_to_array($node->fields);
@ -123,31 +147,33 @@ class ValuesOfCorrectType extends AbstractValidationRule
);
}
},
NodeKind::ENUM => function(EnumValueNode $node) use ($context) {
NodeKind::ENUM => function(EnumValueNode $node) use ($context, &$fieldName) {
$type = Type::getNamedType($context->getInputType());
if (!$type instanceof EnumType) {
$this->isValidScalar($context, $node);
$this->isValidScalar($context, $node, $fieldName);
} else if (!$type->getValue($node->value)) {
$context->reportError(
new Error(
self::badValueMessage(
self::getBadValueMessage(
$type->name,
Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node)
$this->enumTypeSuggestion($type, $node),
$context,
$fieldName
),
$node
)
);
}
},
NodeKind::INT => function (IntValueNode $node) use ($context) { $this->isValidScalar($context, $node); },
NodeKind::FLOAT => function (FloatValueNode $node) use ($context) { $this->isValidScalar($context, $node); },
NodeKind::STRING => function (StringValueNode $node) use ($context) { $this->isValidScalar($context, $node); },
NodeKind::BOOLEAN => function (BooleanValueNode $node) use ($context) { $this->isValidScalar($context, $node); },
NodeKind::INT => function (IntValueNode $node) use ($context, &$fieldName) { $this->isValidScalar($context, $node, $fieldName); },
NodeKind::FLOAT => function (FloatValueNode $node) use ($context, &$fieldName) { $this->isValidScalar($context, $node, $fieldName); },
NodeKind::STRING => function (StringValueNode $node) use ($context, &$fieldName) { $this->isValidScalar($context, $node, $fieldName); },
NodeKind::BOOLEAN => function (BooleanValueNode $node) use ($context, &$fieldName) { $this->isValidScalar($context, $node, $fieldName); },
];
}
private function isValidScalar(ValidationContext $context, ValueNode $node)
private function isValidScalar(ValidationContext $context, ValueNode $node, $fieldName)
{
// Report any error at the full type expected by the location.
$locationType = $context->getInputType();
@ -161,10 +187,12 @@ class ValuesOfCorrectType extends AbstractValidationRule
if (!$type instanceof ScalarType) {
$context->reportError(
new Error(
self::badValueMessage(
self::getBadValueMessage(
(string) $locationType,
Printer::doPrint($node),
$this->enumTypeSuggestion($type, $node)
$this->enumTypeSuggestion($type, $node),
$context,
$fieldName
),
$node
)
@ -177,35 +205,33 @@ class ValuesOfCorrectType extends AbstractValidationRule
try {
$type->parseLiteral($node);
} catch (\Exception $error) {
// Ensure a reference to the original error is maintained.
// We should not pass $error to "previous" parameter of Error's constructor here,
// otherwise this error will be in "internal" category instead of "graphql".
$context->reportError(
new Error(
self::badValueMessage(
self::getBadValueMessage(
(string) $locationType,
Printer::doPrint($node),
$error->getMessage()
$error->getMessage(),
$context,
$fieldName
),
$node,
null,
null,
null,
$error
$node
)
);
} catch (\Throwable $error) {
// Ensure a reference to the original error is maintained.
// We should not pass $error to "previous" parameter of Error's constructor here,
// otherwise this error will be in "internal" category instead of "graphql".
$context->reportError(
new Error(
self::badValueMessage(
self::getBadValueMessage(
(string) $locationType,
Printer::doPrint($node),
$error->getMessage()
$error->getMessage(),
$context,
$fieldName
),
$node,
null,
null,
null,
$error
$node
)
);
}

View File

@ -220,7 +220,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
'{ colorEnum(fromEnum: "GREEN") }',
null,
[
'message' => "Expected type Color, found \"GREEN\"; Did you mean the enum value GREEN?",
'message' => "Field \"colorEnum\" argument \"fromEnum\" requires type Color, found \"GREEN\"; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)]
]
);
@ -235,7 +235,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
'{ colorEnum(fromEnum: GREENISH) }',
null,
[
'message' => "Expected type Color, found GREENISH; Did you mean the enum value GREEN?",
'message' => "Field \"colorEnum\" argument \"fromEnum\" requires type Color, found GREENISH; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)]
]
);
@ -250,7 +250,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
'{ colorEnum(fromEnum: green) }',
null,
[
'message' => "Expected type Color, found green; Did you mean the enum value GREEN?",
'message' => "Field \"colorEnum\" argument \"fromEnum\" requires type Color, found green; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)]
]
);
@ -280,7 +280,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
$this->expectFailure(
'{ colorEnum(fromEnum: 1) }',
null,
"Expected type Color, found 1."
"Field \"colorEnum\" argument \"fromEnum\" requires type Color, found 1."
);
}
@ -292,7 +292,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
$this->expectFailure(
'{ colorEnum(fromInt: GREEN) }',
null,
"Expected type Int, found GREEN."
"Field \"colorEnum\" argument \"fromInt\" requires type Int, found GREEN."
);
}

View File

@ -36,7 +36,7 @@ class ValidationTest extends TestCase
';
$expectedError = [
'message' => "Expected type Invalid, found \"bad value\"; Invalid scalar is always invalid: bad value",
'message' => "Field \"invalidArg\" argument \"arg\" requires type Invalid, found \"bad value\"; Invalid scalar is always invalid: bad value",
'locations' => [ ['line' => 3, 'column' => 25] ]
];

View File

@ -19,6 +19,11 @@ class ValuesOfCorrectTypeTest extends TestCase
);
}
private function badValueWithMessage($message, $line, $column)
{
return FormattedError::create($message, [new SourceLocation($line, $column)]);
}
private function requiredField($typeName, $fieldName, $fieldTypeName, $line, $column) {
return FormattedError::create(
ValuesOfCorrectType::requiredFieldMessage(
@ -231,7 +236,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', '1', 4, 39)
$this->badValueWithMessage("Field \"stringArgField\" argument \"stringArg\" requires type String, found 1.", 4, 39)
]);
}
@ -247,7 +252,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', '1.0', 4, 39)
$this->badValueWithMessage("Field \"stringArgField\" argument \"stringArg\" requires type String, found 1.0.", 4, 39)
]);
}
@ -263,7 +268,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', 'true', 4, 39)
$this->badValueWithMessage("Field \"stringArgField\" argument \"stringArg\" requires type String, found true.", 4, 39)
]);
}
@ -279,7 +284,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', 'BAR', 4, 39)
$this->badValueWithMessage("Field \"stringArgField\" argument \"stringArg\" requires type String, found BAR.", 4, 39)
]);
}
@ -297,7 +302,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int', '"3"', 4, 33)
$this->badValueWithMessage("Field \"intArgField\" argument \"intArg\" requires type Int, found \"3\".", 4, 33)
]);
}
@ -313,7 +318,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int', '829384293849283498239482938', 4, 33)
$this->badValueWithMessage("Field \"intArgField\" argument \"intArg\" requires type Int, found 829384293849283498239482938.", 4, 33)
]);
}
@ -329,7 +334,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int', 'FOO', 4, 33)
$this->badValueWithMessage("Field \"intArgField\" argument \"intArg\" requires type Int, found FOO.", 4, 33)
]);
}
@ -345,7 +350,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int', '3.0', 4, 33)
$this->badValueWithMessage("Field \"intArgField\" argument \"intArg\" requires type Int, found 3.0.", 4, 33)
]);
}
@ -361,7 +366,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int', '3.333', 4, 33)
$this->badValueWithMessage("Field \"intArgField\" argument \"intArg\" requires type Int, found 3.333.", 4, 33)
]);
}
@ -379,7 +384,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Float', '"3.333"', 4, 37)
$this->badValueWithMessage("Field \"floatArgField\" argument \"floatArg\" requires type Float, found \"3.333\".", 4, 37)
]);
}
@ -395,7 +400,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Float', 'true', 4, 37)
$this->badValueWithMessage("Field \"floatArgField\" argument \"floatArg\" requires type Float, found true.", 4, 37)
]);
}
@ -411,7 +416,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Float', 'FOO', 4, 37)
$this->badValueWithMessage("Field \"floatArgField\" argument \"floatArg\" requires type Float, found FOO.", 4, 37)
]);
}
@ -429,7 +434,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Boolean', '2', 4, 41)
$this->badValueWithMessage("Field \"booleanArgField\" argument \"booleanArg\" requires type Boolean, found 2.", 4, 41)
]);
}
@ -445,7 +450,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Boolean', '1.0', 4, 41)
$this->badValueWithMessage("Field \"booleanArgField\" argument \"booleanArg\" requires type Boolean, found 1.0.", 4, 41)
]);
}
@ -461,7 +466,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Boolean', '"true"', 4, 41)
$this->badValueWithMessage("Field \"booleanArgField\" argument \"booleanArg\" requires type Boolean, found \"true\".", 4, 41)
]);
}
@ -477,7 +482,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Boolean', 'TRUE', 4, 41)
$this->badValueWithMessage("Field \"booleanArgField\" argument \"booleanArg\" requires type Boolean, found TRUE.", 4, 41)
]);
}
@ -495,7 +500,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('ID', '1.0', 4, 31)
$this->badValueWithMessage("Field \"idArgField\" argument \"idArg\" requires type ID, found 1.0.", 4, 31)
]);
}
@ -511,7 +516,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('ID', 'true', 4, 31)
$this->badValueWithMessage("Field \"idArgField\" argument \"idArg\" requires type ID, found true.", 4, 31)
]);
}
@ -527,7 +532,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('ID', 'SOMETHING', 4, 31)
$this->badValueWithMessage("Field \"idArgField\" argument \"idArg\" requires type ID, found SOMETHING.", 4, 31)
]);
}
@ -545,7 +550,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('DogCommand', '2', 4, 41)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found 2.", 4, 41)
]);
}
@ -561,7 +566,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('DogCommand', '1.0', 4, 41)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found 1.0.", 4, 41)
]);
}
@ -577,13 +582,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue(
'DogCommand',
'"SIT"',
4,
41,
'Did you mean the enum value SIT?'
)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found \"SIT\"; Did you mean the enum value SIT?", 4, 41)
]);
}
@ -599,7 +598,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('DogCommand', 'true', 4, 41)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found true.", 4, 41)
]);
}
@ -615,7 +614,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('DogCommand', 'JUGGLE', 4, 41)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found JUGGLE.", 4, 41)
]);
}
@ -631,13 +630,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue(
'DogCommand',
'sit',
4,
41,
'Did you mean the enum value SIT?'
)
$this->badValueWithMessage("Field \"doesKnowCommand\" argument \"dogCommand\" requires type DogCommand, found sit; Did you mean the enum value SIT?", 4, 41)
]);
}
@ -713,7 +706,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', '2', 4, 55),
$this->badValueWithMessage("Field \"stringListArgField\" argument \"stringListArg\" requires type String, found 2.", 4, 55),
]);
}
@ -729,7 +722,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('[String]', '1', 4, 47),
$this->badValueWithMessage("Field \"stringListArgField\" argument \"stringListArg\" requires type [String], found 1.", 4, 47),
]);
}
@ -889,8 +882,8 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int!', '"two"', 4, 32),
$this->badValue('Int!', '"one"', 4, 45),
$this->badValueWithMessage("Field \"multipleReqs\" argument \"req2\" requires type Int!, found \"two\".", 4, 32),
$this->badValueWithMessage("Field \"multipleReqs\" argument \"req1\" requires type Int!, found \"one\".", 4, 45),
]);
}
@ -906,7 +899,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int!', '"one"', 4, 32),
$this->badValueWithMessage("Field \"multipleReqs\" argument \"req1\" requires type Int!, found \"one\".", 4, 32),
]);
}
@ -922,7 +915,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Int!', 'null', 4, 32),
$this->badValueWithMessage("Field \"multipleReqs\" argument \"req1\" requires type Int!, found null.", 4, 32),
]);
}
@ -1058,7 +1051,7 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('String', '2', 5, 40),
$this->badValueWithMessage("Field \"complexArgField\" argument \"complexArg\" requires type String, found 2.", 5, 40),
]);
}
@ -1102,18 +1095,12 @@ class ValuesOfCorrectTypeTest extends TestCase
invalidArg(arg: 123)
}
', [
$this->badValue(
'Invalid',
'123',
3,
27,
'Invalid scalar is always invalid: 123'
),
$this->badValueWithMessage("Field \"invalidArg\" argument \"arg\" requires type Invalid, found 123; Invalid scalar is always invalid: 123", 3, 27),
]);
$this->assertEquals(
'Invalid scalar is always invalid: 123',
$errors[0]->getPrevious()->getMessage()
"Field \"invalidArg\" argument \"arg\" requires type Invalid, found 123; Invalid scalar is always invalid: 123",
$errors[0]->getMessage()
);
}
@ -1163,8 +1150,8 @@ class ValuesOfCorrectTypeTest extends TestCase
}
}
', [
$this->badValue('Boolean!', '"yes"', 3, 28),
$this->badValue('Boolean!', 'ENUM', 4, 28),
$this->badValueWithMessage("Field \"dog\" argument \"if\" requires type Boolean!, found \"yes\".", 3, 28),
$this->badValueWithMessage("Field \"name\" argument \"if\" requires type Boolean!, found ENUM.", 4, 28),
]);
}