Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ coverage*.txt
.claude/
e2e/newenemy/spicedb
e2e/newenemy/cockroach
e2e/newenemy/chaosd
e2e/newenemy/chaosd

# Auto Claude data directory
.auto-claude/
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,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=
Expand Down
17 changes: 10 additions & 7 deletions pkg/composableschemadsl/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
),
),
},
Expand Down
107 changes: 107 additions & 0 deletions pkg/composableschemadsl/compiler/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,9 @@
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
Expand All @@ -446,6 +449,14 @@
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)

Check warning on line 456 in pkg/composableschemadsl/compiler/translator.go

View check run for this annotation

Codecov / codecov/patch

pkg/composableschemadsl/compiler/translator.go#L456

Added line #L456 was not covered by tests
}
}

if !tctx.skipValidate {
if err := permission.Validate(); err != nil {
return nil, permissionNode.Errorf("error in permission %s: %w", permissionName, err)
Expand Down Expand Up @@ -576,6 +587,14 @@
case dslshape.NodeTypeNilExpression:
return namespace.Nil(), 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

Check warning on line 594 in pkg/composableschemadsl/compiler/translator.go

View check run for this annotation

Codecov / codecov/patch

pkg/composableschemadsl/compiler/translator.go#L594

Added line #L594 was not covered by tests
}
return translateExpressionOperation(tctx, innerExprNode)

case dslshape.NodeTypeArrowExpression:
leftChild, err := expressionOpNode.Lookup(dslshape.NodeExpressionPredicateLeftExpr)
if err != nil {
Expand Down Expand Up @@ -943,3 +962,91 @@
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

Check warning on line 1002 in pkg/composableschemadsl/compiler/translator.go

View check run for this annotation

Codecov / codecov/patch

pkg/composableschemadsl/compiler/translator.go#L1000-L1002

Added lines #L1000 - L1002 were not covered by tests
}
node = innerNode

Check warning on line 1004 in pkg/composableschemadsl/compiler/translator.go

View check run for this annotation

Codecov / codecov/patch

pkg/composableschemadsl/compiler/translator.go#L1004

Added line #L1004 was not covered by tests
}
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

Check warning on line 1046 in pkg/composableschemadsl/compiler/translator.go

View check run for this annotation

Codecov / codecov/patch

pkg/composableschemadsl/compiler/translator.go#L1046

Added line #L1046 was not covered by tests
}
}
}

return nil
}
12 changes: 10 additions & 2 deletions pkg/composableschemadsl/dslshape/dslshape.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ const (

NodeTypeArrowExpression // A TTU in arrow form.

NodeTypeIdentifier // An identifier under an expression.
NodeTypeNilExpression // A nil keyword
NodeTypeIdentifier // An identifier under an expression.
NodeTypeNilExpression // A nil keyword
NodeTypeParenthesizedExpression // A parenthesized expression wrapper.

NodeTypeCaveatTypeReference // A type reference for a caveat parameter.

Expand Down Expand Up @@ -211,6 +212,13 @@ const (
NodeExpressionPredicateLeftExpr = "left-expr"
NodeExpressionPredicateRightExpr = "right-expr"

//
// NodeTypeParenthesizedExpression
//

// The inner expression wrapped by parentheses.
NodeParenthesizedExpressionPredicateInnerExpr = "inner-expr"

//
// NodeTypeImport
//
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 10 additions & 5 deletions pkg/composableschemadsl/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

// Nil expression.
case p.isKeyword("nil"):
Expand Down
31 changes: 18 additions & 13 deletions pkg/composableschemadsl/parser/tests/basic.zed.expected
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading