Merge pull request #342 from Leonidas-from-XIV/add-warning-for-double-semicolons

Add warning for phrases that are missing trailing ;;

Author: Leonidas-from-XIV

CHANGES.md

@@ -8,6 +8,8 @@
 
 #### Deprecated
 
+- Add a deprecation warning for toplevel blocks that are not terminated with `;;` (#342, @Leonidas-from-XIV)
+
 #### Fixed
 
 - Fix accidental redirect of stderr to stdout (#343, @Leonidas-from-XIV)

lib/deprecated.ml

@@ -23,3 +23,31 @@ let warn ?replacement s ~since =
   Format.eprintf
     "Warning: %s is deprecated since %s and will be removed in 2.0.0. %s\n%!" s
     since replacement
+
+module Missing_double_semicolon = struct
+  let missing_semicolon = ref false
+
+  let check_toplevel : Toplevel.t -> unit =
+   fun toplevel ->
+    match List.rev toplevel.command with
+    | cmd :: _ ->
+        let ends_with_semi =
+          cmd |> Astring.String.trim |> Astring.String.is_suffix ~affix:";;"
+        in
+        if not ends_with_semi then missing_semicolon := true
+    | [] -> ()
+
+  let check_block block = List.iter check_toplevel block
+
+  let report ~filename =
+    if !missing_semicolon then
+      Format.eprintf
+        "Warning: OCaml toplevel block without trailing ;; detected in file \
+         '%s'.\n\
+         Non-semicolon terminated phrases are deprecated.\n\
+         MDX 2.0 will accept them as input but will output them with ;; \
+         appended.\n\
+         In MDX 3.0 support for toplevel blocks without ;; will be removed \
+         completely.\n"
+        filename
+end

lib/deprecated.mli

@@ -16,3 +16,9 @@
 
 val warn : ?replacement:string -> string -> since:string -> unit
 (** Emits a warning on the standard error. *)
+
+module Missing_double_semicolon : sig
+  val check_block : Toplevel.t list -> unit
+
+  val report : filename:string -> unit
+end

lib/test/mdx_test.ml

@@ -287,7 +287,7 @@ let run_exn ~non_deterministic ~silent_eval ~record_backtrace ~syntax ~silent
 
   let test_block ~ppf ~temp_file t =
     let print_block () = Block.pp ?syntax ppf t in
-    if Block.is_active ?section t then
+    if Block.is_active ?section t then (
       match Block.value t with
       | Raw _ -> print_block ()
       | Include { file_included; file_kind = Fk_ocaml { part_included } } ->
@@ -317,30 +317,31 @@ let run_exn ~non_deterministic ~silent_eval ~record_backtrace ~syntax ~silent
             ~det:(fun () ->
               run_cram_tests ?syntax t ?root ppf temp_file pad tests)
       | Toplevel { non_det; env } ->
-          let tests =
+          let phrases =
             let syntax = Util.Option.value syntax ~default:Normal in
             Toplevel.of_lines ~syntax ~loc:t.loc t.contents
           in
+          Deprecated.Missing_double_semicolon.check_block phrases;
           with_non_det non_deterministic non_det ~command:print_block
             ~output:(fun () ->
               assert (syntax <> Some Cram);
               print_block ();
               List.iter
-                (fun (test : Toplevel.t) ->
+                (fun (phrase : Toplevel.t) ->
                   match
                     Mdx_top.in_env env (fun () ->
-                        eval_test ~block:t ?root c test.command)
+                        eval_test ~block:t ?root c phrase.command)
                   with
                   | Ok _ -> ()
                   | Error e ->
                       let output = List.map (fun l -> `Output l) e in
-                      if Output.equal test.output output then ()
-                      else err_eval ~cmd:test.command e)
-                tests)
+                      if Output.equal phrase.output output then ()
+                      else err_eval ~cmd:phrase.command e)
+                phrases)
             ~det:(fun () ->
               assert (syntax <> Some Cram);
               Mdx_top.in_env env (fun () ->
-                  run_toplevel_tests ?syntax ?root c ppf tests t))
+                  run_toplevel_tests ?syntax ?root c ppf phrases t)))
     else print_block ()
   in
   let gen_corrected file_contents items =
@@ -364,6 +365,7 @@ let run_exn ~non_deterministic ~silent_eval ~record_backtrace ~syntax ~silent
             try test_block ~ppf ~temp_file t
             with Failure msg -> raise (Test_block_failure (t, msg))))
       items;
+    Deprecated.Missing_double_semicolon.report ~filename:file;
     Format.pp_print_flush ppf ();
     Buffer.contents buf
   in

test/bin/mdx-pp/expect/spaces/test-case.md

@@ -14,15 +14,15 @@ let x =
 
 # let x = 1
 
-  in x
+  in x;;
 - : int = 1
 
 
-# 3
+# 3;;
 - : int = 3
 
 
-# Printf.printf "foo\n\nbar\n"
+# Printf.printf "foo\n\nbar\n";;
 foo
 
 bar

test/bin/mdx-pp/expect/spaces/test_case.ml.expected

@@ -9,9 +9,8 @@ let x =
 #15 "test-case.md"
   let x = 1
 
-  in x
+  in x;;
 #21 "test-case.md"
-  3
+  3;;
 #25 "test-case.md"
-  Printf.printf "foo\n\nbar\n"
-;;
+  Printf.printf "foo\n\nbar\n";;

test/bin/mdx-test/expect/code/test-case.md

@@ -11,9 +11,9 @@ let () = Printf.printf "n: %d\n%!" (f 42)
 Yo!
 
 ```ocaml
-# let x = 3
+# let x = 3;;
 val x : int = 3
-# type t = int
+# type t = int;;
 type t = int
 ```
 
@@ -22,12 +22,12 @@ class istack = object end
 ```
 
 ```ocaml
-# module type Foo = sig type t end
+# module type Foo = sig type t end;;
 module type Foo = sig type t end
 ```
 
 
 ```ocaml skip
-# Pipe.f ()
+# Pipe.f ();;
 - : unit
 ```

test/bin/mdx-test/expect/ellipsis/test-case.md

@@ -10,7 +10,7 @@ $ for i in `seq 1 10`; do echo $i; done
 ```
 
 ```ocaml
-# for i = 1 to 10 do Printf.printf "%d\n%!" i; done
+# for i = 1 to 10 do Printf.printf "%d\n%!" i; done;;
 1
 2
 ...

test/bin/mdx-test/expect/empty-line/test-case.md

@@ -1,8 +1,8 @@
 Testing empty line output
 
 ```ocaml
-# print_newline ()
+# print_newline ();;
 - : unit = ()
-# print_endline ""
+# print_endline "";;
 - : unit = ()
 ```

test/bin/mdx-test/expect/empty-lines/test-case.md

@@ -8,6 +8,6 @@ This shell block contains an empty line within a padded block:
 This toplevel block contains an empty line within a padded block:
 
 ```ocaml
- # print_newline
+ # print_newline;;
 
 ```