From c0a381e43ddb63c65f1c12e3954227236a879046 Mon Sep 17 00:00:00 2001
From: Daniel Trowbridge <blujupiter32@pm.me>
Date: Thu, 26 Dec 2024 19:49:46 +0000
Subject: [PATCH] Fix semantics of `not_` applied over `&&.` and `||.` (#379)

* Add tests for not_ applied to more complex expressions

* Always parenthesize arguments to not_

* Update test/Common/Test.hs

---------

Co-authored-by: Matt Parsons <parsonsmatt@gmail.com>
---
 src/Database/Esqueleto/Internal/Internal.hs |  7 ++-
 test/Common/Test.hs                         | 55 +++++++++++++++++----
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/src/Database/Esqueleto/Internal/Internal.hs b/src/Database/Esqueleto/Internal/Internal.hs
index fa9fc19b2..c750c7cae 100644
--- a/src/Database/Esqueleto/Internal/Internal.hs
+++ b/src/Database/Esqueleto/Internal/Internal.hs
@@ -717,16 +717,15 @@ countDistinct :: Num a => SqlExpr (Value typ) -> SqlExpr (Value a)
 countDistinct = countHelper "(DISTINCT " ")"
 
 not_ :: SqlExpr (Value Bool) -> SqlExpr (Value Bool)
-not_ v = ERaw noMeta $ \p info -> first ("NOT " <>) $ x p info
+not_ v = ERaw noMeta (const $ first ("NOT " <>) . x)
   where
-    x p info =
+    x info =
         case v of
             ERaw m f ->
                 if hasCompositeKeyMeta m then
                     throw (CompositeKeyErr NotError)
                 else
-                    let (b, vals) = f Never info
-                    in (parensM p b, vals)
+                    f Parens info
 
 (==.) :: PersistField typ => SqlExpr (Value typ) -> SqlExpr (Value typ) -> SqlExpr (Value Bool)
 (==.) = unsafeSqlBinOpComposite " = " " AND "
diff --git a/test/Common/Test.hs b/test/Common/Test.hs
index b74850143..f79cafd55 100644
--- a/test/Common/Test.hs
+++ b/test/Common/Test.hs
@@ -878,16 +878,6 @@ testSelectWhere = describe "select where_" $ do
                return p
         asserting $ ret `shouldBe` [ p1e ]
 
-    itDb "works for a simple example with (>.) and not_ [uses just . val]" $ do
-        _   <- insert' p1
-        _   <- insert' p2
-        p3e <- insert' p3
-        ret <- select $
-               from $ \p -> do
-               where_ (not_ $ p ^. PersonAge >. just (val 17))
-               return p
-        asserting $ ret `shouldBe` [ p3e ]
-
     describe "when using between" $ do
         itDb "works for a simple example with [uses just . val]" $ do
             p1e  <- insert' p1
@@ -921,6 +911,51 @@ testSelectWhere = describe "select where_" $ do
                         , val $ PointKey 5 6 )
                 asserting $ ret `shouldBe` [()]
 
+    describe "when using not_" $ do
+        itDb "works for a single expression" $ do
+            ret <-
+                select $
+                pure $ not_ $ val True
+            asserting $ do
+                ret `shouldBe` [Value False]
+
+        itDb "works for a simple example with (>.) [uses just . val]" $ do
+            _   <- insert' p1
+            _   <- insert' p2
+            p3e <- insert' p3
+            ret <- select $
+                   from $ \p -> do
+                   where_ (not_ $ p ^. PersonAge >. just (val 17))
+                   return p
+            asserting $ ret `shouldBe` [ p3e ]
+        itDb "works with (==.) and (||.)" $ do
+            _   <- insert' p1
+            _   <- insert' p2
+            p3e <- insert' p3
+            ret <- select $
+                   from $ \p -> do
+                   where_ (not_ $ p ^. PersonName ==. val "John" ||. p ^. PersonName ==. val "Rachel")
+                   return p
+            asserting $ ret `shouldBe` [ p3e ]
+        itDb "works with (>.), (<.) and (&&.) [uses just . val]" $ do
+            p1e <- insert' p1
+            _   <- insert' p2
+            _   <- insert' p3
+            ret <- select $
+                   from $ \p -> do
+                   where_ (not_ $ (p ^. PersonAge >. just (val 10)) &&. (p ^. PersonAge <. just (val 30)))
+                   return p
+            asserting $ ret `shouldBe` [ p1e ]
+        itDb "works with between [uses just . val]" $ do
+            _   <- insert' p1
+            _   <- insert' p2
+            p3e <- insert' p3
+            ret <- select $
+                   from $ \p -> do
+                   where_ (not_ $ (p ^. PersonAge) `between` (just $ val 20, just $ val 40))
+                   return p
+            asserting $ ret `shouldBe` [ p3e ]
+
     itDb "works with avg_" $ do
         _ <- insert' p1
         _ <- insert' p2