Skip to content

Commit 5c111b1

Browse files
q-uintq-uint
authored
fix: preserve operator precedence in LogicalExpression.String() (#28)
* 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. * chore: update CI and dev tooling * fix(ci): build golangci-lint with Go 1.26 toolchain
1 parent c1804a6 commit 5c111b1

9 files changed

Lines changed: 156 additions & 111 deletions

File tree

‎.github/workflows/pull_request.yml‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,41 @@ jobs:
33
arrange:
44
runs-on: ubuntu-latest
55
steps:
6-
- uses: actions/checkout@v2
7-
- uses: actions/setup-go@v2
6+
- uses: actions/checkout@v4
7+
- uses: actions/setup-go@v5
88
with:
9-
go-version: '1.16'
10-
- run: go get github.com/jdeflander/goarrange
11-
working-directory: ${{ runner.temp }}
9+
go-version-file: go.mod
10+
- run: go install github.com/jdeflander/goarrange@v1.0.0
1211
- run: test -z "$(goarrange run -r -d)"
1312

1413
lint:
1514
runs-on: ubuntu-latest
1615
steps:
17-
- uses: actions/checkout@v2
18-
- uses: golangci/golangci-lint-action@v2
16+
- uses: actions/checkout@v4
17+
- uses: actions/setup-go@v5
1918
with:
20-
version: v1.39
19+
go-version-file: go.mod
20+
- uses: golangci/golangci-lint-action@v9
21+
with:
22+
version: v2.11.3
23+
install-mode: goinstall
2124
args: -E misspell,godot,whitespace
2225

2326
test:
24-
strategy:
25-
matrix:
26-
go-version: [ 1.15.x, 1.16.x ]
2727
runs-on: ubuntu-latest
2828
steps:
29-
- uses: actions/checkout@v2
30-
- uses: actions/setup-go@v2
29+
- uses: actions/checkout@v4
30+
- uses: actions/setup-go@v5
3131
with:
32-
go-version: ${{ matrix.go-version }}
32+
go-version-file: go.mod
3333
- run: go test -v ./...
3434

3535
tidy:
3636
runs-on: ubuntu-latest
3737
steps:
38-
- uses: actions/checkout@v2
39-
- uses: actions/setup-go@v2
38+
- uses: actions/checkout@v4
39+
- uses: actions/setup-go@v5
4040
with:
41-
go-version: '1.16'
41+
go-version-file: go.mod
4242
- run: go mod tidy
4343
- run: git diff --quiet go.mod go.sum

‎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
@@ -105,3 +105,54 @@ func TestParseFilter(t *testing.T) {
105105
})
106106
}
107107
}
108+
109+
func TestParseFilter_precedence(t *testing.T) {
110+
for _, tc := range []struct {
111+
input string
112+
want string
113+
}{
114+
{
115+
input: "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
116+
want: "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
117+
},
118+
{
119+
input: "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
120+
want: "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
121+
},
122+
{
123+
input: "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
124+
want: "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
125+
},
126+
{
127+
input: "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
128+
want: "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
129+
},
130+
{
131+
input: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
132+
want: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
133+
},
134+
{
135+
input: "(a eq \"1\" and b eq \"2\") or c eq \"3\"",
136+
want: "a eq \"1\" and b eq \"2\" or c eq \"3\"",
137+
},
138+
{
139+
input: "(a eq \"1\")",
140+
want: "a eq \"1\"",
141+
},
142+
{
143+
input: "a eq \"1\" or (b eq \"2\" and c eq \"3\")",
144+
want: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
145+
},
146+
} {
147+
t.Run(tc.input, func(t *testing.T) {
148+
exp, err := ParseFilter([]byte(tc.input))
149+
if err != nil {
150+
t.Fatal(err)
151+
}
152+
got := fmt.Sprintf("%v", exp)
153+
if got != tc.want {
154+
t.Errorf("got %q, want %q", got, tc.want)
155+
}
156+
})
157+
}
158+
}

‎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: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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+
pkgs.golangci-lint
21+
];
22+
};
23+
}
24+
);
25+
};
26+
}

‎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)