Begin support for warnings and linting in schema

Fixes #8

pkg/development/warningdefs.go

@@ -0,0 +1,184 @@
+package development
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	corev1 "github.com/authzed/spicedb/pkg/proto/core/v1"
+	devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1"
+	"github.com/authzed/spicedb/pkg/tuple"
+	"github.com/authzed/spicedb/pkg/typesystem"
+)
+
+var lintRelationReferencesParentType = func(
+	ctx context.Context,
+	relation *corev1.Relation,
+	ts *typesystem.TypeSystem,
+) (*devinterface.DeveloperWarning, error) {
+	parentDef := ts.Namespace()
+	if strings.HasSuffix(relation.Name, parentDef.Name) {
+		if ts.IsPermission(relation.Name) {
+			return warningForMetadata(
+				fmt.Sprintf("Permission %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
+				relation,
+			), nil
+		}
+
+		return warningForMetadata(
+			fmt.Sprintf("Relation %q references parent type %q in its name; it is recommended to drop the suffix", relation.Name, parentDef.Name),
+			relation,
+		), nil
+	}
+
+	return nil, nil
+}
+
+var lintPermissionReferencingItself = func(
+	ctx context.Context,
+	computedUserset *corev1.ComputedUserset,
+	ts *typesystem.TypeSystem,
+) (*devinterface.DeveloperWarning, error) {
+	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
+	permName := parentRelation.Name
+	if computedUserset.Relation == permName {
+		return warningForMetadata(
+			fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
+			parentRelation,
+		), nil
+	}
+
+	return nil, nil
+}
+
+var lintArrowReferencingUnreachable = func(
+	ctx context.Context,
+	ttu *corev1.TupleToUserset,
+	ts *typesystem.TypeSystem,
+) (*devinterface.DeveloperWarning, error) {
+	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
+
+	referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
+	if !ok {
+		return nil, nil
+	}
+
+	allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	wasFound := false
+	for _, subjectType := range allowedSubjectTypes {
+		nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
+		if err != nil {
+			return nil, err
+		}
+
+		_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
+		if ok {
+			wasFound = true
+		}
+	}
+
+	if !wasFound {
+		return warningForMetadata(
+			fmt.Sprintf(
+				"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
+				ttu.Tupleset.Relation,
+				ttu.ComputedUserset.Relation,
+				parentRelation.Name,
+				ttu.ComputedUserset.Relation,
+				ttu.Tupleset.Relation,
+			),
+			parentRelation,
+		), nil
+	}
+
+	return nil, nil
+}
+
+var lintArrowOverSubRelation = func(
+	ctx context.Context,
+	ttu *corev1.TupleToUserset,
+	ts *typesystem.TypeSystem,
+) (*devinterface.DeveloperWarning, error) {
+	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
+
+	referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
+	if !ok {
+		return nil, nil
+	}
+
+	allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, subjectType := range allowedSubjectTypes {
+		if subjectType.Relation != tuple.Ellipsis {
+			return warningForMetadata(
+				fmt.Sprintf(
+					"Arrow `%s->%s` under permission %q references relation %q that has relation %q on subject %q: *the subject relation will be ignored for the arrow*",
+					ttu.Tupleset.Relation,
+					ttu.ComputedUserset.Relation,
+					parentRelation.Name,
+					ttu.Tupleset.Relation,
+					subjectType.Relation,
+					subjectType.Namespace,
+				),
+				parentRelation,
+			), nil
+		}
+	}
+
+	return nil, nil
+}
+
+var lintArrowReferencingRelation = func(
+	ctx context.Context,
+	ttu *corev1.TupleToUserset,
+	ts *typesystem.TypeSystem,
+) (*devinterface.DeveloperWarning, error) {
+	parentRelation := ctx.Value(relationKey).(*corev1.Relation)
+
+	referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
+	if !ok {
+		return nil, nil
+	}
+
+	// For each subject type of the referenced relation, check if the referenced permission
+	// is, in fact, a relation.
+	allowedSubjectTypes, err := ts.AllowedSubjectRelations(referencedRelation.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, subjectType := range allowedSubjectTypes {
+		nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
+		if err != nil {
+			return nil, err
+		}
+
+		targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
+		if !ok {
+			continue
+		}
+
+		if !nts.IsPermission(targetRelation.Name) {
+			return warningForMetadata(
+				fmt.Sprintf(
+					"Arrow `%s->%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
+					ttu.Tupleset.Relation,
+					ttu.ComputedUserset.Relation,
+					parentRelation.Name,
+					targetRelation.Name,
+					subjectType.Namespace,
+				),
+				parentRelation,
+			), nil
+		}
+	}
+
+	return nil, nil
+}

pkg/development/warnings.go

@@ -0,0 +1,177 @@
+package development
+
+import (
+	"context"
+
+	"github.com/authzed/spicedb/pkg/namespace"
+	corev1 "github.com/authzed/spicedb/pkg/proto/core/v1"
+	devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1"
+	"github.com/authzed/spicedb/pkg/spiceerrors"
+	"github.com/authzed/spicedb/pkg/typesystem"
+)
+
+var allChecks = checkers{
+	relationCheckers: []relationChecker{
+		lintRelationReferencesParentType,
+	},
+	computedUsersetCheckers: []computedUsersetChecker{
+		lintPermissionReferencingItself,
+	},
+	ttuCheckers: []ttuChecker{
+		lintArrowReferencingRelation,
+		lintArrowReferencingUnreachable,
+		lintArrowOverSubRelation,
+	},
+}
+
+func warningForMetadata(message string, metadata namespace.WithSourcePosition) *devinterface.DeveloperWarning {
+	if metadata.GetSourcePosition() == nil {
+		return &devinterface.DeveloperWarning{
+			Message: message,
+		}
+	}
+
+	lineNumber := metadata.GetSourcePosition().ZeroIndexedLineNumber + 1
+	columnNumber := metadata.GetSourcePosition().ZeroIndexedColumnPosition + 1
+
+	return &devinterface.DeveloperWarning{
+		Message: message,
+		Line:    uint32(lineNumber),
+		Column:  uint32(columnNumber),
+	}
+}
+
+// GetWarnings returns a list of warnings for the given developer context.
+func GetWarnings(ctx context.Context, devCtx *DevContext) ([]*devinterface.DeveloperWarning, error) {
+	warnings := []*devinterface.DeveloperWarning{}
+	resolver := typesystem.ResolverForSchema(*devCtx.CompiledSchema)
+
+	for _, def := range devCtx.CompiledSchema.ObjectDefinitions {
+		found, err := addDefinitionWarnings(ctx, def, resolver)
+		if err != nil {
+			return nil, err
+		}
+		warnings = append(warnings, found...)
+	}
+
+	return warnings, nil
+}
+
+type contextKey string
+
+var relationKey = contextKey("relation")
+
+func addDefinitionWarnings(ctx context.Context, def *corev1.NamespaceDefinition, resolver typesystem.Resolver) ([]*devinterface.DeveloperWarning, error) {
+	ts, err := typesystem.NewNamespaceTypeSystem(def, resolver)
+	if err != nil {
+		return nil, err
+	}
+
+	warnings := []*devinterface.DeveloperWarning{}
+	for _, rel := range def.Relation {
+		ctx = context.WithValue(ctx, relationKey, rel)
+		for _, checker := range allChecks.relationCheckers {
+			checkerWarning, err := checker(ctx, rel, ts)
+			if err != nil {
+				return nil, err
+			}
+
+			if checkerWarning != nil {
+				warnings = append(warnings, checkerWarning)
+			}
+		}
+
+		if ts.IsPermission(rel.Name) {
+			found, err := walkUsersetRewrite(ctx, rel.UsersetRewrite, allChecks, ts)
+			if err != nil {
+				return nil, err
+			}
+
+			warnings = append(warnings, found...)
+		}
+	}
+
+	return warnings, nil
+}
+
+type (
+	relationChecker        func(ctx context.Context, relation *corev1.Relation, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
+	computedUsersetChecker func(ctx context.Context, computedUserset *corev1.ComputedUserset, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
+	ttuChecker             func(ctx context.Context, ttu *corev1.TupleToUserset, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
+)
+
+type checkers struct {
+	relationCheckers        []relationChecker
+	computedUsersetCheckers []computedUsersetChecker
+	ttuCheckers             []ttuChecker
+}
+
+func walkUsersetRewrite(ctx context.Context, rewrite *corev1.UsersetRewrite, checkers checkers, ts *typesystem.TypeSystem) ([]*devinterface.DeveloperWarning, error) {
+	if rewrite == nil {
+		return nil, nil
+	}
+
+	switch t := (rewrite.RewriteOperation).(type) {
+	case *corev1.UsersetRewrite_Union:
+		return walkUsersetOperations(ctx, t.Union.Child, checkers, ts)
+
+	case *corev1.UsersetRewrite_Intersection:
+		return walkUsersetOperations(ctx, t.Intersection.Child, checkers, ts)
+
+	case *corev1.UsersetRewrite_Exclusion:
+		return walkUsersetOperations(ctx, t.Exclusion.Child, checkers, ts)
+
+	default:
+		return nil, spiceerrors.MustBugf("unexpected rewrite operation type %T", t)
+	}
+}
+
+func walkUsersetOperations(ctx context.Context, ops []*corev1.SetOperation_Child, checkers checkers, ts *typesystem.TypeSystem) ([]*devinterface.DeveloperWarning, error) {
+	warnings := []*devinterface.DeveloperWarning{}
+	for _, op := range ops {
+		switch t := op.ChildType.(type) {
+		case *corev1.SetOperation_Child_XThis:
+			continue
+
+		case *corev1.SetOperation_Child_ComputedUserset:
+			for _, checker := range checkers.computedUsersetCheckers {
+				checkerWarning, err := checker(ctx, t.ComputedUserset, ts)
+				if err != nil {
+					return nil, err
+				}
+
+				if checkerWarning != nil {
+					warnings = append(warnings, checkerWarning)
+				}
+			}
+
+		case *corev1.SetOperation_Child_UsersetRewrite:
+			found, err := walkUsersetRewrite(ctx, t.UsersetRewrite, checkers, ts)
+			if err != nil {
+				return nil, err
+			}
+
+			warnings = append(warnings, found...)
+
+		case *corev1.SetOperation_Child_TupleToUserset:
+			for _, checker := range checkers.ttuCheckers {
+				checkerWarning, err := checker(ctx, t.TupleToUserset, ts)
+				if err != nil {
+					return nil, err
+				}
+
+				if checkerWarning != nil {
+					warnings = append(warnings, checkerWarning)
+				}
+			}
+
+		case *corev1.SetOperation_Child_XNil:
+			continue
+
+		default:
+			return nil, spiceerrors.MustBugf("unexpected set operation type %T", t)
+		}
+	}
+
+	return warnings, nil
+}