Add suggestions for invalid values

For misspelled enums or field names, these suggestions can be helpful.

This also changes the suggestions algorithm to better detect case-sensitivity mistakes, which are common

ref: graphql/graphql-js#1153
This commit is contained in:
Daniel Tschinder 2018-02-16 16:19:25 +01:00
parent 48c5e64a08
commit d92a2dab21
7 changed files with 258 additions and 39 deletions

View File

@ -512,6 +512,9 @@ class Utils
* Given an invalid input string and a list of valid options, returns a filtered * Given an invalid input string and a list of valid options, returns a filtered
* list of valid options sorted based on their similarity with the input. * list of valid options sorted based on their similarity with the input.
* *
* Includes a custom alteration from Damerau-Levenshtein to treat case changes
* as a single edit which helps identify mis-cased values with an edit distance
* of 1
* @param string $input * @param string $input
* @param array $options * @param array $options
* @return string[] * @return string[]
@ -521,7 +524,11 @@ class Utils
$optionsByDistance = []; $optionsByDistance = [];
$inputThreshold = mb_strlen($input) / 2; $inputThreshold = mb_strlen($input) / 2;
foreach ($options as $option) { foreach ($options as $option) {
$distance = levenshtein($input, $option); $distance = $input === $option
? 0
: (strtolower($input) === strtolower($option)
? 1
: levenshtein($input, $option));
$threshold = max($inputThreshold, mb_strlen($option) / 2, 1); $threshold = max($inputThreshold, mb_strlen($option) / 2, 1);
if ($distance <= $threshold) { if ($distance <= $threshold) {
$optionsByDistance[$option] = $distance; $optionsByDistance[$option] = $distance;

View File

@ -2,6 +2,7 @@
namespace GraphQL\Utils; namespace GraphQL\Utils;
use GraphQL\Error\Error; use GraphQL\Error\Error;
use GraphQL\Language\AST\Node;
use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\InputType; use GraphQL\Type\Definition\InputType;
@ -26,7 +27,7 @@ class Value
if ($value === null) { if ($value === null) {
return self::ofErrors([ return self::ofErrors([
self::coercionError( self::coercionError(
"Expected non-nullable type $type", "Expected non-nullable type $type not to be null",
$blameNode, $blameNode,
$path $path
), ),
@ -55,11 +56,23 @@ class Value
return self::ofValue($parseResult); return self::ofValue($parseResult);
} catch (\Exception $error) { } catch (\Exception $error) {
return self::ofErrors([ return self::ofErrors([
self::coercionError("Expected type {$type->name}", $blameNode, $path, $error), self::coercionError(
"Expected type {$type->name}",
$blameNode,
$path,
$error->getMessage(),
$error
),
]); ]);
} catch (\Throwable $error) { } catch (\Throwable $error) {
return self::ofErrors([ return self::ofErrors([
self::coercionError("Expected type {$type->name}", $blameNode, $path, $error), self::coercionError(
"Expected type {$type->name}",
$blameNode,
$path,
$error->getMessage(),
$error
),
]); ]);
} }
} }
@ -72,8 +85,21 @@ class Value
} }
} }
$suggestions = Utils::suggestionList(
Utils::printSafe($value),
array_map(function($enumValue) { return $enumValue->name; }, $type->getValues())
);
$didYouMean = $suggestions
? "did you mean " . Utils::orList($suggestions) . "?"
: null;
return self::ofErrors([ return self::ofErrors([
self::coercionError("Expected type {$type->name}", $blameNode, $path), self::coercionError(
"Expected type {$type->name}",
$blameNode,
$path,
$didYouMean
),
]); ]);
} }
@ -105,7 +131,11 @@ class Value
if ($type instanceof InputObjectType) { if ($type instanceof InputObjectType) {
if (!is_object($value) && !is_array($value) && !$value instanceof \Traversable) { if (!is_object($value) && !is_array($value) && !$value instanceof \Traversable) {
return self::ofErrors([ return self::ofErrors([
self::coercionError("Expected object type {$type->name}", $blameNode, $path), self::coercionError(
"Expected type {$type->name} to be an object",
$blameNode,
$path
),
]); ]);
} }
@ -146,12 +176,20 @@ class Value
// Ensure every provided field is defined. // Ensure every provided field is defined.
foreach ($value as $fieldName => $field) { foreach ($value as $fieldName => $field) {
if (!array_key_exists($fieldName, $fields)) { if (!array_key_exists($fieldName, $fields)) {
$suggestions = Utils::suggestionList(
$fieldName,
array_keys($fields)
);
$didYouMean = $suggestions
? "did you mean " . Utils::orList($suggestions) . "?"
: null;
$errors = self::add( $errors = self::add(
$errors, $errors,
self::coercionError( self::coercionError(
"Field \"{$fieldName}\" is not defined by type {$type->name}", "Field \"{$fieldName}\" is not defined by type {$type->name}",
$blameNode, $blameNode,
$path $path,
$didYouMean
) )
); );
} }
@ -183,18 +221,17 @@ class Value
* @param string $message * @param string $message
* @param Node $blameNode * @param Node $blameNode
* @param array|null $path * @param array|null $path
* @param string $subMessage
* @param \Exception|\Throwable|null $originalError * @param \Exception|\Throwable|null $originalError
* @return Error * @return Error
*/ */
private static function coercionError($message, $blameNode, array $path = null, $originalError = null) { private static function coercionError($message, $blameNode, array $path = null, $subMessage = null, $originalError = null) {
$pathStr = self::printPath($path); $pathStr = self::printPath($path);
// Return a GraphQLError instance // Return a GraphQLError instance
return new Error( return new Error(
$message . $message .
($pathStr ? ' at ' . $pathStr : '') . ($pathStr ? ' at ' . $pathStr : '') .
($originalError && $originalError->getMessage() ($subMessage ? '; ' . $subMessage : '.'),
? '; ' . $originalError->getMessage()
: '.'),
$blameNode, $blameNode,
null, null,
null, null,

View File

@ -45,9 +45,12 @@ class ValuesOfCorrectType extends AbstractValidationRule
"{$fieldTypeName} was not provided."; "{$fieldTypeName} was not provided.";
} }
static function unknownFieldMessage($typeName, $fieldName) static function unknownFieldMessage($typeName, $fieldName, $message = null)
{ {
return "Field \"{$fieldName}\" is not defined by type {$typeName}."; return (
"Field \"{$fieldName}\" is not defined by type {$typeName}" .
($message ? "; {$message}" : '.')
);
} }
public function getVisitor(ValidationContext $context) public function getVisitor(ValidationContext $context)
@ -103,10 +106,18 @@ class ValuesOfCorrectType extends AbstractValidationRule
NodeKind::OBJECT_FIELD => function(ObjectFieldNode $node) use ($context) { NodeKind::OBJECT_FIELD => function(ObjectFieldNode $node) use ($context) {
$parentType = Type::getNamedType($context->getParentInputType()); $parentType = Type::getNamedType($context->getParentInputType());
$fieldType = $context->getInputType(); $fieldType = $context->getInputType();
if (!$fieldType && $parentType) { if (!$fieldType && $parentType instanceof InputObjectType) {
$suggestions = Utils::suggestionList(
$node->name->value,
array_keys($parentType->getFields())
);
$didYouMean = $suggestions
? "Did you mean " . Utils::orList($suggestions) . "?"
: null;
$context->reportError( $context->reportError(
new Error( new Error(
self::unknownFieldMessage($parentType->name, $node->name->value), self::unknownFieldMessage($parentType->name, $node->name->value, $didYouMean),
$node $node
) )
); );
@ -148,15 +159,12 @@ class ValuesOfCorrectType extends AbstractValidationRule
$type = Type::getNamedType($locationType); $type = Type::getNamedType($locationType);
if (!$type instanceof ScalarType) { if (!$type instanceof ScalarType) {
$suggestions = $type instanceof EnumType
? $this->enumTypeSuggestion($type, $node)
: null;
$context->reportError( $context->reportError(
new Error( new Error(
self::badValueMessage( self::badValueMessage(
(string) $locationType, (string) $locationType,
Printer::doPrint($node), Printer::doPrint($node),
$suggestions $this->enumTypeSuggestion($type, $node)
), ),
$node $node
) )
@ -214,13 +222,17 @@ class ValuesOfCorrectType extends AbstractValidationRule
} }
} }
private function enumTypeSuggestion(EnumType $type, ValueNode $node) private function enumTypeSuggestion($type, ValueNode $node)
{ {
$suggestions = Utils::suggestionList( if ($type instanceof EnumType) {
Printer::doPrint($node), $suggestions = Utils::suggestionList(
array_map(function (EnumValueDefinition $value) { return $value->name; }, $type->getValues()) Printer::doPrint($node),
); array_map(function (EnumValueDefinition $value) {
return $value->name;
}, $type->getValues())
);
return $suggestions ? 'Did you mean the enum value: ' . Utils::orList($suggestions) . '?' : ''; return $suggestions ? 'Did you mean the enum value ' . Utils::orList($suggestions) . '?' : null;
}
} }
} }

View File

@ -160,7 +160,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
'message' => 'message' =>
'Variable "$input" got invalid value ' . 'Variable "$input" got invalid value ' .
'{"a":"foo","b":"bar","c":null}; ' . '{"a":"foo","b":"bar","c":null}; ' .
'Expected non-nullable type String! at value.c.', 'Expected non-nullable type String! not to be null at value.c.',
'locations' => [['line' => 2, 'column' => 17]], 'locations' => [['line' => 2, 'column' => 17]],
'category' => 'graphql' 'category' => 'graphql'
] ]
@ -177,7 +177,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$input" got invalid value "foo bar"; ' . 'Variable "$input" got invalid value "foo bar"; ' .
'Expected object type TestInputObject.', 'Expected type TestInputObject to be an object.',
'locations' => [ [ 'line' => 2, 'column' => 17 ] ], 'locations' => [ [ 'line' => 2, 'column' => 17 ] ],
'category' => 'graphql', 'category' => 'graphql',
] ]
@ -411,7 +411,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$value" got invalid value null; ' . 'Variable "$value" got invalid value null; ' .
'Expected non-nullable type String!.', 'Expected non-nullable type String! not to be null.',
'locations' => [['line' => 2, 'column' => 31]], 'locations' => [['line' => 2, 'column' => 31]],
'category' => 'graphql', 'category' => 'graphql',
] ]
@ -613,7 +613,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$input" got invalid value null; ' . 'Variable "$input" got invalid value null; ' .
'Expected non-nullable type [String]!.', 'Expected non-nullable type [String]! not to be null.',
'locations' => [['line' => 2, 'column' => 17]], 'locations' => [['line' => 2, 'column' => 17]],
'category' => 'graphql', 'category' => 'graphql',
] ]
@ -701,7 +701,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$input" got invalid value ["A",null,"B"]; ' . 'Variable "$input" got invalid value ["A",null,"B"]; ' .
'Expected non-nullable type String! at value[1].', 'Expected non-nullable type String! not to be null at value[1].',
'locations' => [ ['line' => 2, 'column' => 17] ], 'locations' => [ ['line' => 2, 'column' => 17] ],
'category' => 'graphql', 'category' => 'graphql',
] ]
@ -727,7 +727,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$input" got invalid value null; ' . 'Variable "$input" got invalid value null; ' .
'Expected non-nullable type [String!]!.', 'Expected non-nullable type [String!]! not to be null.',
'locations' => [ ['line' => 2, 'column' => 17] ], 'locations' => [ ['line' => 2, 'column' => 17] ],
'category' => 'graphql', 'category' => 'graphql',
] ]
@ -768,7 +768,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase
[ [
'message' => 'message' =>
'Variable "$input" got invalid value ["A",null,"B"]; ' . 'Variable "$input" got invalid value ["A",null,"B"]; ' .
'Expected non-nullable type String! at value[1].', 'Expected non-nullable type String! not to be null at value[1].',
'locations' => [ ['line' => 2, 'column' => 17] ], 'locations' => [ ['line' => 2, 'column' => 17] ],
'category' => 'graphql', 'category' => 'graphql',
] ]

View File

@ -220,7 +220,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
'{ colorEnum(fromEnum: "GREEN") }', '{ colorEnum(fromEnum: "GREEN") }',
null, null,
[ [
'message' => "Expected type Color, found \"GREEN\"; Did you mean the enum value: GREEN?", 'message' => "Expected type Color, found \"GREEN\"; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)] 'locations' => [new SourceLocation(1, 23)]
] ]
); );
@ -235,7 +235,22 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase
'{ colorEnum(fromEnum: GREENISH) }', '{ colorEnum(fromEnum: GREENISH) }',
null, null,
[ [
'message' => "Expected type Color, found GREENISH; Did you mean the enum value: GREEN?", 'message' => "Expected type Color, found GREENISH; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)]
]
);
}
/**
* @it does not accept values with incorrect casing
*/
public function testDoesNotAcceptValuesWithIncorrectCasing()
{
$this->expectFailure(
'{ colorEnum(fromEnum: green) }',
null,
[
'message' => "Expected type Color, found green; Did you mean the enum value GREEN?",
'locations' => [new SourceLocation(1, 23)] 'locations' => [new SourceLocation(1, 23)]
] ]
); );

View File

@ -1,12 +1,38 @@
<?php <?php
namespace GraphQL\Tests\Utils; namespace GraphQL\Tests\Utils;
use GraphQL\Error\Error;
use GraphQL\Executor\Values; use GraphQL\Executor\Values;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\Type;
use GraphQL\Utils\Utils;
use GraphQL\Utils\Value; use GraphQL\Utils\Value;
class CoerceValueTest extends \PHPUnit_Framework_TestCase class CoerceValueTest extends \PHPUnit_Framework_TestCase
{ {
private $testEnum;
private $testInputObject;
public function setUp()
{
$this->testEnum = new EnumType([
'name' => 'TestEnum',
'values' => [
'FOO' => 'InternalFoo',
'BAR' => 123456789,
],
]);
$this->testInputObject = new InputObjectType([
'name' => 'TestInputObject',
'fields' => [
'foo' => Type::nonNull(Type::int()),
'bar' => Type::int(),
],
]);
}
// Describe: coerceValue // Describe: coerceValue
/** /**
@ -186,16 +212,125 @@ class CoerceValueTest extends \PHPUnit_Framework_TestCase
); );
} }
// DESCRIBE: for GraphQLEnum
/**
* @it returns no error for a known enum name
*/
public function testReturnsNoErrorForAKnownEnumName()
{
$fooResult = Value::coerceValue('FOO', $this->testEnum);
$this->expectNoErrors($fooResult);
$this->assertEquals('InternalFoo', $fooResult['value']);
$barResult = Value::coerceValue('BAR', $this->testEnum);
$this->expectNoErrors($barResult);
$this->assertEquals(123456789, $barResult['value']);
}
/**
* @it results error for misspelled enum value
*/
public function testReturnsErrorForMisspelledEnumValue()
{
$result = Value::coerceValue('foo', $this->testEnum);
$this->expectError($result, 'Expected type TestEnum; did you mean FOO?');
}
/**
* @it results error for incorrect value type
*/
public function testReturnsErrorForIncorrectValueType()
{
$result1 = Value::coerceValue(123, $this->testEnum);
$this->expectError($result1, 'Expected type TestEnum.');
$result2 = Value::coerceValue(['field' => 'value'], $this->testEnum);
$this->expectError($result2, 'Expected type TestEnum.');
}
// DESCRIBE: for GraphQLInputObject
/**
* @it returns no error for a valid input
*/
public function testReturnsNoErrorForValidInput()
{
$result = Value::coerceValue(['foo' => 123], $this->testInputObject);
$this->expectNoErrors($result);
$this->assertEquals(['foo' => 123], $result['value']);
}
/**
* @it returns no error for a non-object type
*/
public function testReturnsErrorForNonObjectType()
{
$result = Value::coerceValue(123, $this->testInputObject);
$this->expectError($result, 'Expected type TestInputObject to be an object.');
}
/**
* @it returns no error for an invalid field
*/
public function testReturnErrorForAnInvalidField()
{
$result = Value::coerceValue(['foo' => 'abc'], $this->testInputObject);
$this->expectError($result, 'Expected type Int at value.foo; Int cannot represent non 32-bit signed integer value: abc');
}
/**
* @it returns multiple errors for multiple invalid fields
*/
public function testReturnsMultipleErrorsForMultipleInvalidFields()
{
$result = Value::coerceValue(['foo' => 'abc', 'bar' => 'def'], $this->testInputObject);
$this->assertEquals([
'Expected type Int at value.foo; Int cannot represent non 32-bit signed integer value: abc',
'Expected type Int at value.bar; Int cannot represent non 32-bit signed integer value: def',
], $result['errors']);
}
/**
* @it returns error for a missing required field
*/
public function testReturnsErrorForAMissingRequiredField()
{
$result = Value::coerceValue(['bar' => 123], $this->testInputObject);
$this->expectError($result, 'Field value.foo of required type Int! was not provided.');
}
/**
* @it returns error for an unknown field
*/
public function testReturnsErrorForAnUnknownField()
{
$result = Value::coerceValue(['foo' => 123, 'unknownField' => 123], $this->testInputObject);
$this->expectError($result, 'Field "unknownField" is not defined by type TestInputObject.');
}
/**
* @it returns error for a misspelled field
*/
public function testReturnsErrorForAMisspelledField()
{
$result = Value::coerceValue(['foo' => 123, 'bart' => 123], $this->testInputObject);
$this->expectError($result, 'Field "bart" is not defined by type TestInputObject; did you mean bar?');
}
private function expectNoErrors($result) private function expectNoErrors($result)
{ {
$this->assertInternalType('array', $result); $this->assertInternalType('array', $result);
$this->assertNull($result['errors']); $this->assertNull($result['errors']);
$this->assertNotEquals(Utils::undefined(), $result['value']);
} }
private function expectError($result, $expected) { private function expectError($result, $expected) {
$this->assertInternalType('array', $result); $this->assertInternalType('array', $result);
$this->assertInternalType('array', $result['errors']); $this->assertInternalType('array', $result['errors']);
$this->assertCount(1, $result['errors']); $this->assertCount(1, $result['errors']);
$this->assertEquals($expected, $result['errors'][0]->getMessage()); $this->assertEquals($expected, $result['errors'][0]->getMessage());
$this->assertEquals(Utils::undefined(), $result['value']);
} }
} }

View File

@ -30,11 +30,12 @@ class ValuesOfCorrectTypeTest extends TestCase
); );
} }
private function unknownField($typeName, $fieldName, $line, $column) { private function unknownField($typeName, $fieldName, $line, $column, $message = null) {
return FormattedError::create( return FormattedError::create(
ValuesOfCorrectType::unknownFieldMessage( ValuesOfCorrectType::unknownFieldMessage(
$typeName, $typeName,
$fieldName $fieldName,
$message
), ),
[new SourceLocation($line, $column)] [new SourceLocation($line, $column)]
); );
@ -581,7 +582,7 @@ class ValuesOfCorrectTypeTest extends TestCase
'"SIT"', '"SIT"',
4, 4,
41, 41,
'Did you mean the enum value: SIT?' 'Did you mean the enum value SIT?'
) )
]); ]);
} }
@ -630,7 +631,13 @@ class ValuesOfCorrectTypeTest extends TestCase
} }
} }
', [ ', [
$this->badValue('DogCommand', 'sit', 4, 41) $this->badValue(
'DogCommand',
'sit',
4,
41,
'Did you mean the enum value SIT?'
)
]); ]);
} }
@ -1070,7 +1077,13 @@ class ValuesOfCorrectTypeTest extends TestCase
} }
} }
', [ ', [
$this->unknownField('ComplexInput', 'unknownField', 6, 15), $this->unknownField(
'ComplexInput',
'unknownField',
6,
15,
'Did you mean intField or booleanField?'
),
]); ]);
} }