fix handling of implicit and before parens

Fix an oddity in matching options and preferred content expressions such as
"foo (bar or baz)", which was incorrectly handled as if it were "(foo or
bar) and baz)" rather than the intended "foo and (bar or baz)"

Seemed like a change to consume should be able to handle this case
better, but I was having trouble writing it that way, so instead added
a separate pass that inserts the implicit ands explicitly. Also added
several test cases to make sure versions with and without explicit ands
generate the same.

CHANGELOG

@@ -5,6 +5,10 @@ git-annex (8.20210128) UNRELEASED; urgency=medium
     of that special remote.
   * When adding files to an adjusted branch set up by --unlock-present,
     add them unlocked, not locked.
+  * Fix an oddity in matching options and preferred content expressions
+    such as "foo (bar or baz)", which was incorrectly handled as if
+    it were "(foo or bar) and baz)" rather than the intended
+    "foo and (bar or baz)"
 
  -- Joey Hess <id@joeyh.name>  Thu, 28 Jan 2021 12:34:32 -0400
 

Utility/Matcher.hs

@@ -10,7 +10,7 @@
  - Is forgiving about misplaced closing parens, so "foo and (bar or baz"
  - will be handled, as will "foo and ( bar or baz ) )"
  -
- - Copyright 2011-2020 Joey Hess <id@joeyh.name>
+ - Copyright 2011-2021 Joey Hess <id@joeyh.name>
  -
  - License: BSD-2-clause
  -}
@@ -57,7 +57,7 @@ syntaxToken t = Left $ "unknown token " ++ t
 
 {- Converts a list of Tokens into a Matcher. -}
 generate :: [Token op] -> Matcher op
-generate = simplify . process MAny . tokenGroups
+generate = simplify . process MAny . implicitAnd . tokenGroups
   where
 	process m [] = m
 	process m ts = uncurry process $ consume m ts
@@ -108,6 +108,16 @@ findClose l =
 			in go (Group (reverse c') : c) ts'
 		dispatch _ = go (One t:c) ts
 
+implicitAnd :: [TokenGroup op] -> [TokenGroup op]
+implicitAnd [] = []
+implicitAnd [v] = [v]
+implicitAnd (a:b:rest) | need a && need b = a : One And : implicitAnd (b:rest)
+  where
+	need (One (Operation _)) = True
+	need (Group _) = True
+	need _ = False
+implicitAnd (a:rest) = a : implicitAnd rest
+
 {- Checks if a Matcher matches, using a supplied function to check
  - the value of Operations. -}
 match :: (op -> v -> Bool) -> Matcher op -> v -> Bool
@@ -153,27 +163,56 @@ introspect :: (a -> Bool) -> Matcher a -> Bool
 introspect = any
 
 prop_matcher_sane :: Bool
-prop_matcher_sane = all (\m -> match dummy m ()) $ map generate
+prop_matcher_sane = and
-	[ [Operation True]
+	[ all (\m -> match (\b _ -> b) m ()) (map generate evaltrue)
-	, []
+	, all (\(x,y) -> generate x == generate y) evalsame
-	, [Operation False, Or, Operation True, Or, Operation False]
-	, [Operation True, Or, Operation True]
-	, [Operation True, And, Operation True]
-	, [Not, Open, Operation True, And, Operation False, Close]
-	, [Not, Open, Not, Open, Not, Operation False, Close, Close]
-	, [Not, Open, Not, Open, Not, Open, Not, Operation True, Close, Close]
-	, [Operation True, And, Not, Operation False]
-	, [Operation True, Not, Operation False]
-	, [Operation True, Not, Not, Not, Operation False]
-	, [Operation True, Not, Not, Not, Operation False, And, Operation True]
-	, [Operation True, Not, Not, Not, Operation False, Operation True]
-	, [Not, Open, Operation True, And, Operation False, Close, 
-		And, Open, 
-			Open, Operation True, And, Operation False, Close,
-			Or,
-			Open, Operation True, And, Open, Not, Operation False, Close, Close,
-		Close, And,
-			Open, Not, Operation False, Close]
 	]
   where
-	dummy b _ = b
+	evaltrue =
+		[ [Operation True]
+		, []
+		, [Operation False, Or, Operation True, Or, Operation False]
+		, [Operation True, Or, Operation True]
+		, [Operation True, And, Operation True]
+		, [Not, Open, Operation True, And, Operation False, Close]
+		, [Not, Open, Not, Open, Not, Operation False, Close, Close]
+		, [Not, Open, Not, Open, Not, Open, Not, Operation True, Close, Close]
+		, [Operation True, And, Not, Operation False]
+		, [Operation True, Not, Operation False]
+		, [Operation True, Not, Not, Not, Operation False]
+		, [Operation True, Not, Not, Not, Operation False, And, Operation True]
+		, [Operation True, Not, Not, Not, Operation False, Operation True]
+		, [Not, Open, Operation True, And, Operation False, Close, 
+			And, Open, 
+				Open, Operation True, And, Operation False, Close,
+				Or,
+				Open, Operation True, And, Open, Not, Operation False, Close, Close,
+			Close, And,
+				Open, Not, Operation False, Close]
+		]
+	evalsame =
+		[
+			( [Operation "foo", Open, Operation "bar", Or, Operation "baz", Close]
+			, [Operation "foo", And, Open, Operation "bar", Or, Operation "baz", Close]
+			)
+		,
+			( [Operation "foo", Not, Open, Operation "bar", Or, Operation "baz", Close]
+			, [Operation "foo", And, Not, Open, Operation "bar", Or, Operation "baz", Close]
+			)
+		,
+			( [Open, Operation "bar", Or, Operation "baz", Close, Operation "foo"]
+			, [Open, Operation "bar", Or, Operation "baz", Close, And, Operation "foo"]
+			)
+		,
+			( [Open, Operation "bar", Or, Operation "baz", Close, Not, Operation "foo"]
+			, [Open, Operation "bar", Or, Operation "baz", Close, And, Not, Operation "foo"]
+			)
+		,
+			( [Operation "foo", Operation "bar"]
+			, [Operation "foo", And, Operation "bar"]
+			)
+		,
+			( [Operation "foo", Not, Operation "bar"]
+			, [Operation "foo", And, Not, Operation "bar"]
+			)
+		]

doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches.mdwn

@@ -82,3 +82,5 @@ b.2
 ### Have you had any luck using git-annex before? (Sometimes we get tired of reading bug reports all day and a lil' positive end note does wonders)
 
 Yes! [My description from before](https://git-annex.branchable.com/bugs/async_external_special_remote__39__s_stdin_not_closed/) still applies. And I've continued growing my annex and expanded onto a new drive since then.
+
+> [[fixed|done]] --[[Joey]] 

doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches/comment_1_8160d9308dc19b3c6a0596f437134e28._comment

@@ -0,0 +1,20 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2021-01-28T16:54:29Z"
+ content="""
+Let's see, here's the equivilant after parsing:
+
+	# ghci Utility/Matcher.hs
+	-- foo and (bar or baz)
+	ghci> generate [Operation "foo", And, Open, Operation "bar", Or, Operation "baz", Close]
+	MAnd (MOp "foo") (MOr (MOp "bar") (MOp "baz"))
+	-- foo (bar or baz)
+	ghci> generate [Operation "foo", Open, Operation "bar", Or, Operation "baz", Close]
+	MOr (MAnd (MOp "foo") (MOp "bar")) (MOp "baz")
+
+So it's interpreting "foo (bar or baz) like "(foo and bar) or baz"
+which is surely a bug.
+
+Fixed.
+"""]]