Add a 'constant.ref' check (#44)

This check reports an error if a referenced constant or enum value
cannot be found in the current, or any included, file.

To support this, I've generalized Resolve() to handle any named
reference and introduced a ResolveConstant() function to resolve
ast.ConstantReference values.

README.md

@@ -71,6 +71,11 @@ You can enable or disable checks using the configuration file's top-level
 from the full list first, and then the resulting list is filtered by the list
 of `enabled` checks. Either list can be empty (the default).
 
+### `constant.ref`
+
+This check reports an error if a referenced constant or enum value cannot be
+found in either the current scope or in an included file (using dot notation).
+
 ### `enum.size`
 
 This check warns or errors if an enumeration's element size grows beyond a

ast.go

@@ -48,14 +48,13 @@ func Doc(node ast.Node) string {
 	return ""
 }
 
-// Resolve resolves an ast.TypeReference to its target node.
+// Resolve resolves a named reference to its target node.
 //
 // The target can either be in the current program's scope or it can refer to
 // an included file using dot notation. Included files must exist in one of the
 // given search directories.
-func Resolve(ref ast.TypeReference, program *ast.Program, dirs []string) (ast.Node, error) {
+func Resolve(name string, program *ast.Program, dirs []string) (ast.Node, error) {
 	defs := program.Definitions
-	name := ref.Name
 
 	if strings.Contains(name, ".") {
 		parts := strings.SplitN(name, ".", 2)
@@ -71,7 +70,7 @@ func Resolve(ref ast.TypeReference, program *ast.Program, dirs []string) (ast.No
 			}
 		}
 		if ipath == "" {
-			return nil, fmt.Errorf("missing \"include\" for type reference %q", ref.Name)
+			return nil, fmt.Errorf("missing \"include\" for type reference %q", name)
 		}
 
 		program, _, err := ParseFile(ipath, dirs)
@@ -89,15 +88,45 @@ func Resolve(ref ast.TypeReference, program *ast.Program, dirs []string) (ast.No
 		}
 	}
 
-	return nil, fmt.Errorf("%q could not be resolved", ref.Name)
+	return nil, fmt.Errorf("%q could not be resolved", name)
+}
+
+// ResolveConstant resolves an ast.ConstantReference to its target node.
+//
+// The following name formats are supported:
+// 	- "Constant" (ast.Constant)
+// 	- "Enum.Value" (ast.EnumItem)
+// 	- "include.Constant" (ast.Constant)
+// 	- "include.Enum.Value" (ast.EnumItem)
+func ResolveConstant(ref ast.ConstantReference, program *ast.Program, dirs []string) (ast.Node, error) {
+	parts := strings.SplitN(ref.Name, ".", 3)
+
+	n, err := Resolve(parts[0], program, dirs)
+	if err != nil && len(parts) > 1 {
+		n, err = Resolve(parts[0]+"."+parts[1], program, dirs)
+	}
+	if err != nil {
+		return n, err
+	}
+
+	if e, ok := n.(*ast.Enum); ok {
+		for _, ei := range e.Items {
+			if ei.Name == parts[len(parts)-1] {
+				return ei, nil
+			}
+		}
+		return nil, fmt.Errorf("enum value %q could not be resolved", ref.Name)
+	}
+
+	return n, err
 }
 
 // ResolveType calls Resolve and goes one step further by attempting to
 // resolve the target node's own type. This is useful when the reference
 // points to an ast.Typedef or ast.Constant, for example, and the caller
 // is primarily intererested in the target's ast.Type.
 func ResolveType(ref ast.TypeReference, program *ast.Program, dirs []string) (ast.Node, error) {
-	n, err := Resolve(ref, program, dirs)
+	n, err := Resolve(ref.Name, program, dirs)
 	if err != nil {
 		return nil, err
 	}

ast_test.go

@@ -15,6 +15,7 @@
 package thriftcheck
 
 import (
+	"reflect"
 	"testing"
 
 	"go.uber.org/thriftrw/ast"
@@ -37,3 +38,52 @@ func TestDoc(t *testing.T) {
 		}
 	}
 }
+
+func TestResolveConstant(t *testing.T) {
+	tests := []struct {
+		ref  ast.ConstantReference
+		prog *ast.Program
+		want reflect.Type
+		err  bool
+	}{
+		{
+			ast.ConstantReference{Name: "Constant"},
+			&ast.Program{Definitions: []ast.Definition{
+				&ast.Constant{Name: "Constant"},
+			}},
+			reflect.TypeOf((*ast.Constant)(nil)),
+			false,
+		},
+		{
+			ast.ConstantReference{Name: "Enum.Value"}, &ast.Program{Definitions: []ast.Definition{
+				&ast.Enum{
+					Name: "Enum",
+					Items: []*ast.EnumItem{
+						{Name: "Value"},
+					},
+				},
+			}}, reflect.TypeOf((*ast.EnumItem)(nil)),
+			false,
+		},
+		{
+			ast.ConstantReference{Name: "Unknown"},
+			&ast.Program{},
+			nil,
+			true,
+		},
+	}
+
+	for _, tt := range tests {
+		n, err := ResolveConstant(tt.ref, tt.prog, nil)
+		if tt.err {
+			if err == nil {
+				t.Errorf("expected an error, got %s", n)
+			}
+		} else if err != nil {
+			t.Errorf("unexpected error: %s", err)
+		}
+		if got := reflect.TypeOf(n); tt.want != nil && got != tt.want {
+			t.Errorf("expected %v but got %v", tt.want, got)
+		}
+	}
+}

check.go

@@ -202,9 +202,17 @@ func (c *C) Errorf(node ast.Node, message string, args ...interface{}) {
 	c.Messages = append(c.Messages, m)
 }
 
-// Resolve resolves a type reference.
-func (c *C) Resolve(ref ast.TypeReference) ast.Node {
-	if n, err := Resolve(ref, c.Program, c.Dirs); err == nil {
+// Resolve resolves a name.
+func (c *C) Resolve(name string) ast.Node {
+	if n, err := Resolve(name, c.Program, c.Dirs); err == nil {
+		return n
+	}
+	return nil
+}
+
+// ResolveConstant resolves a constant reference to its target.
+func (c *C) ResolveConstant(ref ast.ConstantReference) ast.Node {
+	if n, err := ResolveConstant(ref, c.Program, c.Dirs); err == nil {
 		return n
 	}
 	return nil

checks/constants.go

@@ -0,0 +1,30 @@
+// Copyright 2022 Pinterest
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package checks
+
+import (
+	"github.com/pinterest/thriftcheck"
+	"go.uber.org/thriftrw/ast"
+)
+
+// CheckConstantRef returns a thriftcheck.Check that ensures that a constant
+// reference's target can be resolved.
+func CheckConstantRef() *thriftcheck.Check {
+	return thriftcheck.NewCheck("constant.ref", func(c *thriftcheck.C, ref ast.ConstantReference) {
+		if c.ResolveConstant(ref) == nil {
+			c.Errorf(ref, "unable to find a constant or enum value named %q", ref.Name)
+		}
+	})
+}

checks/constants_test.go

@@ -0,0 +1,56 @@
+// Copyright 2022 Pinterest
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package checks_test
+
+import (
+	"testing"
+
+	"github.com/pinterest/thriftcheck/checks"
+	"go.uber.org/thriftrw/ast"
+)
+
+func TestCheckConstantRef(t *testing.T) {
+	tests := []Test{
+		{
+			prog: &ast.Program{Definitions: []ast.Definition{
+				&ast.Constant{Name: "Constant"},
+			}},
+			node: ast.ConstantReference{Name: "Constant"},
+			want: []string{},
+		},
+		{
+			prog: &ast.Program{Definitions: []ast.Definition{
+				&ast.Enum{
+					Name: "Enum",
+					Items: []*ast.EnumItem{
+						{Name: "Value"},
+					},
+				},
+			}},
+			node: ast.ConstantReference{Name: "Enum.Value"},
+			want: []string{},
+		},
+		{
+			prog: &ast.Program{},
+			node: ast.ConstantReference{Name: "Unknown"},
+			want: []string{
+				`t.thrift:0:1: error: unable to find a constant or enum value named "Unknown" (constant.ref)`,
+			},
+		},
+	}
+
+	check := checks.CheckConstantRef()
+	RunTests(t, check, tests)
+}

cmd/main.go

@@ -151,6 +151,7 @@ func main() {
 
 	// Build the set of checks we'll use for the linter
 	allChecks := thriftcheck.Checks{
+		checks.CheckConstantRef(),
 		checks.CheckEnumSize(cfg.Checks.Enum.Size.Warning, cfg.Checks.Enum.Size.Error),
 		checks.CheckFieldIDMissing(),
 		checks.CheckFieldIDNegative(),