Skip to content

Commit

Permalink
Merge pull request #260 from uber-go/bug1012
Browse files Browse the repository at this point in the history
Better error message for malformed entity tag
  • Loading branch information
phliar authored Dec 14, 2017
2 parents 42c9c5c + 483d6c9 commit 2f4bf60
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
24 changes: 24 additions & 0 deletions entity_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ func parsePartitionKey(pkStr string) []string {

// parsePrimaryKey func parses the primary key of DOSA object
func parsePrimaryKey(tableName, pkStr string) (*PrimaryKey, error) {
// parens must be matched
if !parensBalanced(pkStr) {
return nil, fmt.Errorf("unmatched parentheses: %q", pkStr)
}
// filter out "trailing comma and space"
pkStr = strings.TrimRight(pkStr, ", ")
pkStr = strings.TrimSpace(pkStr)
Expand Down Expand Up @@ -384,6 +388,26 @@ func parseField(typ Type, isPointer bool, name string, tag string) (*ColumnDefin
return &ColumnDefinition{Name: name, IsPointer: isPointer, Type: typ}, nil
}

func parensBalanced(s string) bool {
// This is effectively pushing left parens on the stack, and popping them when
// a right paren is seen. Since the stack only ever contains the same character,
// we don't actually need the stack -- only its size.
var ssize uint
for i := 0; i < len(s); i++ {
if s[i] == '(' {
ssize++
} else if s[i] == ')' {
if ssize == 0 {
// Extra right paren
return false
}
ssize--
}
}
// Stack must be empty
return ssize == 0
}

var (
uuidType = reflect.TypeOf(UUID(""))
blobType = reflect.TypeOf([]byte{})
Expand Down
29 changes: 29 additions & 0 deletions entity_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,35 @@ func TestInvalidFieldInTag(t *testing.T) {
assert.Contains(t, err.Error(), "invalid")
}

func TestInvalidSyntaxInTag(t *testing.T) {
type HasInvalidTagSyntax struct {
Entity `dosa:"primaryKey=((Val, Key), TS DESC"`
Val string
Key string
TS time.Time
}
table, err := TableFromInstance(&HasInvalidTagSyntax{})
assert.Nil(t, table)
assert.Error(t, err)
assert.Contains(t, err.Error(), "unmatched parentheses")
}

func TestParensBalanced(t *testing.T) {
assert.True(t, parensBalanced("()"))
assert.True(t, parensBalanced("()()"))
assert.True(t, parensBalanced("(()())"))
assert.True(t, parensBalanced("()"))
assert.True(t, parensBalanced(""))
assert.True(t, parensBalanced("()(()(()(()()))())()()"))

assert.False(t, parensBalanced("("))
assert.False(t, parensBalanced(")"))
assert.False(t, parensBalanced("(()"))
assert.False(t, parensBalanced(")("))
assert.False(t, parensBalanced("(()))"))
assert.False(t, parensBalanced("((()())"))
}

/*
These tests do not currently pass, but I think they should
*/
Expand Down
2 changes: 1 addition & 1 deletion finder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestParser(t *testing.T) {

assert.Equal(t, len(expectedEntities)+len(entitiesExcludedForTest), len(entities), fmt.Sprintf("%s", entities))
// TODO(jzhan): remove the hard-coded number of errors.
assert.Equal(t, 21, len(errs), fmt.Sprintf("%v", errs))
assert.Equal(t, 22, len(errs), fmt.Sprintf("%v", errs))
assert.Nil(t, err)

for _, entity := range entities {
Expand Down

0 comments on commit 2f4bf60

Please sign in to comment.