fix(opam): cleanly reject argumentless (and)/(or)

Otherwise they would raise a code error later.

Signed-off-by: Etienne Millon <me@emillon.org>

CHANGES.md

@@ -41,6 +41,9 @@ Unreleased
 
 - Attach melange rules to the default alias (#7926, @haochenx)
 
+- In opam constraints, reject `(and)` and `(or)` with no arguments at parse
+  time (#7730, @emillon)
+
 3.8.1 (2023-06-05)
 ------------------
 

src/dune_rules/package.ml

@@ -157,6 +157,17 @@ module Dependency = struct
       | And conjuncts -> list sexp (string "and" :: List.map ~f:encode conjuncts)
       | Or disjuncts -> list sexp (string "or" :: List.map ~f:encode disjuncts)
 
+    let logical_op t =
+      let open Dune_lang.Decoder in
+      let+ x = repeat t
+      and+ version = Syntax.get_exn Stanza.syntax
+      and+ loc = loc in
+      let empty_list_rejected_since = (3, 9) in
+      if List.is_empty x && version >= empty_list_rejected_since then
+        Syntax.Error.deleted_in loc Stanza.syntax empty_list_rejected_since
+          ~what:"Logical operators with no arguments";
+      x
+
     let decode =
       let open Dune_lang.Decoder in
       let ops =
@@ -183,10 +194,10 @@ module Dependency = struct
       fix (fun t ->
           let logops =
             [ ( "and"
-              , let+ x = repeat t in
+              , let+ x = logical_op t in
                 And x )
             ; ( "or"
-              , let+ x = repeat t in
+              , let+ x = logical_op t in
                 Or x )
             ]
           in
@@ -262,7 +273,9 @@ module Dependency = struct
     | Or [ c ] -> opam_constraint c
     | Or (c :: cs) ->
       nopos (Logop (nopos `Or, opam_constraint c, opam_constraint (And cs)))
-    | And [] | Or [] -> Code_error.raise "opam_constraint" []
+    | And [] | Or [] ->
+      User_error.raise
+        [ Pp.textf "logical operations with no arguments are not supported" ]
 
   let opam_depend { name; constraint_ } =
     let constraint_ = Option.map ~f:opam_constraint constraint_ in

test/blackbox-tests/test-cases/opam-constraints-and-or-no-args.t

@@ -0,0 +1,97 @@
+`(and)`/`(or)` with no argument should get rejected.
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.9)
+  > (generate_opam_files)
+  > (package
+  >  (name p)
+  >  (depends
+  >   (p (and))))
+  > EOF
+
+  $ dune build
+  File "dune-project", line 6, characters 5-10:
+  6 |   (p (and))))
+           ^^^^^
+  Error: Logical operators with no arguments was deleted in version 3.9 of the
+  dune language.
+  [1]
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.9)
+  > (generate_opam_files)
+  > (package
+  >  (name p)
+  >  (depends
+  >   (p (or))))
+  > EOF
+
+  $ dune build
+  File "dune-project", line 6, characters 5-9:
+  6 |   (p (or))))
+           ^^^^
+  Error: Logical operators with no arguments was deleted in version 3.9 of the
+  dune language.
+  [1]
+
+In < 3.9 they are accepted, assuming they are not used to generate opam files.
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.8)
+  > (package
+  >  (name p)
+  >  (allow_empty)
+  >  (depends
+  >   (p (and))))
+  > EOF
+
+  $ dune build
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.8)
+  > (package
+  >  (name p)
+  >  (allow_empty)
+  >  (depends
+  >   (p (or))))
+  > EOF
+
+  $ dune build
+
+Generating opam files should trigger a (nice) error:
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.8)
+  > (generate_opam_files)
+  > (package
+  >  (name p)
+  >  (depends
+  >   (p (and))))
+  > EOF
+
+  $ dune build
+  Error: logical operations with no arguments are not supported
+  -> required by _build/default/p.opam
+  -> required by _build/install/default/lib/p/opam
+  -> required by _build/default/p.install
+  -> required by alias all
+  -> required by alias default
+  [1]
+
+  $ cat > dune-project << EOF
+  > (lang dune 3.8)
+  > (generate_opam_files)
+  > (package
+  >  (name p)
+  >  (depends
+  >   (p (or))))
+  > EOF
+
+  $ dune build
+  Error: logical operations with no arguments are not supported
+  -> required by _build/default/p.opam
+  -> required by _build/install/default/lib/p/opam
+  -> required by _build/default/p.install
+  -> required by alias all
+  -> required by alias default
+  [1]

test/blackbox-tests/test-cases/opam-constraints.t

@@ -67,29 +67,3 @@ constraints.
       "@doc" {with-doc}
     ]
   ]
-
-`(and)`/`(or)` with no argument should get rejected.
-
-  $ cat > dune-project << EOF
-  > (lang dune 2.1)
-  > (generate_opam_files)
-  > (package
-  >  (name p)
-  >  (depends
-  >   (p (and))))
-  > EOF
-
-  $ dune build 2>&1 | grep 'Internal error'
-  Internal error, please report upstream including the contents of _build/log.
-
-  $ cat > dune-project << EOF
-  > (lang dune 2.1)
-  > (generate_opam_files)
-  > (package
-  >  (name p)
-  >  (depends
-  >   (p (or))))
-  > EOF
-
-  $ dune build 2>&1 | grep 'Internal error'
-  Internal error, please report upstream including the contents of _build/log.