Make the set language more future proof (#930)

- forbid list starting by an atom not starting with - or :
- allow to avoid the toplevel parentheses in dune files

Signed-off-by: Jeremie Dimino <jeremie@dimino.org>

bin/dune

@@ -1,13 +1,13 @@
 (library
  (name      main)
- (modules   (main))
+ (modules   main)
  (libraries unix dune cmdliner))
 
 (executable
  (name        main_dune)
  (public_name dune)
  (package     dune)
- (modules     (main_dune))
+ (modules     main_dune)
  (libraries   which_program_dune main)
  (preprocess  no_preprocessing))
 
@@ -15,6 +15,6 @@
  (name        main_jbuilder)
  (public_name jbuilder)
  (package     dune)
- (modules     (main_jbuilder))
+ (modules     main_jbuilder)
  (libraries   which_program_jbuilder main)
  (preprocess  no_preprocessing))

doc/jbuild.rst

@@ -764,6 +764,19 @@ syntax ``(:include <filename>)``. This is useful for instance when you need to
 run a script to figure out some compilation flags. ``<filename>`` is expected to
 contain a single S-expression and cannot contain ``(:include ...)`` forms.
 
+Note that inside an ordered set, the first element of a list cannot be
+an atom except if it starts with `-` or `:`. The reason for this is
+that we are planning to add simple programmatic features in the
+futures so that one may write:
+
+.. code::
+
+   (flags (if (>= %{ocaml_version} 4.06) ...))
+
+This restriction will allow to add this feature without introducing a
+breaking changes. If you want to write a list where the first element
+doesn't start by `-`, you can simply quote it: ``("x" y z)``.
+
 Most fields using the ordered set language also support `Variables expansion`_.
 Variables are expanded after the set language is interpreted.
 

src/ordered_set_lang.ml

@@ -28,7 +28,7 @@ module Parse = struct
 
   let generic ~inc ~elt =
     let open Stanza.Of_sexp in
-    let rec one () =
+    let rec one (kind : Stanza.File_kind.t) =
       peek_exn >>= function
       | Atom (loc, A "\\") -> Loc.fail loc "unexpected \\"
       | (Atom (_, A "") | Quoted_string (_, _)) | Template _ ->
@@ -45,19 +45,30 @@ module Parse = struct
           | _ ->
             elt >>| fun x -> Element x
         end
-      | List (_, Atom (_, A ":include") :: _) -> inc
+      | List (_, Atom (loc, A s) :: _) -> begin
-      | List _ -> enter (many [])
+          match s, kind with
-    and many acc =
+          | ":include", _ -> inc
+          | s, Dune when s <> "" && s.[0] <> '-' && s.[0] <> ':' ->
+            Loc.fail loc
+              "This atom must be quoted because it is the first element \
+               of a list and doesn't start with - or :"
+          | _ -> enter (many [] kind)
+        end
+      | List _ -> enter (many [] kind)
+    and many acc kind =
       peek >>= function
       | None -> return (Union (List.rev acc))
       | Some (Atom (_, A "\\")) ->
-        junk >>> many [] >>| fun to_remove ->
+        junk >>> many [] kind >>| fun to_remove ->
         Diff (Union (List.rev acc), to_remove)
       | Some _ ->
-        one () >>= fun x ->
+        one kind >>= fun x ->
-        many (x :: acc)
+        many (x :: acc) kind
     in
-    one ()
+    Stanza.file_kind () >>= fun kind ->
+    match kind with
+    | Dune -> many [] kind
+    | Jbuild -> one kind
 
   let with_include ~elt =
     generic ~elt ~inc:(

test/blackbox-tests/test-cases/copy_files/dune

@@ -8,12 +8,12 @@
 (library
  (name foo)
  (c_names bar)
- (modules (dummy))
+ (modules dummy)
  (wrapped false))
 
 (executable
  (name test)
- (modules (:standard \ dummy))
+ (modules :standard \ dummy)
  (libraries foo))
 
 (alias

test/blackbox-tests/test-cases/dune-ppx-driver-system/dune

@@ -2,21 +2,21 @@
 (library
  (name foo1)
  (public_name foo.1)
- (modules (foo1))
+ (modules foo1)
  (preprocess (pps)))
 
 ; Too many drivers
 (library
  (name foo2)
  (public_name foo.2)
- (modules (foo2))
+ (modules foo2)
  (preprocess (pps ppx1 ppx2)))
 
 ; Incompatible with Dune
 (library
  (name foo3)
  (public_name foo.3)
- (modules (foo3))
+ (modules foo3)
  (preprocess (pps ppx_other)))
 
 (rule (with-stdout-to foo1.ml (echo "")))
@@ -64,5 +64,5 @@
 
 (library
  (name test_ppx_args)
- (modules (test_ppx_args))
+ (modules test_ppx_args)
  (preprocess (pps -arg1 driver_print_args -arg2 -- -foo bar)))

test/blackbox-tests/test-cases/exclude-missing-module/dune

@@ -1,3 +1,3 @@
 (library
  (name foo)
- (modules (:standard \ fake)))
+ (modules :standard \ fake))

test/blackbox-tests/test-cases/exclude-missing-module/run.t

@@ -1,3 +1,3 @@
   $ dune build --display short
-  File "dune", line 3, characters 23-27:
+  File "dune", line 3, characters 22-26:
   Warning: Module Fake is excluded but it doesn't exist.

test/blackbox-tests/test-cases/gen-opam-install-file/dune

@@ -8,13 +8,13 @@
 
 (library
  (name foo_byte)
- (modules (foo_byte))
+ (modules foo_byte)
  (modes byte)
  (public_name foo.byte))
 
 (executables
  (names bar)
- (modules (bar))
+ (modules bar)
  (public_names bar)
  (libraries foo))
 
@@ -28,4 +28,4 @@
  (action (echo "%{read:foo.install}")))
 
 (documentation
- (mld_files (doc)))
+ (mld_files doc))

test/blackbox-tests/test-cases/github25/root/dune

@@ -7,5 +7,5 @@
 (library
  (name        pas_de_bol)
  (public_name pas-de-bol)
- (modules (a b))
+ (modules a b)
  (libraries plop.ca-marche-pas))

test/blackbox-tests/test-cases/github568/dune

@@ -2,7 +2,7 @@
 (library
  (name lib1)
  (public_name lib1)
- (modules (Lib1)))
+ (modules Lib1))
 
 (alias
  (name runtest)
@@ -12,14 +12,14 @@
 
 (executable
  (name test1)
- (modules (Test1))
+ (modules Test1)
  (libraries lib1))
 
 
 (library
  (name lib2)
  (public_name lib2)
- (modules (Lib2)))
+ (modules Lib2))
 
 (alias
  (name runtest)
@@ -29,5 +29,5 @@
 
 (executable
  (name test2)
- (modules (Test2))
+ (modules Test2)
  (libraries lib2))

test/blackbox-tests/test-cases/github734/src/dune

@@ -1,9 +1,9 @@
 (library
  (name foo)
- (modules (foo))
+ (modules foo)
  (c_names stubs/x))
 
 (executable
  (name bar)
- (modules (bar))
+ (modules bar)
  (libraries foo))

test/blackbox-tests/test-cases/inline_tests/dune-file/dune

@@ -11,8 +11,8 @@
     (echo "let () = print_int 42")
     (echo "\n")
     (echo "let () = print_int 43;;")))
-  (flags (inline-test-runner %{library-name}
+  (flags inline-test-runner %{library-name}
-           -source-tree-root %{ROOT} -diff-cmd -))))
+          -source-tree-root %{ROOT} -diff-cmd -)))
 
 (library
  (name foo_tests)

test/blackbox-tests/test-cases/ocamldep-multi-stanzas/dune/dune

@@ -1,6 +1,6 @@
 (library
  (name lib)
- (modules (:standard \ test)))
+ (modules :standard \ test))
 
 (executable
  (name test)

test/blackbox-tests/test-cases/odoc/dune

@@ -1,17 +1,17 @@
 (library
  (name foo)
- (modules (foo foo2))
+ (modules foo foo2)
  (wrapped false)
  (public_name foo))
 
 (library
  (name bar)
  (public_name bar)
- (modules (bar)))
+ (modules bar))
 
 (library
  (name foo_byte)
- (modules (foo_byte))
+ (modules foo_byte)
  (modes byte)
  (public_name foo.byte))
 

test/blackbox-tests/test-cases/package-dep/dune

@@ -1,13 +1,13 @@
 (library
  (name        foo)
  (public_name foo)
- (modules     (foo)))
+ (modules     foo))
 
 (library
  (name        bar)
  (public_name bar)
  (libraries   foo)
- (modules     (bar)))
+ (modules     bar))
 
 (rule
  (with-stdout-to foo.ml

test/blackbox-tests/test-cases/ppx-rewriter/jbuild

@@ -2,15 +2,15 @@
 
 (executable
  ((name w_omp_driver)
-  (modules (w_omp_driver))
+  (modules w_omp_driver)
   (preprocess (pps (fooppx -flag (-arg omp))))))
 
 (executable
  ((name w_ppx_driver)
-  (modules (w_ppx_driver))
+  (modules w_ppx_driver)
   (preprocess (pps (ppx_driver.runner)))))
 
 (executable
  ((name w_ppx_driver_flags)
-  (modules (w_ppx_driver_flags))
+  (modules w_ppx_driver_flags)
-  (preprocess (pps (fooppx -flag (-arg omp) ppx_driver.runner)))))
+  (preprocess (pps (fooppx -flag (-arg omp) ppx_driver.runner)))))

test/blackbox-tests/test-cases/private-public-overlap/exes/dune

@@ -1,9 +1,9 @@
 (library
  (name privatelib)
- (modules (privatelib)))
+ (modules privatelib))
 
 (executable
  (name publicbin)
- (modules (publicbin))
+ (modules publicbin)
  (public_name publicbin)
  (package publicbin))

test/blackbox-tests/test-cases/private-public-overlap/optional/dune

@@ -1,10 +1,10 @@
 (library
  (name privatelib)
- (modules (privatelib)))
+ (modules privatelib))
 
 (library
  (name publiclib)
  (public_name publiclib)
- (modules (publiclib))
+ (modules publiclib)
  (libraries privatelib)
  (optional))

test/blackbox-tests/test-cases/private-public-overlap/private-dep/dune

@@ -1,9 +1,9 @@
 (library
  (name privatelib)
- (modules (privatelib)))
+ (modules privatelib))
 
 (library
  (name publiclib)
  (public_name publiclib)
  (libraries privatelib)
- (modules (publiclib)))
+ (modules publiclib))

test/blackbox-tests/test-cases/private-public-overlap/private-rewriter/jbuild

@@ -9,4 +9,4 @@
  ((name mylib)
   (public_name mylib)
   (preprocess (pps (ppx_internal)))
-  (modules (mylib))))
+  (modules mylib)))

test/blackbox-tests/test-cases/private-public-overlap/private-runtime-deps/jbuild

@@ -2,7 +2,7 @@
 
 (library
  ((name private_runtime_dep)
-  (modules (private_runtime_dep))))
+  (modules private_runtime_dep)))
 
 (library
  ((name private_ppx)
@@ -14,4 +14,4 @@
  ((name mylib)
   (public_name mylib)
   (preprocess (pps (private_ppx)))
-  (modules (mylib))))
+  (modules mylib)))

test/blackbox-tests/test-cases/reason/dune

@@ -4,7 +4,7 @@
 (library
  ((name rlib)
   (public_name rlib)
-  (modules (bar cppome foo hello pped))
+  (modules bar cppome foo hello pped)
   (lint
    (per_module
     ((pps (reasonppx (-lint true))) (hello cppome))
@@ -17,7 +17,7 @@
 
 (executable
  ((name rbin)
-  (modules (rbin))
+  (modules rbin)
   (lint (action (run ./pp/reasononlypp.exe -lint %{<})))
   (preprocess (action (run ./pp/reasononlypp.exe %{<})))
   (libraries (rlib))))