diff --git a/src/Utils/Utils.php b/src/Utils/Utils.php index 853fbf1..c9c3452 100644 --- a/src/Utils/Utils.php +++ b/src/Utils/Utils.php @@ -512,6 +512,9 @@ class Utils * 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. * + * 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 array $options * @return string[] @@ -521,7 +524,11 @@ class Utils $optionsByDistance = []; $inputThreshold = mb_strlen($input) / 2; 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); if ($distance <= $threshold) { $optionsByDistance[$option] = $distance; diff --git a/src/Utils/Value.php b/src/Utils/Value.php index 6d45c49..5606091 100644 --- a/src/Utils/Value.php +++ b/src/Utils/Value.php @@ -2,6 +2,7 @@ namespace GraphQL\Utils; use GraphQL\Error\Error; +use GraphQL\Language\AST\Node; use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\InputObjectType; use GraphQL\Type\Definition\InputType; @@ -26,7 +27,7 @@ class Value if ($value === null) { return self::ofErrors([ self::coercionError( - "Expected non-nullable type $type", + "Expected non-nullable type $type not to be null", $blameNode, $path ), @@ -55,11 +56,23 @@ class Value return self::ofValue($parseResult); } catch (\Exception $error) { 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) { 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([ - 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 (!is_object($value) && !is_array($value) && !$value instanceof \Traversable) { 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. foreach ($value as $fieldName => $field) { 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::coercionError( "Field \"{$fieldName}\" is not defined by type {$type->name}", $blameNode, - $path + $path, + $didYouMean ) ); } @@ -183,18 +221,17 @@ class Value * @param string $message * @param Node $blameNode * @param array|null $path + * @param string $subMessage * @param \Exception|\Throwable|null $originalError * @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); // Return a GraphQLError instance return new Error( $message . ($pathStr ? ' at ' . $pathStr : '') . - ($originalError && $originalError->getMessage() - ? '; ' . $originalError->getMessage() - : '.'), + ($subMessage ? '; ' . $subMessage : '.'), $blameNode, null, null, diff --git a/src/Validator/Rules/ValuesOfCorrectType.php b/src/Validator/Rules/ValuesOfCorrectType.php index a70de1f..c77c93b 100644 --- a/src/Validator/Rules/ValuesOfCorrectType.php +++ b/src/Validator/Rules/ValuesOfCorrectType.php @@ -45,9 +45,12 @@ class ValuesOfCorrectType extends AbstractValidationRule "{$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) @@ -103,10 +106,18 @@ class ValuesOfCorrectType extends AbstractValidationRule NodeKind::OBJECT_FIELD => function(ObjectFieldNode $node) use ($context) { $parentType = Type::getNamedType($context->getParentInputType()); $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( new Error( - self::unknownFieldMessage($parentType->name, $node->name->value), + self::unknownFieldMessage($parentType->name, $node->name->value, $didYouMean), $node ) ); @@ -148,15 +159,12 @@ class ValuesOfCorrectType extends AbstractValidationRule $type = Type::getNamedType($locationType); if (!$type instanceof ScalarType) { - $suggestions = $type instanceof EnumType - ? $this->enumTypeSuggestion($type, $node) - : null; $context->reportError( new Error( self::badValueMessage( (string) $locationType, Printer::doPrint($node), - $suggestions + $this->enumTypeSuggestion($type, $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( - Printer::doPrint($node), - array_map(function (EnumValueDefinition $value) { return $value->name; }, $type->getValues()) - ); + if ($type instanceof EnumType) { + $suggestions = Utils::suggestionList( + 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; + } } } diff --git a/tests/Executor/VariablesTest.php b/tests/Executor/VariablesTest.php index ccb16bb..89c931f 100644 --- a/tests/Executor/VariablesTest.php +++ b/tests/Executor/VariablesTest.php @@ -160,7 +160,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'message' => 'Variable "$input" got invalid value ' . '{"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]], 'category' => 'graphql' ] @@ -177,7 +177,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => 'Variable "$input" got invalid value "foo bar"; ' . - 'Expected object type TestInputObject.', + 'Expected type TestInputObject to be an object.', 'locations' => [ [ 'line' => 2, 'column' => 17 ] ], 'category' => 'graphql', ] @@ -411,7 +411,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => '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]], 'category' => 'graphql', ] @@ -613,7 +613,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => '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]], 'category' => 'graphql', ] @@ -701,7 +701,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => '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] ], 'category' => 'graphql', ] @@ -727,7 +727,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => '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] ], 'category' => 'graphql', ] @@ -768,7 +768,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase [ 'message' => '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] ], 'category' => 'graphql', ] diff --git a/tests/Type/EnumTypeTest.php b/tests/Type/EnumTypeTest.php index 9c6910d..79741d3 100644 --- a/tests/Type/EnumTypeTest.php +++ b/tests/Type/EnumTypeTest.php @@ -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' => "Expected type Color, found \"GREEN\"; Did you mean the enum value GREEN?", 'locations' => [new SourceLocation(1, 23)] ] ); @@ -235,7 +235,22 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase '{ colorEnum(fromEnum: GREENISH) }', 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)] ] ); diff --git a/tests/Utils/CoerceValueTest.php b/tests/Utils/CoerceValueTest.php index 999ca89..bb5823a 100644 --- a/tests/Utils/CoerceValueTest.php +++ b/tests/Utils/CoerceValueTest.php @@ -1,12 +1,38 @@ 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 /** @@ -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) { $this->assertInternalType('array', $result); $this->assertNull($result['errors']); + $this->assertNotEquals(Utils::undefined(), $result['value']); } + private function expectError($result, $expected) { $this->assertInternalType('array', $result); $this->assertInternalType('array', $result['errors']); $this->assertCount(1, $result['errors']); $this->assertEquals($expected, $result['errors'][0]->getMessage()); + $this->assertEquals(Utils::undefined(), $result['value']); } } diff --git a/tests/Validator/ValuesOfCorrectTypeTest.php b/tests/Validator/ValuesOfCorrectTypeTest.php index dd62ebd..322d984 100644 --- a/tests/Validator/ValuesOfCorrectTypeTest.php +++ b/tests/Validator/ValuesOfCorrectTypeTest.php @@ -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( ValuesOfCorrectType::unknownFieldMessage( $typeName, - $fieldName + $fieldName, + $message ), [new SourceLocation($line, $column)] ); @@ -581,7 +582,7 @@ class ValuesOfCorrectTypeTest extends TestCase '"SIT"', 4, 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?' + ), ]); }