fix: preserve operator precedence in LogicalExpression.String()

.github/workflows/pull_request.yml

@@ -3,41 +3,41 @@ jobs:
   arrange:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-go@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
         with:
-          go-version: '1.16'
-      - run: go get github.com/jdeflander/goarrange
-        working-directory: ${{ runner.temp }}
+          go-version-file: go.mod
+      - run: go install github.com/jdeflander/goarrange@v1.0.0
       - run: test -z "$(goarrange run -r -d)"
 
   lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: golangci/golangci-lint-action@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
         with:
-          version: v1.39
+          go-version-file: go.mod
+      - uses: golangci/golangci-lint-action@v9
+        with:
+          version: v2.11.3
+          install-mode: goinstall
           args: -E misspell,godot,whitespace
 
   test:
-    strategy:
-      matrix:
-        go-version: [ 1.15.x, 1.16.x ]
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-go@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
         with:
-          go-version: ${{ matrix.go-version }}
+          go-version-file: go.mod
       - run: go test -v ./...
 
   tidy:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
-      - uses: actions/setup-go@v2
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
         with:
-          go-version: '1.16'
+          go-version-file: go.mod
       - run: go mod tidy
       - run: git diff --quiet go.mod go.sum

ast.go

@@ -36,7 +36,7 @@ const (
 type AttributeExpression struct {
 	AttributePath AttributePath
 	Operator      CompareOperator
-	CompareValue  interface{}
+	CompareValue  any
 }
 
 func (e AttributeExpression) String() string {
@@ -54,12 +54,13 @@ func (e AttributeExpression) String() string {
 
 func (*AttributeExpression) exprNode() {}
 
-// AttributePath represents an attribute path. Both URIPrefix and SubAttr are
-// optional values and can be nil.
-// e.g. urn:ietf:params:scim:schemas:core:2.0:User:name.givenName
-//      ^                                          ^    ^
-//      URIPrefix                                  |    SubAttribute
-//                                                 AttributeName
+// AttributePath represents an attribute path with an optional URIPrefix and
+// SubAttribute.
+//
+// Example: urn:ietf:params:scim:schemas:core:2.0:User:name.givenName
+//   - URIPrefix: urn:ietf:params:scim:schemas:core:2.0:User
+//   - AttributeName: name
+//   - SubAttribute: givenName
 type AttributePath struct {
 	URIPrefix     *string
 	AttributeName string
@@ -100,10 +101,10 @@ type CompareOperator string
 
 // Expression is a type to assign to implemented expressions.
 // Valid expressions are:
-//	- ValuePath
-//	- AttributeExpression
-//	- LogicalExpression
-// 	- NotExpression
+//   - ValuePath
+//   - AttributeExpression
+//   - LogicalExpression
+//   - NotExpression
 type Expression interface {
 	exprNode()
 }
@@ -115,7 +116,19 @@ type LogicalExpression struct {
 }
 
 func (e LogicalExpression) String() string {
-	return fmt.Sprintf("%v %s %v", e.Left, e.Operator, e.Right)
+	left := fmt.Sprintf("%v", e.Left)
+	if e.Operator == AND {
+		if l, ok := e.Left.(*LogicalExpression); ok && l.Operator == OR {
+			left = fmt.Sprintf("(%v)", e.Left)
+		}
+	}
+	right := fmt.Sprintf("%v", e.Right)
+	if e.Operator == AND {
+		if r, ok := e.Right.(*LogicalExpression); ok && r.Operator == OR {
+			right = fmt.Sprintf("(%v)", e.Right)
+		}
+	}
+	return fmt.Sprintf("%s %s %s", left, e.Operator, right)
 }
 
 func (*LogicalExpression) exprNode() {}
@@ -134,12 +147,13 @@ func (e NotExpression) String() string {
 
 func (*NotExpression) exprNode() {}
 
-// Path describes the target of a PATCH operation. Path can have an optional
+// Path describes the target of a PATCH operation with an optional
 // ValueExpression and SubAttribute.
-// e.g. members[value eq "2819c223-7f76-453a-919d-413861904646"].displayName
-//      ^       ^                                                ^
-//      |       ValueExpression                                  SubAttribute
-//      AttributePath
+//
+// Example: members[value eq "2819c223-..."].displayName
+//   - AttributePath: members
+//   - ValueExpression: value eq "2819c223-..."
+//   - SubAttribute: displayName
 type Path struct {
 	AttributePath   AttributePath
 	ValueExpression Expression

attrexp.go

@@ -65,7 +65,7 @@ func (p config) parseAttrExp(node *ast.Node) (AttributeExpression, error) {
 
 	var (
 		compareOp    = CompareOperator(strings.ToLower(children[1].Value))
-		compareValue interface{}
+		compareValue any
 	)
 	switch node := children[2]; node.Type {
 	case typ.False:
@@ -96,7 +96,7 @@ func (p config) parseAttrExp(node *ast.Node) (AttributeExpression, error) {
 	}, nil
 }
 
-func (p config) parseNumber(node *ast.Node) (interface{}, error) {
+func (p config) parseNumber(node *ast.Node) (any, error) {
 	var frac, exp bool
 	var nStr string
 	for _, node := range node.Children() {

attrexp_test.go

@@ -24,7 +24,7 @@ func ExampleParseAttrExp_sw() {
 func TestParseNumber(t *testing.T) {
 	for _, test := range []struct {
 		nStr     string
-		expected interface{}
+		expected any
 	}{
 		{
 			nStr:     "-5.1e-2",

filter_test.go

@@ -105,3 +105,54 @@ func TestParseFilter(t *testing.T) {
 		})
 	}
 }
+
+func TestParseFilter_precedence(t *testing.T) {
+	for _, tc := range []struct {
+		input string
+		want  string
+	}{
+		{
+			input: "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
+			want:  "(a eq \"1\" or b eq \"2\") and c eq \"3\"",
+		},
+		{
+			input: "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
+			want:  "c eq \"3\" and (a eq \"1\" or b eq \"2\")",
+		},
+		{
+			input: "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
+			want:  "(a eq \"1\" or b eq \"2\") and (c eq \"3\" or d eq \"4\")",
+		},
+		{
+			input: "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
+			want:  "a eq \"1\" and (b eq \"2\" or c eq \"3\") and d eq \"4\"",
+		},
+		{
+			input: "a eq \"1\" or b eq \"2\" and c eq \"3\"",
+			want:  "a eq \"1\" or b eq \"2\" and c eq \"3\"",
+		},
+		{
+			input: "(a eq \"1\" and b eq \"2\") or c eq \"3\"",
+			want:  "a eq \"1\" and b eq \"2\" or c eq \"3\"",
+		},
+		{
+			input: "(a eq \"1\")",
+			want:  "a eq \"1\"",
+		},
+		{
+			input: "a eq \"1\" or (b eq \"2\" and c eq \"3\")",
+			want:  "a eq \"1\" or b eq \"2\" and c eq \"3\"",
+		},
+	} {
+		t.Run(tc.input, func(t *testing.T) {
+			exp, err := ParseFilter([]byte(tc.input))
+			if err != nil {
+				t.Fatal(err)
+			}
+			got := fmt.Sprintf("%v", exp)
+			if got != tc.want {
+				t.Errorf("got %q, want %q", got, tc.want)
+			}
+		})
+	}
+}

flake.nix

@@ -0,0 +1,26 @@
+{
+  inputs = {
+    nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";
+  };
+
+  outputs = { self, nixpkgs }:
+    let
+      supportedSystems = [ "x86_64-linux" "aarch64-linux" "x86_64-darwin" "aarch64-darwin" ];
+      forAllSystems = nixpkgs.lib.genAttrs supportedSystems;
+    in
+    {
+      devShells = forAllSystems (system:
+        let
+          pkgs = nixpkgs.legacyPackages.${system};
+        in
+        {
+          default = pkgs.mkShell {
+            packages = [
+              pkgs.go_1_26
+              pkgs.golangci-lint
+            ];
+          };
+        }
+      );
+    };
+}

go.mod

@@ -1,5 +1,5 @@
 module github.com/scim2/filter-parser/v2
 
-go 1.16
+go 1.26
 
 require github.com/di-wu/parser v0.2.2