Skip to content

Commit

Permalink
Add a 'field.id.zero' check (#49)
Browse files Browse the repository at this point in the history
This check reports an error if a field's ID is zero, which is generally
unsupported by the Apache Thrift compiler. This is distinct from the
`field.id.negative` check given the existence of the `--allow-neg-keys`
Apache Thrift compiler option.
  • Loading branch information
jparise authored Apr 28, 2022
1 parent e950ef2 commit 80432a2
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 0 deletions.
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ implicit/auto-assigning syntax).

This check reports an error if a field's ID is negative.

### `field.id.zero`

This check reports an error if a field's ID is zero, which is generally
unsupported by the Apache Thrift compiler. This is distinct from the
`field.id.negative` check given the existence of the `--allow-neg-keys`
Apache Thrift compiler option.

### `field.optional`

This check warns if a field isn't declared as "optional", which is considered
Expand Down
9 changes: 9 additions & 0 deletions checks/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ func CheckFieldIDNegative() *thriftcheck.Check {
})
}

// CheckFieldIDZero reports an error if a field's ID is zero.
func CheckFieldIDZero() *thriftcheck.Check {
return thriftcheck.NewCheck("field.id.zero", func(c *thriftcheck.C, f *ast.Field) {
if f.ID == 0 {
c.Errorf(f, "field ID for %q is zero", f.Name)
}
})
}

// CheckFieldOptional warns if a field isn't declared as "optional".
func CheckFieldOptional() *thriftcheck.Check {
return thriftcheck.NewCheck("field.optional", func(c *thriftcheck.C, f *ast.Field) {
Expand Down
22 changes: 22 additions & 0 deletions checks/fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,28 @@ func TestCheckFieldIDNegative(t *testing.T) {
RunTests(t, check, tests)
}

func TestCheckFieldIDZero(t *testing.T) {
tests := []Test{
{
node: &ast.Field{ID: 1, Name: "Field"},
want: []string{},
},
{
node: &ast.Field{ID: 0, Name: "Field"},
want: []string{
`t.thrift:0:1: error: field ID for "Field" is zero (field.id.zero)`,
},
},
{
node: &ast.Field{ID: -1, Name: "Field"},
want: []string{},
},
}

check := checks.CheckFieldIDZero()
RunTests(t, check, tests)
}

func TestCheckFieldOptional(t *testing.T) {
tests := []Test{
{
Expand Down
1 change: 1 addition & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ func main() {
checks.CheckEnumSize(cfg.Checks.Enum.Size.Warning, cfg.Checks.Enum.Size.Error),
checks.CheckFieldIDMissing(),
checks.CheckFieldIDNegative(),
checks.CheckFieldIDZero(),
checks.CheckFieldOptional(),
checks.CheckFieldRequiredness(),
checks.CheckIncludePath(),
Expand Down

0 comments on commit 80432a2

Please sign in to comment.