diff --git a/.gitignore b/.gitignore index 148a4040d..af0d44fe0 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,7 @@ coverage*.txt .claude/ e2e/newenemy/spicedb e2e/newenemy/cockroach -e2e/newenemy/chaosd \ No newline at end of file +e2e/newenemy/chaosd + +# Auto Claude data directory +.auto-claude/ diff --git a/go.sum b/go.sum index a7bb9679d..20ec5bd75 100644 --- a/go.sum +++ b/go.sum @@ -2503,6 +2503,7 @@ google.golang.org/grpc v1.54.0/go.mod h1:PUSEXI6iWghWaB6lXM4knEgpJNu2qUcKfDtNci3 google.golang.org/grpc v1.56.3/go.mod h1:I9bI3vqKfayGqPUAwGdOSu7kt6oIJLixfffKrpXqQ9s= google.golang.org/grpc v1.77.0 h1:wVVY6/8cGA6vvffn+wWK5ToddbgdU3d8MNENr4evgXM= google.golang.org/grpc v1.77.0/go.mod h1:z0BY1iVj0q8E1uSQCjL9cppRj+gnZjzDnzV0dHhrNig= +google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0 h1:M1YKkFIboKNieVO5DLUEVzQfGwJD30Nv2jfUgzb5UcE= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= diff --git a/pkg/composableschemadsl/compiler/compiler_test.go b/pkg/composableschemadsl/compiler/compiler_test.go index 64429892c..d41468850 100644 --- a/pkg/composableschemadsl/compiler/compiler_test.go +++ b/pkg/composableschemadsl/compiler/compiler_test.go @@ -573,16 +573,19 @@ func TestCompile(t *testing.T) { "", []SchemaDefinition{ namespace.Namespace("sometenant/complex", - namespace.MustRelation("foos", - namespace.Exclusion( - namespace.Rewrite( - namespace.Union( - namespace.ComputedUserset("bars"), - namespace.ComputedUserset("bazs"), + namespace.MustWithMixedOperators( + namespace.MustRelation("foos", + namespace.Exclusion( + namespace.Rewrite( + namespace.Union( + namespace.ComputedUserset("bars"), + namespace.ComputedUserset("bazs"), + ), ), + namespace.ComputedUserset("mehs"), ), - namespace.ComputedUserset("mehs"), ), + 1, 22, // line 1, column 22 (0-indexed) ), ), }, diff --git a/pkg/composableschemadsl/compiler/translator.go b/pkg/composableschemadsl/compiler/translator.go index bb00ebb11..3b6047789 100644 --- a/pkg/composableschemadsl/compiler/translator.go +++ b/pkg/composableschemadsl/compiler/translator.go @@ -437,6 +437,9 @@ func translatePermission(tctx *translationContext, permissionNode *dslNode) (*co return nil, permissionNode.Errorf("invalid permission expression: %w", err) } + // Detect mixed operators without parentheses before translation (which may flatten the AST). + mixedOpsPosition := detectMixedOperatorsWithoutParens(tctx, expressionNode) + rewrite, err := translateExpression(tctx, expressionNode) if err != nil { return nil, err @@ -447,6 +450,14 @@ func translatePermission(tctx *translationContext, permissionNode *dslNode) (*co return nil, err } + // Store mixed operators flag in metadata + if mixedOpsPosition != nil { + err = namespace.SetMixedOperatorsWithoutParens(permission, true, mixedOpsPosition) + if err != nil { + return nil, permissionNode.Errorf("error adding mixed operators flag to metadata: %w", err) + } + } + if !tctx.skipValidate { if err := permission.Validate(); err != nil { return nil, permissionNode.Errorf("error in permission %s: %w", permissionName, err) @@ -579,6 +590,14 @@ func translateExpressionOperationDirect(tctx *translationContext, expressionOpNo case dslshape.NodeTypeSelfExpression: return namespace.Self(), nil + case dslshape.NodeTypeParenthesizedExpression: + // Unwrap the parenthesized expression and translate its inner expression. + innerExprNode, err := expressionOpNode.Lookup(dslshape.NodeParenthesizedExpressionPredicateInnerExpr) + if err != nil { + return nil, err + } + return translateExpressionOperation(tctx, innerExprNode) + case dslshape.NodeTypeArrowExpression: leftChild, err := expressionOpNode.Lookup(dslshape.NodeExpressionPredicateLeftExpr) if err != nil { @@ -948,3 +967,91 @@ func translateUseFlag(tctx *translationContext, useFlagNode *dslNode) error { tctx.enabledFlags.Add(flagName) return nil } + +// operatorType represents the type of set operator in an expression. +type operatorType int + +const ( + operatorTypeUnknown operatorType = iota + operatorTypeUnion + operatorTypeIntersection + operatorTypeExclusion +) + +// getOperatorType returns the operator type for a given node type, or operatorTypeUnknown if not a set operator. +func getOperatorType(nodeType dslshape.NodeType) operatorType { + switch nodeType { + case dslshape.NodeTypeUnionExpression: + return operatorTypeUnion + case dslshape.NodeTypeIntersectExpression: + return operatorTypeIntersection + case dslshape.NodeTypeExclusionExpression: + return operatorTypeExclusion + default: + return operatorTypeUnknown + } +} + +// detectMixedOperatorsWithoutParens walks the expression AST and detects if there are mixed +// operators (union, intersection, exclusion) at the same scope level without explicit parentheses. +// Returns the source position of the first mixed operator found, or nil if none. +// Parenthesized expressions act as boundaries - mixing inside parens does not trigger a warning +// since the user explicitly grouped the expression. However, top-level parentheses are unwrapped +// since they don't clarify internal operator precedence. +func detectMixedOperatorsWithoutParens(tctx *translationContext, node *dslNode) *core.SourcePosition { + // Unwrap top-level parenthesized expressions - they don't clarify internal precedence. + // e.g., (a + b - c) should still warn about mixed operators. + for node.GetType() == dslshape.NodeTypeParenthesizedExpression { + innerNode, err := node.Lookup(dslshape.NodeParenthesizedExpressionPredicateInnerExpr) + if err != nil { + break + } + node = innerNode + } + return detectMixedOperatorsInScope(tctx, node, operatorTypeUnknown) +} + +// detectMixedOperatorsInScope recursively checks for mixed operators within a scope. +// parentOp is the operator type seen so far at this scope level. +func detectMixedOperatorsInScope(tctx *translationContext, node *dslNode, parentOp operatorType) *core.SourcePosition { + nodeType := node.GetType() + + // Parenthesized expressions act as a boundary - don't propagate operator checking into them. + // The user explicitly grouped the expression, so we don't warn about mixing inside. + if nodeType == dslshape.NodeTypeParenthesizedExpression { + return nil + } + + currentOp := getOperatorType(nodeType) + + // If this is a set operator and we've seen a different operator at this scope, it's mixed. + if currentOp != operatorTypeUnknown && parentOp != operatorTypeUnknown && currentOp != parentOp { + return getSourcePosition(node, tctx.mapper) + } + + // If this is a set operator, check children with this operator as the scope's operator. + if currentOp != operatorTypeUnknown { + effectiveOp := currentOp + if parentOp != operatorTypeUnknown { + effectiveOp = parentOp // Keep the first operator seen at this scope + } + + // Check left child + leftChild, err := node.Lookup(dslshape.NodeExpressionPredicateLeftExpr) + if err == nil { + if pos := detectMixedOperatorsInScope(tctx, leftChild, effectiveOp); pos != nil { + return pos + } + } + + // Check right child + rightChild, err := node.Lookup(dslshape.NodeExpressionPredicateRightExpr) + if err == nil { + if pos := detectMixedOperatorsInScope(tctx, rightChild, effectiveOp); pos != nil { + return pos + } + } + } + + return nil +} diff --git a/pkg/composableschemadsl/dslshape/dslshape.go b/pkg/composableschemadsl/dslshape/dslshape.go index 3d1d63add..b6a007cc0 100644 --- a/pkg/composableschemadsl/dslshape/dslshape.go +++ b/pkg/composableschemadsl/dslshape/dslshape.go @@ -32,9 +32,10 @@ const ( NodeTypeArrowExpression // A TTU in arrow form. - NodeTypeIdentifier // An identifier under an expression. - NodeTypeNilExpression // A nil keyword - NodeTypeSelfExpression // A self keyword + NodeTypeIdentifier // An identifier under an expression. + NodeTypeNilExpression // A nil keyword + NodeTypeSelfExpression // A self keyword + NodeTypeParenthesizedExpression // A parenthesized expression wrapper. NodeTypeCaveatTypeReference // A type reference for a caveat parameter. @@ -212,6 +213,13 @@ const ( NodeExpressionPredicateLeftExpr = "left-expr" NodeExpressionPredicateRightExpr = "right-expr" + // + // NodeTypeParenthesizedExpression + // + + // The inner expression wrapped by parentheses. + NodeParenthesizedExpressionPredicateInnerExpr = "inner-expr" + // // NodeTypeImport // diff --git a/pkg/composableschemadsl/dslshape/zz_generated.nodetype_string.go b/pkg/composableschemadsl/dslshape/zz_generated.nodetype_string.go index 318cc0873..d477c9dc7 100644 --- a/pkg/composableschemadsl/dslshape/zz_generated.nodetype_string.go +++ b/pkg/composableschemadsl/dslshape/zz_generated.nodetype_string.go @@ -29,15 +29,16 @@ func _() { _ = x[NodeTypeIdentifier-18] _ = x[NodeTypeNilExpression-19] _ = x[NodeTypeSelfExpression-20] - _ = x[NodeTypeCaveatTypeReference-21] - _ = x[NodeTypeImport-22] - _ = x[NodeTypePartial-23] - _ = x[NodeTypePartialReference-24] + _ = x[NodeTypeParenthesizedExpression-21] + _ = x[NodeTypeCaveatTypeReference-22] + _ = x[NodeTypeImport-23] + _ = x[NodeTypePartial-24] + _ = x[NodeTypePartialReference-25] } -const _NodeType_name = "NodeTypeErrorNodeTypeFileNodeTypeCommentNodeTypeUseFlagNodeTypeDefinitionNodeTypeCaveatDefinitionNodeTypeCaveatParameterNodeTypeCaveatExpressionNodeTypeRelationNodeTypePermissionNodeTypeTypeReferenceNodeTypeSpecificTypeReferenceNodeTypeCaveatReferenceNodeTypeTraitReferenceNodeTypeUnionExpressionNodeTypeIntersectExpressionNodeTypeExclusionExpressionNodeTypeArrowExpressionNodeTypeIdentifierNodeTypeNilExpressionNodeTypeSelfExpressionNodeTypeCaveatTypeReferenceNodeTypeImportNodeTypePartialNodeTypePartialReference" +const _NodeType_name = "NodeTypeErrorNodeTypeFileNodeTypeCommentNodeTypeUseFlagNodeTypeDefinitionNodeTypeCaveatDefinitionNodeTypeCaveatParameterNodeTypeCaveatExpressionNodeTypeRelationNodeTypePermissionNodeTypeTypeReferenceNodeTypeSpecificTypeReferenceNodeTypeCaveatReferenceNodeTypeTraitReferenceNodeTypeUnionExpressionNodeTypeIntersectExpressionNodeTypeExclusionExpressionNodeTypeArrowExpressionNodeTypeIdentifierNodeTypeNilExpressionNodeTypeSelfExpressionNodeTypeParenthesizedExpressionNodeTypeCaveatTypeReferenceNodeTypeImportNodeTypePartialNodeTypePartialReference" -var _NodeType_index = [...]uint16{0, 13, 25, 40, 55, 73, 97, 120, 144, 160, 178, 199, 228, 251, 273, 296, 323, 350, 373, 391, 412, 434, 461, 475, 490, 514} +var _NodeType_index = [...]uint16{0, 13, 25, 40, 55, 73, 97, 120, 144, 160, 178, 199, 228, 251, 273, 296, 323, 350, 373, 391, 412, 434, 465, 492, 506, 521, 545} func (i NodeType) String() string { idx := int(i) - 0 diff --git a/pkg/composableschemadsl/parser/parser.go b/pkg/composableschemadsl/parser/parser.go index e59731638..5ac73f633 100644 --- a/pkg/composableschemadsl/parser/parser.go +++ b/pkg/composableschemadsl/parser/parser.go @@ -652,18 +652,23 @@ func (p *sourceParser) tryConsumeArrowExpression() (AstNode, bool) { // ```nil``` func (p *sourceParser) tryConsumeBaseExpression() (AstNode, bool) { switch { - // Nested expression. + // Nested expression - wrap in NodeTypeParenthesizedExpression to preserve parentheses info. case p.isToken(lexer.TokenTypeLeftParen): comments := p.currentToken.comments + wrapperNode := p.startNode(dslshape.NodeTypeParenthesizedExpression) + p.consume(lexer.TokenTypeLeftParen) - exprNode := p.consumeComputeExpression() + innerExprNode := p.consumeComputeExpression() p.consume(lexer.TokenTypeRightParen) - // Attach any comments found to the consumed expression. - p.decorateComments(exprNode, comments) + wrapperNode.Connect(dslshape.NodeParenthesizedExpressionPredicateInnerExpr, innerExprNode) + p.mustFinishNode() - return exprNode, true + // Attach any comments found to the wrapper node. + p.decorateComments(wrapperNode, comments) + + return wrapperNode, true // Self expression. case p.isKeyword("self"): diff --git a/pkg/composableschemadsl/parser/tests/basic.zed.expected b/pkg/composableschemadsl/parser/tests/basic.zed.expected index a98c1030b..aaf6b2b87 100644 --- a/pkg/composableschemadsl/parser/tests/basic.zed.expected +++ b/pkg/composableschemadsl/parser/tests/basic.zed.expected @@ -84,22 +84,27 @@ NodeTypeFile input-source = basic definition test start-rune = 208 left-expr => - NodeTypeExclusionExpression - end-rune = 217 + NodeTypeParenthesizedExpression + end-rune = 218 input-source = basic definition test - start-rune = 209 - left-expr => - NodeTypeIdentifier - end-rune = 211 - identifier-value = foo - input-source = basic definition test - start-rune = 209 - right-expr => - NodeTypeIdentifier + start-rune = 208 + inner-expr => + NodeTypeExclusionExpression end-rune = 217 - identifier-value = meh input-source = basic definition test - start-rune = 215 + start-rune = 209 + left-expr => + NodeTypeIdentifier + end-rune = 211 + identifier-value = foo + input-source = basic definition test + start-rune = 209 + right-expr => + NodeTypeIdentifier + end-rune = 217 + identifier-value = meh + input-source = basic definition test + start-rune = 215 right-expr => NodeTypeIdentifier end-rune = 224 diff --git a/pkg/composableschemadsl/parser/tests/multiparen.zed.expected b/pkg/composableschemadsl/parser/tests/multiparen.zed.expected index 5bf967dd3..d7424687b 100644 --- a/pkg/composableschemadsl/parser/tests/multiparen.zed.expected +++ b/pkg/composableschemadsl/parser/tests/multiparen.zed.expected @@ -42,58 +42,68 @@ NodeTypeFile input-source = multiple parens test start-rune = 44 right-expr => - NodeTypeExclusionExpression - end-rune = 61 + NodeTypeParenthesizedExpression + end-rune = 62 input-source = multiple parens test - start-rune = 51 - left-expr => - NodeTypeArrowExpression - end-rune = 54 + start-rune = 50 + inner-expr => + NodeTypeExclusionExpression + end-rune = 61 input-source = multiple parens test start-rune = 51 left-expr => - NodeTypeIdentifier - end-rune = 51 - identifier-value = a - input-source = multiple parens test - start-rune = 51 - right-expr => - NodeTypeIdentifier + NodeTypeArrowExpression end-rune = 54 - identifier-value = b - input-source = multiple parens test - start-rune = 54 - right-expr => - NodeTypeArrowExpression - end-rune = 61 - input-source = multiple parens test - start-rune = 58 - left-expr => - NodeTypeIdentifier - end-rune = 58 - identifier-value = c input-source = multiple parens test - start-rune = 58 + start-rune = 51 + left-expr => + NodeTypeIdentifier + end-rune = 51 + identifier-value = a + input-source = multiple parens test + start-rune = 51 + right-expr => + NodeTypeIdentifier + end-rune = 54 + identifier-value = b + input-source = multiple parens test + start-rune = 54 right-expr => - NodeTypeIdentifier + NodeTypeArrowExpression end-rune = 61 - identifier-value = d input-source = multiple parens test - start-rune = 61 + start-rune = 58 + left-expr => + NodeTypeIdentifier + end-rune = 58 + identifier-value = c + input-source = multiple parens test + start-rune = 58 + right-expr => + NodeTypeIdentifier + end-rune = 61 + identifier-value = d + input-source = multiple parens test + start-rune = 61 right-expr => - NodeTypeIntersectExpression - end-rune = 75 + NodeTypeParenthesizedExpression + end-rune = 76 input-source = multiple parens test - start-rune = 67 - left-expr => - NodeTypeIdentifier - end-rune = 69 - identifier-value = maz - input-source = multiple parens test - start-rune = 67 - right-expr => - NodeTypeIdentifier + start-rune = 66 + inner-expr => + NodeTypeIntersectExpression end-rune = 75 - identifier-value = beh input-source = multiple parens test - start-rune = 73 \ No newline at end of file + start-rune = 67 + left-expr => + NodeTypeIdentifier + end-rune = 69 + identifier-value = maz + input-source = multiple parens test + start-rune = 67 + right-expr => + NodeTypeIdentifier + end-rune = 75 + identifier-value = beh + input-source = multiple parens test + start-rune = 73 \ No newline at end of file diff --git a/pkg/composableschemadsl/parser/tests/nil.zed.expected b/pkg/composableschemadsl/parser/tests/nil.zed.expected index 90bc5578b..2c6ec7081 100644 --- a/pkg/composableschemadsl/parser/tests/nil.zed.expected +++ b/pkg/composableschemadsl/parser/tests/nil.zed.expected @@ -67,32 +67,37 @@ NodeTypeFile input-source = nil test start-rune = 117 left-expr => - NodeTypeUnionExpression - end-rune = 128 + NodeTypeParenthesizedExpression + end-rune = 129 input-source = nil test - start-rune = 118 - left-expr => + start-rune = 117 + inner-expr => NodeTypeUnionExpression - end-rune = 122 + end-rune = 128 input-source = nil test start-rune = 118 left-expr => - NodeTypeIdentifier - end-rune = 118 - identifier-value = a + NodeTypeUnionExpression + end-rune = 122 input-source = nil test start-rune = 118 + left-expr => + NodeTypeIdentifier + end-rune = 118 + identifier-value = a + input-source = nil test + start-rune = 118 + right-expr => + NodeTypeIdentifier + end-rune = 122 + identifier-value = b + input-source = nil test + start-rune = 122 right-expr => - NodeTypeIdentifier - end-rune = 122 - identifier-value = b + NodeTypeNilExpression + end-rune = 128 input-source = nil test - start-rune = 122 - right-expr => - NodeTypeNilExpression - end-rune = 128 - input-source = nil test - start-rune = 126 + start-rune = 126 right-expr => NodeTypeIdentifier end-rune = 133 diff --git a/pkg/composableschemadsl/parser/tests/parens.zed.expected b/pkg/composableschemadsl/parser/tests/parens.zed.expected index cbd5b3b45..79eb54e54 100644 --- a/pkg/composableschemadsl/parser/tests/parens.zed.expected +++ b/pkg/composableschemadsl/parser/tests/parens.zed.expected @@ -15,19 +15,24 @@ NodeTypeFile relation-name = bar start-rune = 21 compute-expression => - NodeTypeIntersectExpression - end-rune = 47 + NodeTypeParenthesizedExpression + end-rune = 48 input-source = parens test - start-rune = 39 - left-expr => - NodeTypeIdentifier - end-rune = 41 - identifier-value = maz - input-source = parens test - start-rune = 39 - right-expr => - NodeTypeIdentifier + start-rune = 38 + inner-expr => + NodeTypeIntersectExpression end-rune = 47 - identifier-value = beh input-source = parens test - start-rune = 45 \ No newline at end of file + start-rune = 39 + left-expr => + NodeTypeIdentifier + end-rune = 41 + identifier-value = maz + input-source = parens test + start-rune = 39 + right-expr => + NodeTypeIdentifier + end-rune = 47 + identifier-value = beh + input-source = parens test + start-rune = 45 \ No newline at end of file diff --git a/pkg/development/devcontext.go b/pkg/development/devcontext.go index 2ec0f5131..0b06a9580 100644 --- a/pkg/development/devcontext.go +++ b/pkg/development/devcontext.go @@ -48,6 +48,9 @@ type DevContext struct { Revision datastore.Revision CompiledSchema *compiler.CompiledSchema Dispatcher dispatch.Dispatcher + + // SchemaString is the original schema source text, used for source-scanning warnings. + SchemaString string } // NewDevContext creates a new DevContext from the specified request context, parsing and populating @@ -154,6 +157,7 @@ func newDevContextWithDatastore(ctx context.Context, requestContext *devinterfac CompiledSchema: compiled, Revision: currentRevision, Dispatcher: graph.MustNewLocalOnlyDispatcher(params), + SchemaString: requestContext.Schema, }, nil, nil } diff --git a/pkg/development/warningdefs.go b/pkg/development/warningdefs.go index b995933db..2ca26230f 100644 --- a/pkg/development/warningdefs.go +++ b/pkg/development/warningdefs.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "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/schema" @@ -230,3 +231,33 @@ var lintArrowReferencingRelation = ttuCheck{ return nil, nil }, } + +var lintMixedOperatorsWithoutParentheses = relationCheck{ + "mixed-operators-without-parentheses", + func( + ctx context.Context, + relation *corev1.Relation, + def *schema.Definition, + ) (*devinterface.DeveloperWarning, error) { + // Only check permissions, not relations. + if !def.IsPermission(relation.Name) { + return nil, nil + } + + // Check if the permission has mixed operators without parentheses. + if namespace.HasMixedOperatorsWithoutParens(relation) { + position := namespace.GetMixedOperatorsPosition(relation) + return warningForPosition( + "mixed-operators-without-parentheses", + fmt.Sprintf( + "Permission %q mixes operators (union, intersection, exclusion) at the same level without explicit parentheses; consider adding parentheses to clarify precedence", + relation.Name, + ), + relation.Name, + position, + ), nil + } + + return nil, nil + }, +} diff --git a/pkg/development/warnings.go b/pkg/development/warnings.go index 5a6b5a789..048855a78 100644 --- a/pkg/development/warnings.go +++ b/pkg/development/warnings.go @@ -3,6 +3,7 @@ package development import ( "context" "fmt" + "strings" "github.com/ccoveille/go-safecast/v2" @@ -17,6 +18,7 @@ import ( var allChecks = checks{ relationChecks: []relationCheck{ lintRelationReferencesParentType, + lintMixedOperatorsWithoutParentheses, }, computedUsersetChecks: []computedUsersetCheck{ lintPermissionReferencingItself, @@ -64,8 +66,11 @@ func GetWarnings(ctx context.Context, devCtx *DevContext) ([]*devinterface.Devel res := schema.ResolverForCompiledSchema(*devCtx.CompiledSchema) ts := schema.NewTypeSystem(res) + // Pre-split schema string once for performance when checking multiple permissions + schemaLines := strings.Split(devCtx.SchemaString, "\n") + for _, def := range devCtx.CompiledSchema.ObjectDefinitions { - found, err := addDefinitionWarnings(ctx, def, ts) + found, err := addDefinitionWarnings(ctx, def, ts, schemaLines) if err != nil { return nil, err } @@ -79,7 +84,7 @@ type contextKey string var relationKey = contextKey("relation") -func addDefinitionWarnings(ctx context.Context, nsDef *corev1.NamespaceDefinition, ts *schema.TypeSystem) ([]*devinterface.DeveloperWarning, error) { +func addDefinitionWarnings(ctx context.Context, nsDef *corev1.NamespaceDefinition, ts *schema.TypeSystem, schemaLines []string) ([]*devinterface.DeveloperWarning, error) { def, err := schema.NewDefinition(ts, nsDef) if err != nil { return nil, err diff --git a/pkg/development/warnings_test.go b/pkg/development/warnings_test.go index 5cb7ab7ab..f21e4aaa7 100644 --- a/pkg/development/warnings_test.go +++ b/pkg/development/warnings_test.go @@ -240,7 +240,7 @@ func TestWarnings(t *testing.T) { { name: "exclusion operation", schema: `definition user {} - + definition document { relation viewer: user relation editor: user @@ -249,6 +249,133 @@ func TestWarnings(t *testing.T) { `, expectedWarning: nil, }, + { + name: "mixed operators without parentheses", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = viewer + editor - admin + } + `, + expectedWarning: &developerv1.DeveloperWarning{ + Message: "Permission \"view\" mixes operators (union, intersection, exclusion) at the same level without explicit parentheses; consider adding parentheses to clarify precedence (mixed-operators-without-parentheses)", + Line: 7, + Column: 23, + SourceCode: "view", + }, + }, + { + name: "mixed operators with parentheses does not warn", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = (viewer + editor) - admin + } + `, + expectedWarning: nil, + }, + { + name: "mixed operators warning disabled", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + // spicedb-ignore-warning: mixed-operators-without-parentheses + permission view = viewer + editor - admin + } + `, + expectedWarning: nil, + }, + { + name: "mixed operators with outer parentheses still warns", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = (viewer + editor - admin) + } + `, + expectedWarning: &developerv1.DeveloperWarning{ + Message: "Permission \"view\" mixes operators (union, intersection, exclusion) at the same level without explicit parentheses; consider adding parentheses to clarify precedence (mixed-operators-without-parentheses)", + Line: 7, + Column: 24, + SourceCode: "view", + }, + }, + { + name: "mixed intersection and exclusion without parentheses", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = viewer & editor - admin + } + `, + expectedWarning: &developerv1.DeveloperWarning{ + Message: "Permission \"view\" mixes operators (union, intersection, exclusion) at the same level without explicit parentheses; consider adding parentheses to clarify precedence (mixed-operators-without-parentheses)", + Line: 7, + Column: 23, + SourceCode: "view", + }, + }, + { + name: "mixed intersection and union without parentheses", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = viewer & editor + admin + } + `, + expectedWarning: &developerv1.DeveloperWarning{ + Message: "Permission \"view\" mixes operators (union, intersection, exclusion) at the same level without explicit parentheses; consider adding parentheses to clarify precedence (mixed-operators-without-parentheses)", + Line: 7, + Column: 32, + SourceCode: "view", + }, + }, + { + name: "same operators repeated does not warn", + schema: `definition user {} + + definition document { + relation viewer: user + relation editor: user + relation admin: user + permission view = viewer + editor + admin + } + `, + expectedWarning: nil, + }, + { + name: "relation name referencing parent is a relation not permission", + schema: `definition user {} + + definition document { + relation viewer_document: user + permission view = viewer_document + }`, + expectedWarning: &developerv1.DeveloperWarning{ + Message: "Relation \"viewer_document\" references parent type \"document\" in its name; it is recommended to drop the suffix (relation-name-references-parent)", + Line: 4, + Column: 5, + SourceCode: "viewer_document", + }, + }, } for _, tc := range tcs { diff --git a/pkg/namespace/builder.go b/pkg/namespace/builder.go index 88a91d83b..cb116a752 100644 --- a/pkg/namespace/builder.go +++ b/pkg/namespace/builder.go @@ -73,6 +73,19 @@ func MustRelationWithComment(name string, comment string, rewrite *core.UsersetR return rel } +// MustWithMixedOperators marks the relation as having mixed operators without parentheses. +// This should only be used for test expectations. +func MustWithMixedOperators(rel *core.Relation, line uint64, column uint64) *core.Relation { + position := &core.SourcePosition{ + ZeroIndexedLineNumber: line, + ZeroIndexedColumnPosition: column, + } + if err := SetMixedOperatorsWithoutParens(rel, true, position); err != nil { + panic(spiceerrors.MustBugf("failed to set mixed operators: %s", err.Error())) + } + return rel +} + // AllowedRelation creates a relation reference to an allowed relation. func AllowedRelation(namespaceName string, relationName string) *core.AllowedRelation { return &core.AllowedRelation{ diff --git a/pkg/namespace/metadata.go b/pkg/namespace/metadata.go index 27cfa5255..2ef2ae70a 100644 --- a/pkg/namespace/metadata.go +++ b/pkg/namespace/metadata.go @@ -193,3 +193,94 @@ func SetTypeAnnotations(relation *core.Relation, typeAnnotations []string) error relation.Metadata.MetadataMessage = append(relation.Metadata.MetadataMessage, metadataAny) return nil } + +// HasMixedOperatorsWithoutParens returns whether the permission expression contains mixed operators +// (union, intersection, exclusion) at the same scope level without explicit parentheses. +func HasMixedOperatorsWithoutParens(relation *core.Relation) bool { + if relation.Metadata == nil { + return false + } + + for _, metadataAny := range relation.Metadata.MetadataMessage { + var relationMetadata iv1.RelationMetadata + if err := metadataAny.UnmarshalTo(&relationMetadata); err != nil { + continue + } + + if relationMetadata.Kind == iv1.RelationMetadata_PERMISSION { + return relationMetadata.HasMixedOperatorsWithoutParentheses + } + } + + return false +} + +// GetMixedOperatorsPosition returns the source position of the first mixed operator found, +// or nil if none. +func GetMixedOperatorsPosition(relation *core.Relation) *core.SourcePosition { + if relation.Metadata == nil { + return nil + } + + for _, metadataAny := range relation.Metadata.MetadataMessage { + var relationMetadata iv1.RelationMetadata + if err := metadataAny.UnmarshalTo(&relationMetadata); err != nil { + continue + } + + if relationMetadata.Kind == iv1.RelationMetadata_PERMISSION { + return relationMetadata.MixedOperatorsPosition + } + } + + return nil +} + +// SetMixedOperatorsWithoutParens sets the mixed operators flag and position for a permission relation. +func SetMixedOperatorsWithoutParens(relation *core.Relation, hasMixed bool, position *core.SourcePosition) error { + if relation.Metadata == nil { + if !hasMixed { + return nil // Nothing to set + } + relation.Metadata = &core.Metadata{} + } + + // Find existing PERMISSION RelationMetadata and update it + for i, metadataAny := range relation.Metadata.MetadataMessage { + var relationMetadata iv1.RelationMetadata + if err := metadataAny.UnmarshalTo(&relationMetadata); err != nil { + continue + } + + if relationMetadata.Kind == iv1.RelationMetadata_PERMISSION { + relationMetadata.HasMixedOperatorsWithoutParentheses = hasMixed + relationMetadata.MixedOperatorsPosition = position + + updatedAny, err := anypb.New(&relationMetadata) + if err != nil { + return err + } + + relation.Metadata.MetadataMessage[i] = updatedAny + return nil + } + } + + // If no existing PERMISSION RelationMetadata found and we need to set the flag + if hasMixed { + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + HasMixedOperatorsWithoutParentheses: hasMixed, + MixedOperatorsPosition: position, + } + + metadataAny, err := anypb.New(relationMetadata) + if err != nil { + return err + } + + relation.Metadata.MetadataMessage = append(relation.Metadata.MetadataMessage, metadataAny) + } + + return nil +} diff --git a/pkg/namespace/metadata_test.go b/pkg/namespace/metadata_test.go index 73def5f3a..9de5ec07e 100644 --- a/pkg/namespace/metadata_test.go +++ b/pkg/namespace/metadata_test.go @@ -61,6 +61,282 @@ func TestMetadata(t *testing.T) { require.Equal(iv1.RelationMetadata_PERMISSION, GetRelationKind(ns.Relation[0])) } +func TestMixedOperatorsMetadata(t *testing.T) { + tests := []struct { + name string + setupRelation func() *core.Relation + performSet bool + setMixed bool + setPosition *core.SourcePosition + expectedHasMixed bool + expectedPosition *core.SourcePosition + }{ + { + name: "has mixed returns false for nil metadata", + setupRelation: func() *core.Relation { + return &core.Relation{Name: "test"} + }, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "has mixed returns false for empty metadata", + setupRelation: func() *core.Relation { + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{}, + } + }, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "has mixed returns false for non-permission relation", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{Kind: iv1.RelationMetadata_RELATION} + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "has mixed returns false for permission without mixed flag", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{Kind: iv1.RelationMetadata_PERMISSION} + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "has mixed returns true when flag is set", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + HasMixedOperatorsWithoutParentheses: true, + MixedOperatorsPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 5, + ZeroIndexedColumnPosition: 10, + }, + } + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + expectedHasMixed: true, + expectedPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 5, + ZeroIndexedColumnPosition: 10, + }, + }, + { + name: "has mixed returns true with nil position", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + HasMixedOperatorsWithoutParentheses: true, + } + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + expectedHasMixed: true, + expectedPosition: nil, + }, + { + name: "set mixed on relation with no metadata", + setupRelation: func() *core.Relation { + return &core.Relation{Name: "test"} + }, + performSet: true, + setMixed: true, + setPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 3, + ZeroIndexedColumnPosition: 7, + }, + expectedHasMixed: true, + expectedPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 3, + ZeroIndexedColumnPosition: 7, + }, + }, + { + name: "set mixed false on relation with no metadata is no-op", + setupRelation: func() *core.Relation { + return &core.Relation{Name: "test"} + }, + performSet: true, + setMixed: false, + setPosition: nil, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "set mixed updates existing permission metadata", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + } + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + performSet: true, + setMixed: true, + setPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 2, + ZeroIndexedColumnPosition: 15, + }, + expectedHasMixed: true, + expectedPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 2, + ZeroIndexedColumnPosition: 15, + }, + }, + { + name: "set mixed on relation with non-permission metadata creates new entry", + setupRelation: func() *core.Relation { + docComment := &iv1.DocComment{Comment: "test comment"} + docAny, _ := anypb.New(docComment) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{docAny}, + }, + } + }, + performSet: true, + setMixed: true, + setPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 1, + ZeroIndexedColumnPosition: 5, + }, + expectedHasMixed: true, + expectedPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 1, + ZeroIndexedColumnPosition: 5, + }, + }, + { + name: "set mixed false clears existing flag", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + HasMixedOperatorsWithoutParentheses: true, + MixedOperatorsPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 5, + ZeroIndexedColumnPosition: 10, + }, + } + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + performSet: true, + setMixed: false, + setPosition: nil, + expectedHasMixed: false, + expectedPosition: nil, + }, + { + name: "metadata with doc comment before relation metadata", + setupRelation: func() *core.Relation { + docComment := &iv1.DocComment{Comment: "a comment"} + docAny, _ := anypb.New(docComment) + relationMetadata := &iv1.RelationMetadata{ + Kind: iv1.RelationMetadata_PERMISSION, + HasMixedOperatorsWithoutParentheses: true, + MixedOperatorsPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 8, + ZeroIndexedColumnPosition: 20, + }, + } + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{docAny, metadataAny}, + }, + } + }, + expectedHasMixed: true, + expectedPosition: &core.SourcePosition{ + ZeroIndexedLineNumber: 8, + ZeroIndexedColumnPosition: 20, + }, + }, + { + name: "set mixed false on relation with non-permission metadata only", + setupRelation: func() *core.Relation { + relationMetadata := &iv1.RelationMetadata{Kind: iv1.RelationMetadata_RELATION} + metadataAny, _ := anypb.New(relationMetadata) + return &core.Relation{ + Name: "test", + Metadata: &core.Metadata{ + MetadataMessage: []*anypb.Any{metadataAny}, + }, + } + }, + performSet: true, + setMixed: false, + setPosition: nil, + expectedHasMixed: false, + expectedPosition: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + relation := tt.setupRelation() + + if tt.performSet { + err := SetMixedOperatorsWithoutParens(relation, tt.setMixed, tt.setPosition) + require.NoError(t, err) + } + + hasMixed := HasMixedOperatorsWithoutParens(relation) + require.Equal(t, tt.expectedHasMixed, hasMixed) + + position := GetMixedOperatorsPosition(relation) + if tt.expectedPosition == nil { + require.Nil(t, position) + } else { + require.NotNil(t, position) + require.Equal(t, tt.expectedPosition.ZeroIndexedLineNumber, position.ZeroIndexedLineNumber) + require.Equal(t, tt.expectedPosition.ZeroIndexedColumnPosition, position.ZeroIndexedColumnPosition) + } + }) + } +} + func TestTypeAnnotations(t *testing.T) { tests := []struct { name string diff --git a/pkg/proto/impl/v1/impl.pb.go b/pkg/proto/impl/v1/impl.pb.go index 8a2b2eec2..9e3b37b51 100644 --- a/pkg/proto/impl/v1/impl.pb.go +++ b/pkg/proto/impl/v1/impl.pb.go @@ -7,6 +7,7 @@ package implv1 import ( + v1 "github.com/authzed/spicedb/pkg/proto/core/v1" v1alpha1 "google.golang.org/genproto/googleapis/api/expr/v1alpha1" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoimpl "google.golang.org/protobuf/runtime/protoimpl" @@ -571,8 +572,13 @@ type RelationMetadata struct { state protoimpl.MessageState `protogen:"open.v1"` Kind RelationMetadata_RelationKind `protobuf:"varint,1,opt,name=kind,proto3,enum=impl.v1.RelationMetadata_RelationKind" json:"kind,omitempty"` TypeAnnotations *TypeAnnotations `protobuf:"bytes,2,opt,name=type_annotations,json=typeAnnotations,proto3" json:"type_annotations,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Indicates whether the permission expression contains mixed operators (union, intersection, + // exclusion) at the same scope level without explicit parentheses. + HasMixedOperatorsWithoutParentheses bool `protobuf:"varint,3,opt,name=has_mixed_operators_without_parentheses,json=hasMixedOperatorsWithoutParentheses,proto3" json:"has_mixed_operators_without_parentheses,omitempty"` + // The source position of the first mixed operator found, if any. + MixedOperatorsPosition *v1.SourcePosition `protobuf:"bytes,4,opt,name=mixed_operators_position,json=mixedOperatorsPosition,proto3" json:"mixed_operators_position,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *RelationMetadata) Reset() { @@ -619,6 +625,20 @@ func (x *RelationMetadata) GetTypeAnnotations() *TypeAnnotations { return nil } +func (x *RelationMetadata) GetHasMixedOperatorsWithoutParentheses() bool { + if x != nil { + return x.HasMixedOperatorsWithoutParentheses + } + return false +} + +func (x *RelationMetadata) GetMixedOperatorsPosition() *v1.SourcePosition { + if x != nil { + return x.MixedOperatorsPosition + } + return nil +} + type NamespaceAndRevision struct { state protoimpl.MessageState `protogen:"open.v1"` NamespaceName string `protobuf:"bytes,1,opt,name=namespace_name,json=namespaceName,proto3" json:"namespace_name,omitempty"` @@ -905,7 +925,7 @@ var File_impl_v1_impl_proto protoreflect.FileDescriptor const file_impl_v1_impl_proto_rawDesc = "" + "\n" + - "\x12impl/v1/impl.proto\x12\aimpl.v1\x1a&google/api/expr/v1alpha1/checked.proto\"l\n" + + "\x12impl/v1/impl.proto\x12\aimpl.v1\x1a\x12core/v1/core.proto\x1a&google/api/expr/v1alpha1/checked.proto\"l\n" + "\rDecodedCaveat\x129\n" + "\x03cel\x18\x01 \x01(\v2%.google.api.expr.v1alpha1.CheckedExprH\x00R\x03cel\x12\x12\n" + "\x04name\x18\x02 \x01(\tR\x04nameB\f\n" + @@ -948,10 +968,12 @@ const file_impl_v1_impl_proto_rawDesc = "" + "DocComment\x12\x18\n" + "\acomment\x18\x01 \x01(\tR\acomment\"'\n" + "\x0fTypeAnnotations\x12\x14\n" + - "\x05types\x18\x01 \x03(\tR\x05types\"\xd3\x01\n" + + "\x05types\x18\x01 \x03(\tR\x05types\"\xfc\x02\n" + "\x10RelationMetadata\x12:\n" + "\x04kind\x18\x01 \x01(\x0e2&.impl.v1.RelationMetadata.RelationKindR\x04kind\x12C\n" + - "\x10type_annotations\x18\x02 \x01(\v2\x18.impl.v1.TypeAnnotationsR\x0ftypeAnnotations\">\n" + + "\x10type_annotations\x18\x02 \x01(\v2\x18.impl.v1.TypeAnnotationsR\x0ftypeAnnotations\x12T\n" + + "'has_mixed_operators_without_parentheses\x18\x03 \x01(\bR#hasMixedOperatorsWithoutParentheses\x12Q\n" + + "\x18mixed_operators_position\x18\x04 \x01(\v2\x17.core.v1.SourcePositionR\x16mixedOperatorsPosition\">\n" + "\fRelationKind\x12\x10\n" + "\fUNKNOWN_KIND\x10\x00\x12\f\n" + "\bRELATION\x10\x01\x12\x0e\n" + @@ -996,6 +1018,7 @@ var file_impl_v1_impl_proto_goTypes = []any{ (*DecodedZedToken_V1ZedToken)(nil), // 14: impl.v1.DecodedZedToken.V1ZedToken nil, // 15: impl.v1.V1Cursor.FlagsEntry (*v1alpha1.CheckedExpr)(nil), // 16: google.api.expr.v1alpha1.CheckedExpr + (*v1.SourcePosition)(nil), // 17: core.v1.SourcePosition } var file_impl_v1_impl_proto_depIdxs = []int32{ 16, // 0: impl.v1.DecodedCaveat.cel:type_name -> google.api.expr.v1alpha1.CheckedExpr @@ -1007,12 +1030,13 @@ var file_impl_v1_impl_proto_depIdxs = []int32{ 15, // 6: impl.v1.V1Cursor.flags:type_name -> impl.v1.V1Cursor.FlagsEntry 0, // 7: impl.v1.RelationMetadata.kind:type_name -> impl.v1.RelationMetadata.RelationKind 7, // 8: impl.v1.RelationMetadata.type_annotations:type_name -> impl.v1.TypeAnnotations - 9, // 9: impl.v1.V1Alpha1Revision.ns_revisions:type_name -> impl.v1.NamespaceAndRevision - 10, // [10:10] is the sub-list for method output_type - 10, // [10:10] is the sub-list for method input_type - 10, // [10:10] is the sub-list for extension type_name - 10, // [10:10] is the sub-list for extension extendee - 0, // [0:10] is the sub-list for field type_name + 17, // 9: impl.v1.RelationMetadata.mixed_operators_position:type_name -> core.v1.SourcePosition + 9, // 10: impl.v1.V1Alpha1Revision.ns_revisions:type_name -> impl.v1.NamespaceAndRevision + 11, // [11:11] is the sub-list for method output_type + 11, // [11:11] is the sub-list for method input_type + 11, // [11:11] is the sub-list for extension type_name + 11, // [11:11] is the sub-list for extension extendee + 0, // [0:11] is the sub-list for field type_name } func init() { file_impl_v1_impl_proto_init() } diff --git a/pkg/proto/impl/v1/impl.pb.validate.go b/pkg/proto/impl/v1/impl.pb.validate.go index 6811f6333..f2d10aa2c 100644 --- a/pkg/proto/impl/v1/impl.pb.validate.go +++ b/pkg/proto/impl/v1/impl.pb.validate.go @@ -1068,6 +1068,37 @@ func (m *RelationMetadata) validate(all bool) error { } } + // no validation rules for HasMixedOperatorsWithoutParentheses + + if all { + switch v := interface{}(m.GetMixedOperatorsPosition()).(type) { + case interface{ ValidateAll() error }: + if err := v.ValidateAll(); err != nil { + errors = append(errors, RelationMetadataValidationError{ + field: "MixedOperatorsPosition", + reason: "embedded message failed validation", + cause: err, + }) + } + case interface{ Validate() error }: + if err := v.Validate(); err != nil { + errors = append(errors, RelationMetadataValidationError{ + field: "MixedOperatorsPosition", + reason: "embedded message failed validation", + cause: err, + }) + } + } + } else if v, ok := interface{}(m.GetMixedOperatorsPosition()).(interface{ Validate() error }); ok { + if err := v.Validate(); err != nil { + return RelationMetadataValidationError{ + field: "MixedOperatorsPosition", + reason: "embedded message failed validation", + cause: err, + } + } + } + if len(errors) > 0 { return RelationMetadataMultiError(errors) } diff --git a/pkg/proto/impl/v1/impl_vtproto.pb.go b/pkg/proto/impl/v1/impl_vtproto.pb.go index d5323399c..708b68193 100644 --- a/pkg/proto/impl/v1/impl_vtproto.pb.go +++ b/pkg/proto/impl/v1/impl_vtproto.pb.go @@ -6,6 +6,7 @@ package implv1 import ( fmt "fmt" + v1 "github.com/authzed/spicedb/pkg/proto/core/v1" protohelpers "github.com/planetscale/vtprotobuf/protohelpers" v1alpha1 "google.golang.org/genproto/googleapis/api/expr/v1alpha1" proto "google.golang.org/protobuf/proto" @@ -312,6 +313,14 @@ func (m *RelationMetadata) CloneVT() *RelationMetadata { r := new(RelationMetadata) r.Kind = m.Kind r.TypeAnnotations = m.TypeAnnotations.CloneVT() + r.HasMixedOperatorsWithoutParentheses = m.HasMixedOperatorsWithoutParentheses + if rhs := m.MixedOperatorsPosition; rhs != nil { + if vtpb, ok := interface{}(rhs).(interface{ CloneVT() *v1.SourcePosition }); ok { + r.MixedOperatorsPosition = vtpb.CloneVT() + } else { + r.MixedOperatorsPosition = proto.Clone(rhs).(*v1.SourcePosition) + } + } if len(m.unknownFields) > 0 { r.unknownFields = make([]byte, len(m.unknownFields)) copy(r.unknownFields, m.unknownFields) @@ -822,6 +831,16 @@ func (this *RelationMetadata) EqualVT(that *RelationMetadata) bool { if !this.TypeAnnotations.EqualVT(that.TypeAnnotations) { return false } + if this.HasMixedOperatorsWithoutParentheses != that.HasMixedOperatorsWithoutParentheses { + return false + } + if equal, ok := interface{}(this.MixedOperatorsPosition).(interface{ EqualVT(*v1.SourcePosition) bool }); ok { + if !equal.EqualVT(that.MixedOperatorsPosition) { + return false + } + } else if !proto.Equal(this.MixedOperatorsPosition, that.MixedOperatorsPosition) { + return false + } return string(this.unknownFields) == string(that.unknownFields) } @@ -1579,6 +1598,38 @@ func (m *RelationMetadata) MarshalToSizedBufferVT(dAtA []byte) (int, error) { i -= len(m.unknownFields) copy(dAtA[i:], m.unknownFields) } + if m.MixedOperatorsPosition != nil { + if vtmsg, ok := interface{}(m.MixedOperatorsPosition).(interface { + MarshalToSizedBufferVT([]byte) (int, error) + }); ok { + size, err := vtmsg.MarshalToSizedBufferVT(dAtA[:i]) + if err != nil { + return 0, err + } + i -= size + i = protohelpers.EncodeVarint(dAtA, i, uint64(size)) + } else { + encoded, err := proto.Marshal(m.MixedOperatorsPosition) + if err != nil { + return 0, err + } + i -= len(encoded) + copy(dAtA[i:], encoded) + i = protohelpers.EncodeVarint(dAtA, i, uint64(len(encoded))) + } + i-- + dAtA[i] = 0x22 + } + if m.HasMixedOperatorsWithoutParentheses { + i-- + if m.HasMixedOperatorsWithoutParentheses { + dAtA[i] = 1 + } else { + dAtA[i] = 0 + } + i-- + dAtA[i] = 0x18 + } if m.TypeAnnotations != nil { size, err := m.TypeAnnotations.MarshalToSizedBufferVT(dAtA[:i]) if err != nil { @@ -1978,6 +2029,19 @@ func (m *RelationMetadata) SizeVT() (n int) { l = m.TypeAnnotations.SizeVT() n += 1 + l + protohelpers.SizeOfVarint(uint64(l)) } + if m.HasMixedOperatorsWithoutParentheses { + n += 2 + } + if m.MixedOperatorsPosition != nil { + if size, ok := interface{}(m.MixedOperatorsPosition).(interface { + SizeVT() int + }); ok { + l = size.SizeVT() + } else { + l = proto.Size(m.MixedOperatorsPosition) + } + n += 1 + l + protohelpers.SizeOfVarint(uint64(l)) + } n += len(m.unknownFields) return n } @@ -3446,6 +3510,70 @@ func (m *RelationMetadata) UnmarshalVT(dAtA []byte) error { return err } iNdEx = postIndex + case 3: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field HasMixedOperatorsWithoutParentheses", wireType) + } + var v int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return protohelpers.ErrIntOverflow + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + v |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + m.HasMixedOperatorsWithoutParentheses = bool(v != 0) + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field MixedOperatorsPosition", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return protohelpers.ErrIntOverflow + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= int(b&0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return protohelpers.ErrInvalidLength + } + postIndex := iNdEx + msglen + if postIndex < 0 { + return protohelpers.ErrInvalidLength + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.MixedOperatorsPosition == nil { + m.MixedOperatorsPosition = &v1.SourcePosition{} + } + if unmarshal, ok := interface{}(m.MixedOperatorsPosition).(interface { + UnmarshalVT([]byte) error + }); ok { + if err := unmarshal.UnmarshalVT(dAtA[iNdEx:postIndex]); err != nil { + return err + } + } else { + if err := proto.Unmarshal(dAtA[iNdEx:postIndex], m.MixedOperatorsPosition); err != nil { + return err + } + } + iNdEx = postIndex default: iNdEx = preIndex skippy, err := protohelpers.Skip(dAtA[iNdEx:]) diff --git a/pkg/schemadsl/compiler/compiler_test.go b/pkg/schemadsl/compiler/compiler_test.go index c249baed2..ef6f98b1f 100644 --- a/pkg/schemadsl/compiler/compiler_test.go +++ b/pkg/schemadsl/compiler/compiler_test.go @@ -313,16 +313,19 @@ func TestCompile(t *testing.T) { "", []SchemaDefinition{ namespace.Namespace("sometenant/complex", - namespace.MustRelation("foos", - namespace.Exclusion( - namespace.Rewrite( - namespace.Union( - namespace.ComputedUserset("bars"), - namespace.ComputedUserset("bazs"), + namespace.MustWithMixedOperators( + namespace.MustRelation("foos", + namespace.Exclusion( + namespace.Rewrite( + namespace.Union( + namespace.ComputedUserset("bars"), + namespace.ComputedUserset("bazs"), + ), ), + namespace.ComputedUserset("mehs"), ), - namespace.ComputedUserset("mehs"), ), + 1, 22, // line 1, column 22 (0-indexed) ), ), }, diff --git a/pkg/schemadsl/compiler/translator.go b/pkg/schemadsl/compiler/translator.go index bef09c425..13033276d 100644 --- a/pkg/schemadsl/compiler/translator.go +++ b/pkg/schemadsl/compiler/translator.go @@ -401,6 +401,9 @@ func translatePermission(tctx *translationContext, permissionNode *dslNode) (*co return nil, permissionNode.Errorf("invalid permission expression: %w", err) } + // Detect mixed operators without parentheses before translation (which may flatten the AST). + mixedOpsPosition := detectMixedOperatorsWithoutParens(tctx, expressionNode) + rewrite, err := translateExpression(tctx, expressionNode) if err != nil { return nil, err @@ -419,6 +422,14 @@ func translatePermission(tctx *translationContext, permissionNode *dslNode) (*co } } + // Store mixed operators flag in metadata + if mixedOpsPosition != nil { + err = namespace.SetMixedOperatorsWithoutParens(permission, true, mixedOpsPosition) + if err != nil { + return nil, permissionNode.Errorf("error adding mixed operators flag to metadata: %w", err) + } + } + if !tctx.skipValidate { if err := permission.Validate(); err != nil { return nil, permissionNode.Errorf("error in permission %s: %w", permissionName, err) @@ -569,6 +580,14 @@ func translateExpressionOperationDirect(tctx *translationContext, expressionOpNo case dslshape.NodeTypeSelfExpression: return namespace.Self(), nil + case dslshape.NodeTypeParenthesizedExpression: + // Unwrap the parenthesized expression and translate its inner expression. + innerExprNode, err := expressionOpNode.Lookup(dslshape.NodeParenthesizedExpressionPredicateInnerExpr) + if err != nil { + return nil, err + } + return translateExpressionOperation(tctx, innerExprNode) + case dslshape.NodeTypeArrowExpression: leftChild, err := expressionOpNode.Lookup(dslshape.NodeExpressionPredicateLeftExpr) if err != nil { @@ -760,3 +779,91 @@ func translateUseFlag(tctx *translationContext, useFlagNode *dslNode) error { tctx.enabledFlags = append(tctx.enabledFlags, flagName) return nil } + +// operatorType represents the type of set operator in an expression. +type operatorType int + +const ( + operatorTypeUnknown operatorType = iota + operatorTypeUnion + operatorTypeIntersection + operatorTypeExclusion +) + +// getOperatorType returns the operator type for a given node type, or operatorTypeUnknown if not a set operator. +func getOperatorType(nodeType dslshape.NodeType) operatorType { + switch nodeType { + case dslshape.NodeTypeUnionExpression: + return operatorTypeUnion + case dslshape.NodeTypeIntersectExpression: + return operatorTypeIntersection + case dslshape.NodeTypeExclusionExpression: + return operatorTypeExclusion + default: + return operatorTypeUnknown + } +} + +// detectMixedOperatorsWithoutParens walks the expression AST and detects if there are mixed +// operators (union, intersection, exclusion) at the same scope level without explicit parentheses. +// Returns the source position of the first mixed operator found, or nil if none. +// Parenthesized expressions act as boundaries - mixing inside parens does not trigger a warning +// since the user explicitly grouped the expression. However, top-level parentheses are unwrapped +// since they don't clarify internal operator precedence. +func detectMixedOperatorsWithoutParens(tctx *translationContext, node *dslNode) *core.SourcePosition { + // Unwrap top-level parenthesized expressions - they don't clarify internal precedence. + // e.g., (a + b - c) should still warn about mixed operators. + for node.GetType() == dslshape.NodeTypeParenthesizedExpression { + innerNode, err := node.Lookup(dslshape.NodeParenthesizedExpressionPredicateInnerExpr) + if err != nil { + break + } + node = innerNode + } + return detectMixedOperatorsInScope(tctx, node, operatorTypeUnknown) +} + +// detectMixedOperatorsInScope recursively checks for mixed operators within a scope. +// parentOp is the operator type seen so far at this scope level. +func detectMixedOperatorsInScope(tctx *translationContext, node *dslNode, parentOp operatorType) *core.SourcePosition { + nodeType := node.GetType() + + // Parenthesized expressions act as a boundary - don't propagate operator checking into them. + // The user explicitly grouped the expression, so we don't warn about mixing inside. + if nodeType == dslshape.NodeTypeParenthesizedExpression { + return nil + } + + currentOp := getOperatorType(nodeType) + + // If this is a set operator and we've seen a different operator at this scope, it's mixed. + if currentOp != operatorTypeUnknown && parentOp != operatorTypeUnknown && currentOp != parentOp { + return getSourcePosition(node, tctx.mapper) + } + + // If this is a set operator, check children with this operator as the scope's operator. + if currentOp != operatorTypeUnknown { + effectiveOp := currentOp + if parentOp != operatorTypeUnknown { + effectiveOp = parentOp // Keep the first operator seen at this scope + } + + // Check left child + leftChild, err := node.Lookup(dslshape.NodeExpressionPredicateLeftExpr) + if err == nil { + if pos := detectMixedOperatorsInScope(tctx, leftChild, effectiveOp); pos != nil { + return pos + } + } + + // Check right child + rightChild, err := node.Lookup(dslshape.NodeExpressionPredicateRightExpr) + if err == nil { + if pos := detectMixedOperatorsInScope(tctx, rightChild, effectiveOp); pos != nil { + return pos + } + } + } + + return nil +} diff --git a/pkg/schemadsl/dslshape/dslshape.go b/pkg/schemadsl/dslshape/dslshape.go index 331917ea6..027150091 100644 --- a/pkg/schemadsl/dslshape/dslshape.go +++ b/pkg/schemadsl/dslshape/dslshape.go @@ -33,9 +33,10 @@ const ( NodeTypeArrowExpression // A TTU in arrow form. - NodeTypeIdentifier // An identifier under an expression. - NodeTypeNilExpression // A nil keyword - NodeTypeSelfExpression // A self keyword + NodeTypeIdentifier // An identifier under an expression. + NodeTypeNilExpression // A nil keyword + NodeTypeSelfExpression // A self keyword + NodeTypeParenthesizedExpression // A parenthesized expression wrapper. NodeTypeCaveatTypeReference // A type reference for a caveat parameter. ) @@ -218,4 +219,11 @@ const ( // NodeExpressionPredicateLeftExpr = "left-expr" NodeExpressionPredicateRightExpr = "right-expr" + + // + // NodeTypeParenthesizedExpression + // + + // The inner expression wrapped by parentheses. + NodeParenthesizedExpressionPredicateInnerExpr = "inner-expr" ) diff --git a/pkg/schemadsl/dslshape/zz_generated.nodetype_string.go b/pkg/schemadsl/dslshape/zz_generated.nodetype_string.go index f3659d8fc..77a8944b9 100644 --- a/pkg/schemadsl/dslshape/zz_generated.nodetype_string.go +++ b/pkg/schemadsl/dslshape/zz_generated.nodetype_string.go @@ -30,12 +30,13 @@ func _() { _ = x[NodeTypeIdentifier-19] _ = x[NodeTypeNilExpression-20] _ = x[NodeTypeSelfExpression-21] - _ = x[NodeTypeCaveatTypeReference-22] + _ = x[NodeTypeParenthesizedExpression-22] + _ = x[NodeTypeCaveatTypeReference-23] } -const _NodeType_name = "NodeTypeErrorNodeTypeFileNodeTypeCommentNodeTypeUseFlagNodeTypeDefinitionNodeTypeCaveatDefinitionNodeTypeCaveatParameterNodeTypeCaveatExpressionNodeTypeRelationNodeTypePermissionNodeTypeTypeAnnotationNodeTypeTypeReferenceNodeTypeSpecificTypeReferenceNodeTypeCaveatReferenceNodeTypeTraitReferenceNodeTypeUnionExpressionNodeTypeIntersectExpressionNodeTypeExclusionExpressionNodeTypeArrowExpressionNodeTypeIdentifierNodeTypeNilExpressionNodeTypeSelfExpressionNodeTypeCaveatTypeReference" +const _NodeType_name = "NodeTypeErrorNodeTypeFileNodeTypeCommentNodeTypeUseFlagNodeTypeDefinitionNodeTypeCaveatDefinitionNodeTypeCaveatParameterNodeTypeCaveatExpressionNodeTypeRelationNodeTypePermissionNodeTypeTypeAnnotationNodeTypeTypeReferenceNodeTypeSpecificTypeReferenceNodeTypeCaveatReferenceNodeTypeTraitReferenceNodeTypeUnionExpressionNodeTypeIntersectExpressionNodeTypeExclusionExpressionNodeTypeArrowExpressionNodeTypeIdentifierNodeTypeNilExpressionNodeTypeSelfExpressionNodeTypeParenthesizedExpressionNodeTypeCaveatTypeReference" -var _NodeType_index = [...]uint16{0, 13, 25, 40, 55, 73, 97, 120, 144, 160, 178, 200, 221, 250, 273, 295, 318, 345, 372, 395, 413, 434, 456, 483} +var _NodeType_index = [...]uint16{0, 13, 25, 40, 55, 73, 97, 120, 144, 160, 178, 200, 221, 250, 273, 295, 318, 345, 372, 395, 413, 434, 456, 487, 514} func (i NodeType) String() string { idx := int(i) - 0 diff --git a/pkg/schemadsl/parser/parser.go b/pkg/schemadsl/parser/parser.go index fc0d2ce4f..0c2300757 100644 --- a/pkg/schemadsl/parser/parser.go +++ b/pkg/schemadsl/parser/parser.go @@ -650,18 +650,23 @@ func (p *sourceParser) tryConsumeArrowExpression() (AstNode, bool) { // ```self``` func (p *sourceParser) tryConsumeBaseExpression() (AstNode, bool) { switch { - // Nested expression. + // Nested expression - wrap in NodeTypeParenthesizedExpression to preserve parentheses info. case p.isToken(lexer.TokenTypeLeftParen): comments := p.currentToken.comments + wrapperNode := p.startNode(dslshape.NodeTypeParenthesizedExpression) + p.consume(lexer.TokenTypeLeftParen) - exprNode := p.consumeComputeExpression() + innerExprNode := p.consumeComputeExpression() p.consume(lexer.TokenTypeRightParen) - // Attach any comments found to the consumed expression. - p.decorateComments(exprNode, comments) + wrapperNode.Connect(dslshape.NodeParenthesizedExpressionPredicateInnerExpr, innerExprNode) + p.mustFinishNode() + + // Attach any comments found to the wrapper node. + p.decorateComments(wrapperNode, comments) - return exprNode, true + return wrapperNode, true // Self expression. case p.isKeyword("self"): diff --git a/pkg/schemadsl/parser/tests/basic.zed.expected b/pkg/schemadsl/parser/tests/basic.zed.expected index a98c1030b..aaf6b2b87 100644 --- a/pkg/schemadsl/parser/tests/basic.zed.expected +++ b/pkg/schemadsl/parser/tests/basic.zed.expected @@ -84,22 +84,27 @@ NodeTypeFile input-source = basic definition test start-rune = 208 left-expr => - NodeTypeExclusionExpression - end-rune = 217 + NodeTypeParenthesizedExpression + end-rune = 218 input-source = basic definition test - start-rune = 209 - left-expr => - NodeTypeIdentifier - end-rune = 211 - identifier-value = foo - input-source = basic definition test - start-rune = 209 - right-expr => - NodeTypeIdentifier + start-rune = 208 + inner-expr => + NodeTypeExclusionExpression end-rune = 217 - identifier-value = meh input-source = basic definition test - start-rune = 215 + start-rune = 209 + left-expr => + NodeTypeIdentifier + end-rune = 211 + identifier-value = foo + input-source = basic definition test + start-rune = 209 + right-expr => + NodeTypeIdentifier + end-rune = 217 + identifier-value = meh + input-source = basic definition test + start-rune = 215 right-expr => NodeTypeIdentifier end-rune = 224 diff --git a/pkg/schemadsl/parser/tests/multiparen.zed.expected b/pkg/schemadsl/parser/tests/multiparen.zed.expected index 5bf967dd3..d7424687b 100644 --- a/pkg/schemadsl/parser/tests/multiparen.zed.expected +++ b/pkg/schemadsl/parser/tests/multiparen.zed.expected @@ -42,58 +42,68 @@ NodeTypeFile input-source = multiple parens test start-rune = 44 right-expr => - NodeTypeExclusionExpression - end-rune = 61 + NodeTypeParenthesizedExpression + end-rune = 62 input-source = multiple parens test - start-rune = 51 - left-expr => - NodeTypeArrowExpression - end-rune = 54 + start-rune = 50 + inner-expr => + NodeTypeExclusionExpression + end-rune = 61 input-source = multiple parens test start-rune = 51 left-expr => - NodeTypeIdentifier - end-rune = 51 - identifier-value = a - input-source = multiple parens test - start-rune = 51 - right-expr => - NodeTypeIdentifier + NodeTypeArrowExpression end-rune = 54 - identifier-value = b - input-source = multiple parens test - start-rune = 54 - right-expr => - NodeTypeArrowExpression - end-rune = 61 - input-source = multiple parens test - start-rune = 58 - left-expr => - NodeTypeIdentifier - end-rune = 58 - identifier-value = c input-source = multiple parens test - start-rune = 58 + start-rune = 51 + left-expr => + NodeTypeIdentifier + end-rune = 51 + identifier-value = a + input-source = multiple parens test + start-rune = 51 + right-expr => + NodeTypeIdentifier + end-rune = 54 + identifier-value = b + input-source = multiple parens test + start-rune = 54 right-expr => - NodeTypeIdentifier + NodeTypeArrowExpression end-rune = 61 - identifier-value = d input-source = multiple parens test - start-rune = 61 + start-rune = 58 + left-expr => + NodeTypeIdentifier + end-rune = 58 + identifier-value = c + input-source = multiple parens test + start-rune = 58 + right-expr => + NodeTypeIdentifier + end-rune = 61 + identifier-value = d + input-source = multiple parens test + start-rune = 61 right-expr => - NodeTypeIntersectExpression - end-rune = 75 + NodeTypeParenthesizedExpression + end-rune = 76 input-source = multiple parens test - start-rune = 67 - left-expr => - NodeTypeIdentifier - end-rune = 69 - identifier-value = maz - input-source = multiple parens test - start-rune = 67 - right-expr => - NodeTypeIdentifier + start-rune = 66 + inner-expr => + NodeTypeIntersectExpression end-rune = 75 - identifier-value = beh input-source = multiple parens test - start-rune = 73 \ No newline at end of file + start-rune = 67 + left-expr => + NodeTypeIdentifier + end-rune = 69 + identifier-value = maz + input-source = multiple parens test + start-rune = 67 + right-expr => + NodeTypeIdentifier + end-rune = 75 + identifier-value = beh + input-source = multiple parens test + start-rune = 73 \ No newline at end of file diff --git a/pkg/schemadsl/parser/tests/nil.zed.expected b/pkg/schemadsl/parser/tests/nil.zed.expected index 90bc5578b..2c6ec7081 100644 --- a/pkg/schemadsl/parser/tests/nil.zed.expected +++ b/pkg/schemadsl/parser/tests/nil.zed.expected @@ -67,32 +67,37 @@ NodeTypeFile input-source = nil test start-rune = 117 left-expr => - NodeTypeUnionExpression - end-rune = 128 + NodeTypeParenthesizedExpression + end-rune = 129 input-source = nil test - start-rune = 118 - left-expr => + start-rune = 117 + inner-expr => NodeTypeUnionExpression - end-rune = 122 + end-rune = 128 input-source = nil test start-rune = 118 left-expr => - NodeTypeIdentifier - end-rune = 118 - identifier-value = a + NodeTypeUnionExpression + end-rune = 122 input-source = nil test start-rune = 118 + left-expr => + NodeTypeIdentifier + end-rune = 118 + identifier-value = a + input-source = nil test + start-rune = 118 + right-expr => + NodeTypeIdentifier + end-rune = 122 + identifier-value = b + input-source = nil test + start-rune = 122 right-expr => - NodeTypeIdentifier - end-rune = 122 - identifier-value = b + NodeTypeNilExpression + end-rune = 128 input-source = nil test - start-rune = 122 - right-expr => - NodeTypeNilExpression - end-rune = 128 - input-source = nil test - start-rune = 126 + start-rune = 126 right-expr => NodeTypeIdentifier end-rune = 133 diff --git a/pkg/schemadsl/parser/tests/parens.zed.expected b/pkg/schemadsl/parser/tests/parens.zed.expected index cbd5b3b45..79eb54e54 100644 --- a/pkg/schemadsl/parser/tests/parens.zed.expected +++ b/pkg/schemadsl/parser/tests/parens.zed.expected @@ -15,19 +15,24 @@ NodeTypeFile relation-name = bar start-rune = 21 compute-expression => - NodeTypeIntersectExpression - end-rune = 47 + NodeTypeParenthesizedExpression + end-rune = 48 input-source = parens test - start-rune = 39 - left-expr => - NodeTypeIdentifier - end-rune = 41 - identifier-value = maz - input-source = parens test - start-rune = 39 - right-expr => - NodeTypeIdentifier + start-rune = 38 + inner-expr => + NodeTypeIntersectExpression end-rune = 47 - identifier-value = beh input-source = parens test - start-rune = 45 \ No newline at end of file + start-rune = 39 + left-expr => + NodeTypeIdentifier + end-rune = 41 + identifier-value = maz + input-source = parens test + start-rune = 39 + right-expr => + NodeTypeIdentifier + end-rune = 47 + identifier-value = beh + input-source = parens test + start-rune = 45 \ No newline at end of file diff --git a/pkg/schemadsl/parser/tests/permission_edge_cases.zed.expected b/pkg/schemadsl/parser/tests/permission_edge_cases.zed.expected index 104caac02..fef3337bf 100644 --- a/pkg/schemadsl/parser/tests/permission_edge_cases.zed.expected +++ b/pkg/schemadsl/parser/tests/permission_edge_cases.zed.expected @@ -135,22 +135,27 @@ NodeTypeFile input-source = permission edge cases test start-rune = 340 right-expr => - NodeTypeExclusionExpression - end-rune = 364 + NodeTypeParenthesizedExpression + end-rune = 365 input-source = permission edge cases test - start-rune = 350 - left-expr => - NodeTypeIdentifier - end-rune = 355 - identifier-value = viewer - input-source = permission edge cases test - start-rune = 350 - right-expr => - NodeTypeIdentifier + start-rune = 349 + inner-expr => + NodeTypeExclusionExpression end-rune = 364 - identifier-value = viewer input-source = permission edge cases test - start-rune = 359 + start-rune = 350 + left-expr => + NodeTypeIdentifier + end-rune = 355 + identifier-value = viewer + input-source = permission edge cases test + start-rune = 350 + right-expr => + NodeTypeIdentifier + end-rune = 364 + identifier-value = viewer + input-source = permission edge cases test + start-rune = 359 type-annotations => NodeTypeTypeAnnotation end-rune = 336 diff --git a/proto/internal/impl/v1/impl.proto b/proto/internal/impl/v1/impl.proto index cd85ece0c..3f847ac67 100644 --- a/proto/internal/impl/v1/impl.proto +++ b/proto/internal/impl/v1/impl.proto @@ -1,6 +1,7 @@ syntax = "proto3"; package impl.v1; +import "core/v1/core.proto"; import "google/api/expr/v1alpha1/checked.proto"; option go_package = "github.com/authzed/spicedb/pkg/proto/impl/v1"; @@ -90,6 +91,13 @@ message RelationMetadata { RelationKind kind = 1; TypeAnnotations type_annotations = 2; + + // Indicates whether the permission expression contains mixed operators (union, intersection, + // exclusion) at the same scope level without explicit parentheses. + bool has_mixed_operators_without_parentheses = 3; + + // The source position of the first mixed operator found, if any. + core.v1.SourcePosition mixed_operators_position = 4; } message NamespaceAndRevision {