From 7be4d3e75cd65588b4458c3660c9a6d1c5491f1f Mon Sep 17 00:00:00 2001 From: Hafiz Ismail Date: Wed, 1 Jun 2016 23:16:47 +0800 Subject: [PATCH] Include possible field, argument, type names when validation fails (#355) * Add suggestionList to return strings based on how simular they are to the input * Suggests valid fields in `FieldsOnCorrectType` * Suggest argument names * Suggested valid type names * Fix flow and unit test * addressed comments in PR: move file, update comment, filter out more options, remove redundant warning * fix typos * fix lint Commit: 5bc1b2541d1b5767de4016e10ae77021f81310fc [5bc1b25] Parents: 4b08c36e86 Author: Yuzhi Date: 27 April 2016 at 2:48:45 AM SGT Committer: Lee Byron --- rules.go | 188 +++++++++++++++++++++++++-- rules_fields_on_correct_type_test.go | 32 +++-- rules_known_type_names_test.go | 2 +- suggested_list_internal_test.go | 28 ++++ testutil/rules_test_harness.go | 4 - validator_test.go | 8 +- 6 files changed, 229 insertions(+), 33 deletions(-) create mode 100644 suggested_list_internal_test.go diff --git a/rules.go b/rules.go index c71184b6..b17c9ced 100644 --- a/rules.go +++ b/rules.go @@ -7,6 +7,7 @@ import ( "github.com/graphql-go/graphql/language/kinds" "github.com/graphql-go/graphql/language/printer" "github.com/graphql-go/graphql/language/visitor" + "math" "sort" "strings" ) @@ -162,8 +163,14 @@ func DefaultValuesOfCorrectTypeRule(context *ValidationContext) *ValidationRuleI VisitorOpts: visitorOpts, } } - -func UndefinedFieldMessage(fieldName string, ttypeName string, suggestedTypes []string) string { +func quoteStrings(slice []string) []string { + quoted := []string{} + for _, s := range slice { + quoted = append(quoted, fmt.Sprintf(`"%v"`, s)) + } + return quoted +} +func UndefinedFieldMessage(fieldName string, ttypeName string, suggestedTypes []string, suggestedFields []string) string { quoteStrings := func(slice []string) []string { quoted := []string{} @@ -175,15 +182,27 @@ func UndefinedFieldMessage(fieldName string, ttypeName string, suggestedTypes [] // construct helpful (but long) message message := fmt.Sprintf(`Cannot query field "%v" on type "%v".`, fieldName, ttypeName) - suggestions := strings.Join(quoteStrings(suggestedTypes), ", ") const MaxLength = 5 if len(suggestedTypes) > 0 { + suggestions := "" if len(suggestedTypes) > MaxLength { suggestions = strings.Join(quoteStrings(suggestedTypes[0:MaxLength]), ", ") + fmt.Sprintf(`, and %v other types`, len(suggestedTypes)-MaxLength) + } else { + suggestions = strings.Join(quoteStrings(suggestedTypes), ", ") + } + message = fmt.Sprintf(`%v However, this field exists on %v. `+ + `Perhaps you meant to use an inline fragment?`, message, suggestions) + } + if len(suggestedFields) > 0 { + suggestions := "" + if len(suggestedFields) > MaxLength { + suggestions = strings.Join(quoteStrings(suggestedFields[0:MaxLength]), ", ") + + fmt.Sprintf(`, or %v other field`, len(suggestedFields)-MaxLength) + } else { + suggestions = strings.Join(quoteStrings(suggestedFields), ", ") } - message = message + fmt.Sprintf(` However, this field exists on %v.`, suggestions) - message = message + ` Perhaps you meant to use an inline fragment?` + message = fmt.Sprintf(`%v Did you mean to query %v?`, message, suggestions) } return message @@ -232,11 +251,29 @@ func FieldsOnCorrectTypeRule(context *ValidationContext) *ValidationRuleInstance } } - message := UndefinedFieldMessage(nodeName, ttype.Name(), suggestedTypes) + suggestedFieldNames := []string{} + suggestedFields := []string{} + switch ttype := ttype.(type) { + case *Object: + for name := range ttype.Fields() { + suggestedFieldNames = append(suggestedFieldNames, name) + } + suggestedFields = suggestionList(nodeName, suggestedFieldNames) + case *Interface: + for name := range ttype.Fields() { + suggestedFieldNames = append(suggestedFieldNames, name) + } + suggestedFields = suggestionList(nodeName, suggestedFieldNames) + case *InputObject: + for name := range ttype.Fields() { + suggestedFieldNames = append(suggestedFieldNames, name) + } + suggestedFields = suggestionList(nodeName, suggestedFieldNames) + } reportError( context, - message, + UndefinedFieldMessage(nodeName, ttype.Name(), suggestedTypes, suggestedFields), []ast.Node{node}, ) } @@ -380,6 +417,28 @@ func FragmentsOnCompositeTypesRule(context *ValidationContext) *ValidationRuleIn } } +func unknownArgMessage(argName string, fieldName string, parentTypeName string, suggestedArgs []string) string { + message := fmt.Sprintf(`Unknown argument "%v" on field "%v" of type "%v".`, argName, fieldName, parentTypeName) + + if len(suggestedArgs) > 0 { + suggestions := strings.Join(quoteStrings(suggestedArgs), ", ") + message = fmt.Sprintf(`%v Perhaps you meant %v?`, message, suggestions) + } + + return message +} + +func unknownDirectiveArgMessage(argName string, directiveName string, suggestedArgs []string) string { + message := fmt.Sprintf(`Unknown argument "%v" on directive "@%v".`, argName, directiveName) + + if len(suggestedArgs) > 0 { + suggestions := strings.Join(quoteStrings(suggestedArgs), ", ") + message = fmt.Sprintf(`%v Perhaps you meant %v?`, message, suggestions) + } + + return message +} + // KnownArgumentNamesRule Known argument names // // A GraphQL field is only valid if all supplied arguments are defined by @@ -399,6 +458,7 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance if argumentOf == nil { return action, result } + var fieldArgDef *Argument if argumentOf.GetKind() == kinds.Field { fieldDef := context.FieldDef() if fieldDef == nil { @@ -408,8 +468,9 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance if node.Name != nil { nodeName = node.Name.Value } - var fieldArgDef *Argument + argNames := []string{} for _, arg := range fieldDef.Args { + argNames = append(argNames, arg.Name()) if arg.Name() == nodeName { fieldArgDef = arg } @@ -422,7 +483,7 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance } reportError( context, - fmt.Sprintf(`Unknown argument "%v" on field "%v" of type "%v".`, nodeName, fieldDef.Name, parentTypeName), + unknownArgMessage(nodeName, fieldDef.Name, parentTypeName, suggestionList(nodeName, argNames)), []ast.Node{node}, ) } @@ -435,8 +496,10 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance if node.Name != nil { nodeName = node.Name.Value } + argNames := []string{} var directiveArgDef *Argument for _, arg := range directive.Args { + argNames = append(argNames, arg.Name()) if arg.Name() == nodeName { directiveArgDef = arg } @@ -444,7 +507,7 @@ func KnownArgumentNamesRule(context *ValidationContext) *ValidationRuleInstance if directiveArgDef == nil { reportError( context, - fmt.Sprintf(`Unknown argument "%v" on directive "@%v".`, nodeName, directive.Name), + unknownDirectiveArgMessage(nodeName, directive.Name, suggestionList(nodeName, argNames)), []ast.Node{node}, ) } @@ -606,6 +669,23 @@ func KnownFragmentNamesRule(context *ValidationContext) *ValidationRuleInstance } } +func unknownTypeMessage(typeName string, suggestedTypes []string) string { + message := fmt.Sprintf(`Unknown type "%v".`, typeName) + + const MaxLength = 5 + if len(suggestedTypes) > 0 { + suggestions := "" + if len(suggestedTypes) < MaxLength { + suggestions = strings.Join(quoteStrings(suggestedTypes), ", ") + } else { + suggestions = strings.Join(quoteStrings(suggestedTypes[0:MaxLength]), ", ") + } + message = fmt.Sprintf(`%v Perhaps you meant one of the following: %v?`, message, suggestions) + } + + return message +} + // KnownTypeNamesRule Known type names // // A GraphQL document is only valid if referenced types (specifically @@ -643,9 +723,13 @@ func KnownTypeNamesRule(context *ValidationContext) *ValidationRuleInstance { } ttype := context.Schema().Type(typeNameValue) if ttype == nil { + suggestedTypes := []string{} + for key := range context.Schema().TypeMap() { + suggestedTypes = append(suggestedTypes, key) + } reportError( context, - fmt.Sprintf(`Unknown type "%v".`, typeNameValue), + unknownTypeMessage(typeNameValue, suggestionList(typeNameValue, suggestedTypes)), []ast.Node{node}, ) } @@ -2210,3 +2294,85 @@ func isValidLiteralValue(ttype Input, valueAST ast.Value) (bool, []string) { return true, nil } + +// Internal struct to sort results from suggestionList() +type suggestionListResult struct { + Options []string + Distances []float64 +} + +func (s suggestionListResult) Len() int { + return len(s.Options) +} +func (s suggestionListResult) Swap(i, j int) { + s.Options[i], s.Options[j] = s.Options[j], s.Options[i] +} +func (s suggestionListResult) Less(i, j int) bool { + return s.Distances[i] < s.Distances[j] +} + +// suggestionList 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. +func suggestionList(input string, options []string) []string { + dists := []float64{} + filteredOpts := []string{} + inputThreshold := float64(len(input) / 2) + + for _, opt := range options { + dist := lexicalDistance(input, opt) + threshold := math.Max(inputThreshold, float64(len(opt)/2)) + threshold = math.Max(threshold, 1) + if dist <= threshold { + filteredOpts = append(filteredOpts, opt) + dists = append(dists, dist) + } + } + //sort results + suggested := suggestionListResult{filteredOpts, dists} + sort.Sort(suggested) + return suggested.Options +} + +// lexicalDistance Computes the lexical distance between strings A and B. +// The "distance" between two strings is given by counting the minimum number +// of edits needed to transform string A into string B. An edit can be an +// insertion, deletion, or substitution of a single character, or a swap of two +// adjacent characters. +// This distance can be useful for detecting typos in input or sorting +func lexicalDistance(a, b string) float64 { + d := [][]float64{} + aLen := len(a) + bLen := len(b) + for i := 0; i <= aLen; i++ { + d = append(d, []float64{float64(i)}) + } + for k := 1; k <= bLen; k++ { + d[0] = append(d[0], float64(k)) + } + + for i := 1; i <= aLen; i++ { + for k := 1; k <= bLen; k++ { + cost := 1.0 + if a[i-1] == b[k-1] { + cost = 0.0 + } + minCostFloat := math.Min( + d[i-1][k]+1.0, + d[i][k-1]+1.0, + ) + minCostFloat = math.Min( + minCostFloat, + d[i-1][k-1]+cost, + ) + d[i] = append(d[i], minCostFloat) + + if i > 1 && k < 1 && + a[i-1] == b[k-2] && + a[i-2] == b[k-1] { + d[i][k] = math.Min(d[i][k], d[i-2][k-2]+cost) + } + } + } + + return d[aLen][bLen] +} diff --git a/rules_fields_on_correct_type_test.go b/rules_fields_on_correct_type_test.go index 294a0682..e9d00b6a 100644 --- a/rules_fields_on_correct_type_test.go +++ b/rules_fields_on_correct_type_test.go @@ -73,7 +73,7 @@ func TestValidate_FieldsOnCorrectType_FieldNotDefinedOnFragment(t *testing.T) { meowVolume } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Cannot query field "meowVolume" on type "Dog".`, 3, 9), + testutil.RuleError(`Cannot query field "meowVolume" on type "Dog". Did you mean to query "barkVolume"?`, 3, 9), }) } func TestValidate_FieldsOnCorrectType_IgnoreDeeplyUnknownField(t *testing.T) { @@ -106,7 +106,7 @@ func TestValidate_FieldsOnCorrectType_FieldNotDefinedOnInlineFragment(t *testing } } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Cannot query field "meowVolume" on type "Dog".`, 4, 11), + testutil.RuleError(`Cannot query field "meowVolume" on type "Dog". Did you mean to query "barkVolume"?`, 4, 11), }) } func TestValidate_FieldsOnCorrectType_AliasedFieldTargetNotDefined(t *testing.T) { @@ -115,7 +115,7 @@ func TestValidate_FieldsOnCorrectType_AliasedFieldTargetNotDefined(t *testing.T) volume : mooVolume } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Cannot query field "mooVolume" on type "Dog".`, 3, 9), + testutil.RuleError(`Cannot query field "mooVolume" on type "Dog". Did you mean to query "barkVolume"?`, 3, 9), }) } func TestValidate_FieldsOnCorrectType_AliasedLyingFieldTargetNotDefined(t *testing.T) { @@ -124,7 +124,7 @@ func TestValidate_FieldsOnCorrectType_AliasedLyingFieldTargetNotDefined(t *testi barkVolume : kawVolume } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Cannot query field "kawVolume" on type "Dog".`, 3, 9), + testutil.RuleError(`Cannot query field "kawVolume" on type "Dog". Did you mean to query "barkVolume"?`, 3, 9), }) } func TestValidate_FieldsOnCorrectType_NotDefinedOnInterface(t *testing.T) { @@ -142,7 +142,7 @@ func TestValidate_FieldsOnCorrectType_DefinedOnImplementorsButNotOnInterface(t * nickname } `, []gqlerrors.FormattedError{ - testutil.RuleError(`Cannot query field "nickname" on type "Pet". However, this field exists on "Cat", "Dog". Perhaps you meant to use an inline fragment?`, 3, 9), + testutil.RuleError(`Cannot query field "nickname" on type "Pet". However, this field exists on "Cat", "Dog". Perhaps you meant to use an inline fragment? Did you mean to query "name"?`, 3, 9), }) } func TestValidate_FieldsOnCorrectType_MetaFieldSelectionOnUnion(t *testing.T) { @@ -184,27 +184,33 @@ func TestValidate_FieldsOnCorrectType_ValidFieldInInlineFragment(t *testing.T) { } func TestValidate_FieldsOnCorrectTypeErrorMessage_WorksWithNoSuggestions(t *testing.T) { - message := graphql.UndefinedFieldMessage("T", "f", []string{}) - expected := `Cannot query field "T" on type "f".` + message := graphql.UndefinedFieldMessage("f", "T", []string{}, []string{}) + expected := `Cannot query field "f" on type "T".` if message != expected { t.Fatalf("Unexpected message, expected: %v, got %v", expected, message) } } func TestValidate_FieldsOnCorrectTypeErrorMessage_WorksWithNoSmallNumbersOfSuggestions(t *testing.T) { - message := graphql.UndefinedFieldMessage("T", "f", []string{"A", "B"}) - expected := `Cannot query field "T" on type "f". ` + + message := graphql.UndefinedFieldMessage("f", "T", []string{"A", "B"}, []string{"z", "y"}) + expected := `Cannot query field "f" on type "T". ` + `However, this field exists on "A", "B". ` + - `Perhaps you meant to use an inline fragment?` + `Perhaps you meant to use an inline fragment? ` + + `Did you mean to query "z", "y"?` if message != expected { t.Fatalf("Unexpected message, expected: %v, got %v", expected, message) } } func TestValidate_FieldsOnCorrectTypeErrorMessage_WorksWithLotsOfSuggestions(t *testing.T) { - message := graphql.UndefinedFieldMessage("T", "f", []string{"A", "B", "C", "D", "E", "F"}) - expected := `Cannot query field "T" on type "f". ` + + message := graphql.UndefinedFieldMessage( + "f", "T", + []string{"A", "B", "C", "D", "E", "F"}, + []string{"z", "y", "x", "w", "v", "u"}, + ) + expected := `Cannot query field "f" on type "T". ` + `However, this field exists on "A", "B", "C", "D", "E", and 1 other types. ` + - `Perhaps you meant to use an inline fragment?` + `Perhaps you meant to use an inline fragment? ` + + `Did you mean to query "z", "y", "x", "w", "v", or 1 other field?` if message != expected { t.Fatalf("Unexpected message, expected: %v, got %v", expected, message) } diff --git a/rules_known_type_names_test.go b/rules_known_type_names_test.go index eec9a0ae..e984f932 100644 --- a/rules_known_type_names_test.go +++ b/rules_known_type_names_test.go @@ -34,7 +34,7 @@ func TestValidate_KnownTypeNames_UnknownTypeNamesAreInValid(t *testing.T) { `, []gqlerrors.FormattedError{ testutil.RuleError(`Unknown type "JumbledUpLetters".`, 2, 23), testutil.RuleError(`Unknown type "Badger".`, 5, 25), - testutil.RuleError(`Unknown type "Peettt".`, 8, 29), + testutil.RuleError(`Unknown type "Peettt". Perhaps you meant one of the following: "Pet"?`, 8, 29), }) } diff --git a/suggested_list_internal_test.go b/suggested_list_internal_test.go new file mode 100644 index 00000000..52e04ae3 --- /dev/null +++ b/suggested_list_internal_test.go @@ -0,0 +1,28 @@ +package graphql + +import ( + "reflect" + "testing" +) + +func TestSuggestionList_ReturnsResultsWhenInputIsEmpty(t *testing.T) { + expected := []string{"a"} + result := suggestionList("", []string{"a"}) + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Expected %v, got: %v", expected, result) + } +} +func TestSuggestionList_ReturnsEmptyArrayWhenThereAreNoOptions(t *testing.T) { + expected := []string{} + result := suggestionList("input", []string{}) + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Expected %v, got: %v", expected, result) + } +} +func TestSuggestionList_ReturnsOptionsSortedBasedOnSimilarity(t *testing.T) { + expected := []string{"abc", "ab"} + result := suggestionList("abc", []string{"a", "ab", "abc"}) + if !reflect.DeepEqual(expected, result) { + t.Fatalf("Expected %v, got: %v", expected, result) + } +} diff --git a/testutil/rules_test_harness.go b/testutil/rules_test_harness.go index 809a1eff..bb4166b5 100644 --- a/testutil/rules_test_harness.go +++ b/testutil/rules_test_harness.go @@ -182,10 +182,6 @@ func init() { dogType, catType, }, - ResolveType: func(p graphql.ResolveTypeParams) *graphql.Object { - // not used for validation - return nil - }, }) var intelligentInterface = graphql.NewInterface(graphql.InterfaceConfig{ Name: "Intelligent", diff --git a/validator_test.go b/validator_test.go index 67b7a3dd..2600e310 100644 --- a/validator_test.go +++ b/validator_test.go @@ -1,6 +1,7 @@ package graphql_test import ( + "reflect" "testing" "github.com/graphql-go/graphql" @@ -10,7 +11,6 @@ import ( "github.com/graphql-go/graphql/language/parser" "github.com/graphql-go/graphql/language/source" "github.com/graphql-go/graphql/testutil" - "reflect" ) func expectValid(t *testing.T, schema *graphql.Schema, queryString string) { @@ -74,19 +74,19 @@ func TestValidator_SupportsFullValidation_ValidatesUsingACustomTypeInfo(t *testi expectedErrors := []gqlerrors.FormattedError{ { - Message: "Cannot query field \"catOrDog\" on type \"QueryRoot\".", + Message: `Cannot query field "catOrDog" on type "QueryRoot". Did you mean to query "catOrDog"?`, Locations: []location.SourceLocation{ {Line: 3, Column: 9}, }, }, { - Message: "Cannot query field \"furColor\" on type \"Cat\".", + Message: `Cannot query field "furColor" on type "Cat". Did you mean to query "furColor"?`, Locations: []location.SourceLocation{ {Line: 5, Column: 13}, }, }, { - Message: "Cannot query field \"isHousetrained\" on type \"Dog\".", + Message: `Cannot query field "isHousetrained" on type "Dog". Did you mean to query "isHousetrained"?`, Locations: []location.SourceLocation{ {Line: 8, Column: 13}, },