Skip to content

Commit

Permalink
Track validation error positions
Browse files Browse the repository at this point in the history
  • Loading branch information
Adam Scarr committed Jul 19, 2018
1 parent 7b1ade7 commit 9bfd81b
Show file tree
Hide file tree
Showing 33 changed files with 197 additions and 54 deletions.
2 changes: 0 additions & 2 deletions ast/definition.go
Expand Up @@ -74,8 +74,6 @@ type EnumValueDefinition struct {
Position *Position `dump:"-"`
}

// Directive Definitions

type DirectiveDefinition struct {
Description string
Name string
Expand Down
30 changes: 15 additions & 15 deletions ast/document_test.go
Expand Up @@ -31,24 +31,24 @@ func TestQueryDocMethods(t *testing.T) {
}

func TestNamedTypeCompatability(t *testing.T) {
assert.True(t, NamedType("A").IsCompatible(NamedType("A")))
assert.False(t, NamedType("A").IsCompatible(NamedType("B")))
assert.True(t, NamedType("A", nil).IsCompatible(NamedType("A", nil)))
assert.False(t, NamedType("A", nil).IsCompatible(NamedType("B", nil)))

assert.True(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("A"))))
assert.False(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("B"))))
assert.False(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("B"))))
assert.True(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("A", nil), nil)))
assert.False(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("B", nil), nil)))
assert.False(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("B", nil), nil)))

assert.True(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("A"))))
assert.False(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("B"))))
assert.False(t, ListType(NamedType("A")).IsCompatible(ListType(NamedType("B"))))
assert.True(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("A", nil), nil)))
assert.False(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("B", nil), nil)))
assert.False(t, ListType(NamedType("A", nil), nil).IsCompatible(ListType(NamedType("B", nil), nil)))

assert.True(t, NonNullNamedType("A").IsCompatible(NamedType("A")))
assert.False(t, NamedType("A").IsCompatible(NonNullNamedType("A")))
assert.True(t, NonNullNamedType("A", nil).IsCompatible(NamedType("A", nil)))
assert.False(t, NamedType("A", nil).IsCompatible(NonNullNamedType("A", nil)))

assert.True(t, NonNullListType(NamedType("String")).IsCompatible(NonNullListType(NamedType("String"))))
assert.True(t, NonNullListType(NamedType("String")).IsCompatible(ListType(NamedType("String"))))
assert.False(t, ListType(NamedType("String")).IsCompatible(NonNullListType(NamedType("String"))))
assert.True(t, NonNullListType(NamedType("String", nil), nil).IsCompatible(NonNullListType(NamedType("String", nil), nil)))
assert.True(t, NonNullListType(NamedType("String", nil), nil).IsCompatible(ListType(NamedType("String", nil), nil)))
assert.False(t, ListType(NamedType("String", nil), nil).IsCompatible(NonNullListType(NamedType("String", nil), nil)))

assert.True(t, ListType(NonNullNamedType("String")).IsCompatible(ListType(NamedType("String"))))
assert.False(t, ListType(NamedType("String")).IsCompatible(ListType(NonNullNamedType("String"))))
assert.True(t, ListType(NonNullNamedType("String", nil), nil).IsCompatible(ListType(NamedType("String", nil), nil)))
assert.False(t, ListType(NamedType("String", nil), nil).IsCompatible(ListType(NonNullNamedType("String", nil), nil)))
}
5 changes: 5 additions & 0 deletions ast/selection.go
Expand Up @@ -4,12 +4,17 @@ type SelectionSet []Selection

type Selection interface {
isSelection()
GetPosition() *Position
}

func (*Field) isSelection() {}
func (*FragmentSpread) isSelection() {}
func (*InlineFragment) isSelection() {}

func (s *Field) GetPosition() *Position { return s.Position }
func (s *FragmentSpread) GetPosition() *Position { return s.Position }
func (s *InlineFragment) GetPosition() *Position { return s.Position }

type Field struct {
Alias string
Name string
Expand Down
11 changes: 11 additions & 0 deletions gqlerror/error.go
Expand Up @@ -15,6 +15,17 @@ type Error struct {
Rule string `json:"-"`
}

func (err *Error) SetFile(file string) {
if file == "" {
return
}
if err.Extensions == nil {
err.Extensions = map[string]interface{}{}
}

err.Extensions["file"] = file
}

type Location struct {
Line int `json:"line,omitempty"`
Column int `json:"column,omitempty"`
Expand Down
6 changes: 4 additions & 2 deletions parser/query.go
Expand Up @@ -257,20 +257,22 @@ func (p *parser) parseValueLiteral(isConst bool) *Value {

func (p *parser) parseList(isConst bool) *Value {
var values ChildValueList
pos := p.peekPos()
p.many(lexer.BracketL, lexer.BracketR, func() {
values = append(values, &ChildValue{Value: p.parseValueLiteral(isConst)})
})

return &Value{Children: values, Kind: ListValue}
return &Value{Children: values, Kind: ListValue, Position: pos}
}

func (p *parser) parseObject(isConst bool) *Value {
var fields ChildValueList
pos := p.peekPos()
p.many(lexer.BraceL, lexer.BraceR, func() {
fields = append(fields, p.parseObjectField(isConst))
})

return &Value{Children: fields, Kind: ObjectValue}
return &Value{Children: fields, Kind: ObjectValue, Position: pos}
}

func (p *parser) parseObjectField(isConst bool) *ChildValue {
Expand Down
16 changes: 16 additions & 0 deletions validator/error.go
Expand Up @@ -3,6 +3,7 @@ package validator
import (
"fmt"

"github.com/vektah/gqlparser/ast"
"github.com/vektah/gqlparser/gqlerror"
)

Expand All @@ -14,6 +15,21 @@ func Message(msg string, args ...interface{}) ErrorOption {
}
}

func At(position *ast.Position) ErrorOption {
return func(err *gqlerror.Error) {
if position == nil {
return
}
err.Locations = append(err.Locations, gqlerror.Location{
Line: position.Line,
Column: position.Column,
})
if position.Src.Name != "" {
err.SetFile(position.Src.Name)
}
}
}

func SuggestListQuoted(prefix string, typed string, suggestions []string) ErrorOption {
suggested := SuggestionList(typed, suggestions)
return func(err *gqlerror.Error) {
Expand Down
5 changes: 4 additions & 1 deletion validator/rules/fields_on_correct_type.go
Expand Up @@ -23,7 +23,10 @@ func init() {
message += " Did you mean " + QuotedOrList(suggestedFieldNames...) + "?"
}

addError(Message(message))
addError(
Message(message),
At(field.Position),
)
})
})
}
Expand Down
10 changes: 8 additions & 2 deletions validator/rules/fragments_on_composite_types.go
Expand Up @@ -17,7 +17,10 @@ func init() {

message := fmt.Sprintf(`Fragment cannot condition on non composite type "%s".`, inlineFragment.TypeCondition)

addError(Message(message))
addError(
Message(message),
At(inlineFragment.Position),
)
})

observers.OnFragment(func(walker *Walker, fragment *ast.FragmentDefinition) {
Expand All @@ -27,7 +30,10 @@ func init() {

message := fmt.Sprintf(`Fragment "%s" cannot condition on non composite type "%s".`, fragment.Name, fragment.TypeCondition)

addError(Message(message))
addError(
Message(message),
At(fragment.Position),
)
})
})
}
2 changes: 2 additions & 0 deletions validator/rules/known_argument_names.go
Expand Up @@ -26,6 +26,7 @@ func init() {
addError(
Message(`Unknown argument "%s" on field "%s" of type "%s".`, arg.Name, field.Name, field.ObjectDefinition.Name),
SuggestListQuoted("Did you mean", arg.Name, suggestions),
At(field.Position),
)
}
})
Expand All @@ -48,6 +49,7 @@ func init() {
addError(
Message(`Unknown argument "%s" on directive "@%s".`, arg.Name, directive.Name),
SuggestListQuoted("Did you mean", arg.Name, suggestions),
At(directive.Position),
)
}
})
Expand Down
2 changes: 2 additions & 0 deletions validator/rules/known_directives.go
Expand Up @@ -11,6 +11,7 @@ func init() {
if directive.Definition == nil {
addError(
Message(`Unknown directive "%s".`, directive.Name),
At(directive.Position),
)
return
}
Expand All @@ -23,6 +24,7 @@ func init() {

addError(
Message(`Directive "%s" may not be used on %s.`, directive.Name, directive.Location),
At(directive.Position),
)
})
})
Expand Down
5 changes: 4 additions & 1 deletion validator/rules/known_fragment_names.go
Expand Up @@ -9,7 +9,10 @@ func init() {
AddRule("KnownFragmentNames", func(observers *Events, addError AddErrFunc) {
observers.OnFragmentSpread(func(walker *Walker, fragmentSpread *ast.FragmentSpread) {
if fragmentSpread.Definition == nil {
addError(Message(`Unknown fragment "%s".`, fragmentSpread.Name))
addError(
Message(`Unknown fragment "%s".`, fragmentSpread.Name),
At(fragmentSpread.Position),
)
}
})
})
Expand Down
7 changes: 4 additions & 3 deletions validator/rules/known_type_names.go
Expand Up @@ -17,6 +17,7 @@ func init() {

addError(
Message(`Unknown type "%s".`, typeName),
At(operation.Position),
)
}
})
Expand All @@ -34,6 +35,7 @@ func init() {

addError(
Message(`Unknown type "%s".`, typedName),
At(inlineFragment.Position),
)
})

Expand All @@ -49,11 +51,10 @@ func init() {
possibleTypes = append(possibleTypes, t.Name)
}

list := SuggestListQuoted("Did you mean", typeName, possibleTypes)

addError(
Message(`Unknown type "%s".`, typeName),
list,
SuggestListQuoted("Did you mean", typeName, possibleTypes),
At(fragment.Position),
)
})
})
Expand Down
5 changes: 4 additions & 1 deletion validator/rules/lone_anonymous_operation.go
Expand Up @@ -9,7 +9,10 @@ func init() {
AddRule("LoneAnonymousOperation", func(observers *Events, addError AddErrFunc) {
observers.OnOperation(func(walker *Walker, operation *ast.OperationDefinition) {
if operation.Name == "" && len(walker.Document.Operations) > 1 {
addError(Message(`This anonymous operation must be the only defined operation.`))
addError(
Message(`This anonymous operation must be the only defined operation.`),
At(operation.Position),
)
}
})
})
Expand Down
5 changes: 4 additions & 1 deletion validator/rules/no_fragment_cycles.go
Expand Up @@ -51,7 +51,10 @@ func init() {
if len(fragmentNames) != 0 {
via = fmt.Sprintf(" via %s", strings.Join(fragmentNames, ", "))
}
addError(Message(`Cannot spread fragment "%s" within itself%s.`, spreadName, via))
addError(
Message(`Cannot spread fragment "%s" within itself%s.`, spreadName, via),
At(spreadNode.Position),
)
}

spreadPath = spreadPath[:len(spreadPath)-1]
Expand Down
10 changes: 8 additions & 2 deletions validator/rules/no_undefined_variables.go
Expand Up @@ -13,9 +13,15 @@ func init() {
}

if walker.CurrentOperation.Name != "" {
addError(Message(`Variable "$%s" is not defined by operation "%s".`, value, walker.CurrentOperation.Name))
addError(
Message(`Variable "$%s" is not defined by operation "%s".`, value, walker.CurrentOperation.Name),
At(walker.CurrentOperation.Position),
)
} else {
addError(Message(`Variable "$%s" is not defined.`, value))
addError(
Message(`Variable "$%s" is not defined.`, value),
At(value.Position),
)
}
})
})
Expand Down
5 changes: 4 additions & 1 deletion validator/rules/no_unused_fragments.go
Expand Up @@ -20,7 +20,10 @@ func init() {
observers.OnFragment(func(walker *Walker, fragment *ast.FragmentDefinition) {
inFragmentDefinition = true
if !fragmentNameUsed[fragment.Name] {
addError(Message(`Fragment "%s" is never used.`, fragment.Name))
addError(
Message(`Fragment "%s" is never used.`, fragment.Name),
At(fragment.Position),
)
}
})
})
Expand Down
10 changes: 8 additions & 2 deletions validator/rules/no_unused_variables.go
Expand Up @@ -14,9 +14,15 @@ func init() {
}

if operation.Name != "" {
addError(Message(`Variable "$%s" is never used in operation "%s".`, varDef.Variable, operation.Name))
addError(
Message(`Variable "$%s" is never used in operation "%s".`, varDef.Variable, operation.Name),
At(varDef.Position),
)
} else {
addError(Message(`Variable "$%s" is never used.`, varDef.Variable))
addError(
Message(`Variable "$%s" is never used.`, varDef.Variable),
At(varDef.Position),
)
}
}
})
Expand Down
10 changes: 9 additions & 1 deletion validator/rules/overlapping_fields_can_be_merged.go
Expand Up @@ -198,6 +198,7 @@ type ConflictMessage struct {
ResponseName string
Names []string
SubMessage []*ConflictMessage
Position *ast.Position
}

func (m *ConflictMessage) String(buf *bytes.Buffer) {
Expand All @@ -220,7 +221,10 @@ func (m *ConflictMessage) String(buf *bytes.Buffer) {
func (m *ConflictMessage) addFieldsConflictMessage(addError AddErrFunc) {
var buf bytes.Buffer
m.String(&buf)
addError(Message(`Fields "%s" conflict because %s. Use different aliases on the fields to fetch both if this was intentional.`, m.ResponseName, buf.String()))
addError(
Message(`Fields "%s" conflict because %s. Use different aliases on the fields to fetch both if this was intentional.`, m.ResponseName, buf.String()),
At(m.Position),
)
}

type overlappingFieldsCanBeMergedManager struct {
Expand Down Expand Up @@ -429,6 +433,7 @@ func (m *overlappingFieldsCanBeMergedManager) findConflict(parentFieldsAreMutual
return &ConflictMessage{
ResponseName: fieldNameA,
Message: fmt.Sprintf(`%s and %s are different fields`, fieldA.Name, fieldB.Name),
Position: fieldB.Position,
}
}

Expand All @@ -437,6 +442,7 @@ func (m *overlappingFieldsCanBeMergedManager) findConflict(parentFieldsAreMutual
return &ConflictMessage{
ResponseName: fieldNameA,
Message: "they have differing arguments",
Position: fieldB.Position,
}
}
}
Expand All @@ -445,6 +451,7 @@ func (m *overlappingFieldsCanBeMergedManager) findConflict(parentFieldsAreMutual
return &ConflictMessage{
ResponseName: fieldNameA,
Message: fmt.Sprintf(`they return conflicting types %s and %s`, fieldA.Definition.Type.String(), fieldB.Definition.Type.String()),
Position: fieldB.Position,
}
}

Expand All @@ -458,6 +465,7 @@ func (m *overlappingFieldsCanBeMergedManager) findConflict(parentFieldsAreMutual
return &ConflictMessage{
ResponseName: fieldNameA,
SubMessage: conflicts.Conflicts,
Position: fieldB.Position,
}
}

Expand Down
10 changes: 8 additions & 2 deletions validator/rules/possible_fragment_spreads.go
Expand Up @@ -46,7 +46,10 @@ func init() {

observers.OnInlineFragment(func(walker *Walker, inlineFragment *ast.InlineFragment) {
validate(walker, inlineFragment.ObjectDefinition, inlineFragment.TypeCondition, func() {
addError(Message(`Fragment cannot be spread here as objects of type "%s" can never be of type "%s".`, inlineFragment.ObjectDefinition.Name, inlineFragment.TypeCondition))
addError(
Message(`Fragment cannot be spread here as objects of type "%s" can never be of type "%s".`, inlineFragment.ObjectDefinition.Name, inlineFragment.TypeCondition),
At(inlineFragment.Position),
)
})
})

Expand All @@ -55,7 +58,10 @@ func init() {
return
}
validate(walker, fragmentSpread.ObjectDefinition, fragmentSpread.Definition.TypeCondition, func() {
addError(Message(`Fragment "%s" cannot be spread here as objects of type "%s" can never be of type "%s".`, fragmentSpread.Name, fragmentSpread.ObjectDefinition.Name, fragmentSpread.Definition.TypeCondition))
addError(
Message(`Fragment "%s" cannot be spread here as objects of type "%s" can never be of type "%s".`, fragmentSpread.Name, fragmentSpread.ObjectDefinition.Name, fragmentSpread.Definition.TypeCondition),
At(fragmentSpread.Position),
)
})
})
})
Expand Down

0 comments on commit 9bfd81b

Please sign in to comment.