Skip to content

Commit 50cd0d3

Browse files
q-uintq-uint
committed
fix: preserve operator precedence in LogicalExpression.String()
Wrap OR children in parentheses when the parent operator is AND, so the string output matches the parsed grouping.
1 parent c1804a6 commit 50cd0d3

8 files changed

Lines changed: 138 additions & 94 deletions

File tree

‎ast.go‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const (
3636
type AttributeExpression struct {
3737
AttributePath AttributePath
3838
Operator CompareOperator
39-
CompareValue interface{}
39+
CompareValue any
4040
}
4141

4242
func (e AttributeExpression) String() string {
@@ -54,12 +54,13 @@ func (e AttributeExpression) String() string {
5454

5555
func (*AttributeExpression) exprNode() {}
5656

57-
// AttributePath represents an attribute path. Both URIPrefix and SubAttr are
58-
// optional values and can be nil.
59-
// e.g. urn:ietf:params:scim:schemas:core:2.0:User:name.givenName
60-
// ^ ^ ^
61-
// URIPrefix | SubAttribute
62-
// AttributeName
57+
// AttributePath represents an attribute path with an optional URIPrefix and
58+
// SubAttribute.
59+
//
60+
// Example: urn:ietf:params:scim:schemas:core:2.0:User:name.givenName
61+
// - URIPrefix: urn:ietf:params:scim:schemas:core:2.0:User
62+
// - AttributeName: name
63+
// - SubAttribute: givenName
6364
type AttributePath struct {
6465
URIPrefix *string
6566
AttributeName string
@@ -100,10 +101,10 @@ type CompareOperator string
100101

101102
// Expression is a type to assign to implemented expressions.
102103
// Valid expressions are:
103-
// - ValuePath
104-
// - AttributeExpression
105-
// - LogicalExpression
106-
// - NotExpression
104+
// - ValuePath
105+
// - AttributeExpression
106+
// - LogicalExpression
107+
// - NotExpression
107108
type Expression interface {
108109
exprNode()
109110
}
@@ -115,7 +116,19 @@ type LogicalExpression struct {
115116
}
116117

117118
func (e LogicalExpression) String() string {
118-
return fmt.Sprintf("%v %s %v", e.Left, e.Operator, e.Right)
119+
left := fmt.Sprintf("%v", e.Left)
120+
if e.Operator == AND {
121+
if l, ok := e.Left.(*LogicalExpression); ok && l.Operator == OR {
122+
left = fmt.Sprintf("(%v)", e.Left)
123+
}
124+
}
125+
right := fmt.Sprintf("%v", e.Right)
126+
if e.Operator == AND {
127+
if r, ok := e.Right.(*LogicalExpression); ok && r.Operator == OR {
128+
right = fmt.Sprintf("(%v)", e.Right)
129+
}
130+
}
131+
return fmt.Sprintf("%s %s %s", left, e.Operator, right)
119132
}
120133

121134
func (*LogicalExpression) exprNode() {}
@@ -134,12 +147,13 @@ func (e NotExpression) String() string {
134147

135148
func (*NotExpression) exprNode() {}
136149

137-
// Path describes the target of a PATCH operation. Path can have an optional
150+
// Path describes the target of a PATCH operation with an optional
138151
// ValueExpression and SubAttribute.
139-
// e.g. members[value eq "2819c223-7f76-453a-919d-413861904646"].displayName
140-
// ^ ^ ^
141-
// | ValueExpression SubAttribute
142-
// AttributePath
152+
//
153+
// Example: members[value eq "2819c223-..."].displayName
154+
// - AttributePath: members
155+
// - ValueExpression: value eq "2819c223-..."
156+
// - SubAttribute: displayName
143157
type Path struct {
144158
AttributePath AttributePath
145159
ValueExpression Expression

‎attrexp.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (p config) parseAttrExp(node *ast.Node) (AttributeExpression, error) {
6565

6666
var (
6767
compareOp = CompareOperator(strings.ToLower(children[1].Value))
68-
compareValue interface{}
68+
compareValue any
6969
)
7070
switch node := children[2]; node.Type {
7171
case typ.False:
@@ -96,7 +96,7 @@ func (p config) parseAttrExp(node *ast.Node) (AttributeExpression, error) {
9696
}, nil
9797
}
9898

99-
func (p config) parseNumber(node *ast.Node) (interface{}, error) {
99+
func (p config) parseNumber(node *ast.Node) (any, error) {
100100
var frac, exp bool
101101
var nStr string
102102
for _, node := range node.Children() {

‎attrexp_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func ExampleParseAttrExp_sw() {
2424
func TestParseNumber(t *testing.T) {
2525
for _, test := range []struct {
2626
nStr string
27-
expected interface{}
27+
expected any
2828
}{
2929
{
3030
nStr: "-5.1e-2",

‎filter_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,57 @@ func Example_walk() {
7272
// value co "@foo.com"
7373
}
7474

75+
func TestParseFilter_precedence(t *testing.T) {
76+
for _, tc := range []struct {
77+
input string
78+
want string
79+
}{
80+
{
81+
input: "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
82+
want: "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
83+
},
84+
{
85+
input: "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
86+
want: "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
87+
},
88+
{
89+
input: "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
90+
want: "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
91+
},
92+
{
93+
input: "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
94+
want: "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
95+
},
96+
{
97+
input: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
98+
want: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
99+
},
100+
{
101+
input: "(a eq \"1\" and b eq \"2\") or c eq \"3\"",
102+
want: "a eq \"1\" and b eq \"2\" or c eq \"3\"",
103+
},
104+
{
105+
input: "(a eq \"1\")",
106+
want: "a eq \"1\"",
107+
},
108+
{
109+
input: "a eq \"1\" or (b eq \"2\" and c eq \"3\")",
110+
want: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
111+
},
112+
} {
113+
t.Run(tc.input, func(t *testing.T) {
114+
exp, err := ParseFilter([]byte(tc.input))
115+
if err != nil {
116+
t.Fatal(err)
117+
}
118+
got := fmt.Sprintf("%v", exp)
119+
if got != tc.want {
120+
t.Errorf("got %q, want %q", got, tc.want)
121+
}
122+
})
123+
}
124+
}
125+
75126
func TestParseFilter(t *testing.T) {
76127
for _, example := range []string{
77128
"userName eq \"bjensen\"",

‎flake.lock‎

Lines changed: 27 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎flake.nix‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
inputs = {
3+
nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";
4+
};
5+
6+
outputs = { self, nixpkgs }:
7+
let
8+
supportedSystems = [ "x86_64-linux" "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ];
9+
forAllSystems = nixpkgs.lib.genAttrs supportedSystems;
10+
in
11+
{
12+
devShells = forAllSystems (system:
13+
let
14+
pkgs = nixpkgs.legacyPackages.${system};
15+
in
16+
{
17+
default = pkgs.mkShell {
18+
packages = [
19+
pkgs.go_1_26
20+
];
21+
};
22+
}
23+
);
24+
};
25+
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
module github.com/scim2/filter-parser/v2
22

3-
go 1.16
3+
go 1.26
44

55
require github.com/di-wu/parser v0.2.2

‎internal/spec/grammar.pegn‎

Lines changed: 0 additions & 73 deletions
This file was deleted.

0 commit comments

Comments
 (0)