cleanup !single_line_comment

Author: dxu

formatTest/unit_tests/expected_output/comments.re

@@ -1,21 +1,31 @@
 /*
  * Multiline comment
+ */
+// Empty comments, or comments with whitespaces:
+//
+//
+//
+//a
+/*inlinecommentwithnospaces*/
+/* This should be kept as a multiline comment */
+/*
+
+
  */
 /*
  * Multiline comment with a // single line comment
  */
 // Single line comment
-// Single line comment with a multiline /* starting
 let testPostComment = "";
 
 // let commentedCode = "";
-// Test inter-code comments
+// Test: inter-code comments
 let testMultiline a =>
   switch a {
   //  single line comment
   | `Thingy x => {
       print_string
-        // multiline comment should be fine
+        /* multiline comment should be fine */
         "matched thingy x";
       let zz = 10;
       // post line single line comment
@@ -29,22 +39,24 @@ let testMultiline a =>
   };
 
 //  single line comment below
-// short comment
+/* short comment */
 let x = ["test"];
 
-// short comment
+/* short comment */
 let x = {
   // /* */
   let y = "";
   ()
 };
 
 // /* this is a valid nested comment*/ this is a valid comment
-// valid /* this is a valid comment
+// valid /* this is also a valid nested comment */
 let z = 10;
 
+///////////////////////////////////////////////////////////////////////////////////
 // The following tests will test the conversion of /* */ to single line
 // comments as well as the wrapping of interleaved comments within short sequences.
+///////////////////////////////////////////////////////////////////////////////////
 /*
  * Test wrapping every form of named arguments where various parts are
  * commented.
@@ -53,79 +65,116 @@ let a = 10;
 
 let b = 20;
 
-//A
+/*A*/
 let named /* a::a */ a::a /* b::b */ b::b => /* a + b */ a + b;
 
-//B
-let namedAlias /* a::aa */ a::aa /* b::bb */ b::bb => /* aa + bb */ aa + bb;
+/*B*/
+let namedAlias
+    /* a::aa */
+    a::aa
+    /* b::bb */
+    b::bb => /* aa + bb */ aa + bb;
 
-//C
+/*C*/
 let namedAnnot
     /* a::(a: option int) */
     a::(a: option int)
     /* b::(b: option int) */
     b::(b: option int) =>
-  // 20
+  /* 20 */
   20;
 
-//D
+/*D*/
 let namedAliasAnnot
     /* a::(aa: option int) */
     a::(aa: option int)
     /* b::(bb: option int) */
     b::(bb: option int) =>
-  // 20
+  /* 20 */
   20;
 
-//E
-let optional /* a::a=? */ a::a=? /* b::b=? */ b::b=? /* () */ () =>
-  // 10
+/*E*/
+let optional
+    /* a::a=? */
+    a::a=?
+    /* b::b=? */
+    b::b=?
+    /* () */
+    () =>
+  /* 10 */
   10;
 
-//F
-let optionalAlias /* a::aa */ a::aa=? /* ?b:bb */ b::bb=? /* () */ () =>
-  // 10
+/*F*/
+let optionalAlias
+    /* a::aa */
+    a::aa=?
+    /* ?b:bb */
+    b::bb=?
+    /* () */
+    () =>
+  /* 10 */
   10;
 
-//G
+/*G*/
 let optionalAnnot
     /* a::(a: option int)=? */
     a::(a: option int)=?
     /* ?b:(b: option int) */
     b::(b: option int)=?
     /* () */
     () =>
-  // 10
+  /* 10 */
   10;
 
-//H
+/*H*/
 let optionalAliasAnnot
     /* a::(aa: option int)=? */
     a::(aa: option int)=?
     /* b::(bb: option int)=? */
     b::(bb: option int)=?
     /* () => */
     () =>
-  // 10
+  /* 10 */
   10;
 
-//I: This one is really annoying? Where's the visual label?
-let defOptional /* a::a=10 */ a::a=10 /* b::b=10 */ b::b=10 /* () => */ () =>
-  // 10
+/*I: This one is really annoying? Where's the visual label?*/
+let defOptional
+    /* a::a=10 */
+    a::a=10
+    /* b::b=10 */
+    b::b=10
+    /* () => */
+    () =>
+  /* 10 */
   10;
 
-//J
-let defOptionalAlias /* a::aa=10 */ a::aa=10 /* b::bb=10 */ b::bb=10 /* () => */ () =>
-  // 10;
+/*J*/
+let defOptionalAlias
+    /* a::aa=10 */
+    a::aa=10
+    /* b::bb=10 */
+    b::bb=10
+    /* () => */
+    () =>
+  /* 10; */
   10;
 
-//K
+/*K*/
 let defOptionalAnnot
     /* a::(a:int)=10 */
     a::(a: int)=10
     /* b::(b:int)=10 */
     b::(b: int)=10
     /* () => */
     () =>
-  // 10;
+  /* 10; */
   10;
+
+// This tests a short inline comment that should retain it's inline properties when formatted
+let x = [/* sh */ "te"];
+
+// This tests an empty /* */ nested comment
+// This tests a line comment that should be converted to an inline comment when formatted
+let x = [/* sh*/ "te"];
+
+// File ends with a comment

formatTest/unit_tests/input/comments.re

@@ -9,6 +9,7 @@
 //a
 /*inlinecommentwithnospaces*/
 
+/* This should be kept as a multiline comment */
 /*
 
 

src/reason_lexer.mll

@@ -166,8 +166,7 @@ let get_stored_string () =
 (* To store the position of the beginning of a string and comment *)
 let string_start_loc = ref Location.none;;
 let comment_start_loc = ref [];;
-let line_comment_start_loc = ref [];;
-let single_line_comment = ref false;;
+let line_comment_start_loc = ref None;;
 let in_comment () = !comment_start_loc <> [];;
 let is_in_string = ref false
 let in_string () = !is_in_string
@@ -424,8 +423,7 @@ rule token = parse
       }
   | "//"
       { let start_loc = Location.curr lexbuf  in
-        single_line_comment := true;
-        line_comment_start_loc := [start_loc];
+        line_comment_start_loc := Some start_loc;
         reset_string_buffer ();
         let end_loc = comment lexbuf in
         let s = get_stored_string () in
@@ -569,23 +567,24 @@ and comment = parse
       }
   | "*/"
       {
-        match (!comment_start_loc, !single_line_comment) with
-        | ([], false) -> 
+        match (!comment_start_loc, !line_comment_start_loc) with
+        | ([], None) -> 
           assert false
-        | ([], true) ->
-          let loc = Location.curr lexbuf in
-          let start = List.hd (List.rev !line_comment_start_loc) in
-          raise (Error (Unmatched_nested_comment loc, start))
-        | ([_], true) ->
-          comment_start_loc := [];
-          store_lexeme lexbuf;
-          comment lexbuf;
-        | ([_], false) ->
+        | ([_], None) ->
           comment_start_loc := []; Location.curr lexbuf
-        | (_ :: l, _) -> 
+        | (_ :: l, None) -> 
           comment_start_loc := l;
           store_lexeme lexbuf;
           comment lexbuf;
+        (* line comment *)
+        | ([], Some start_loc) ->
+          let loc = Location.curr lexbuf in
+          raise (Error (Unmatched_nested_comment loc, start_loc))
+        (* a multiline comment nested within a line comment *)
+        | (_, Some _) ->
+          comment_start_loc := [];
+          store_lexeme lexbuf;
+          comment lexbuf;
        }
   | "\""
       {
@@ -644,48 +643,39 @@ and comment = parse
   | "'\\" 'x' ['0'-'9' 'a'-'f' 'A'-'F'] ['0'-'9' 'a'-'f' 'A'-'F'] "'"
       { store_lexeme lexbuf; comment lexbuf }
   | eof
-      { match !comment_start_loc with
-        | [] ->
-          (* if the file ends with a single line comment then  *)
-          if !single_line_comment then (
-            single_line_comment := false;
-            match (!comment_start_loc, !line_comment_start_loc) with
-              | ([], []) -> 
-                assert false
-              | ([], _) ->
-                line_comment_start_loc := []; Location.curr lexbuf
-              | (_, _) ->
-                let start = List.hd (List.rev !comment_start_loc) in
-                comment_start_loc := [];
-                raise (Error (Unmatched_nested_comment start, Location.curr lexbuf))
-          ) else assert false
-        | loc :: _ ->
+      { match (!comment_start_loc, !line_comment_start_loc) with
+        | ([], None) -> assert false
+        | (loc :: _, None) ->
           let start = List.hd (List.rev !comment_start_loc) in
           comment_start_loc := [];
           raise (Error (Unterminated_comment start, loc))
+        (* line comment ended with no nested multilines *)
+        | ([], Some _) ->
+          line_comment_start_loc := None; Location.curr lexbuf
+        (* line comment ends with an unfinished multiline *)
+        | (_, Some _) ->
+          let start = List.hd (List.rev !comment_start_loc) in
+          comment_start_loc := [];
+          raise (Error (Unmatched_nested_comment start, Location.curr lexbuf))
       }
   | newline
       {
-        if not !single_line_comment then (
-          update_loc lexbuf None 1 false 0;
-          store_lexeme lexbuf;
-          comment lexbuf
-        )
-        else (
           update_loc lexbuf None 1 false 0;
           (* check if there are any unmatched nested comments *)
 
-          single_line_comment := false;
           match (!comment_start_loc, !line_comment_start_loc) with
-            | ([], []) -> 
+            | ([], None) -> 
               assert false
-            | ([], _) ->
-              line_comment_start_loc := []; Location.curr lexbuf
-            | (_, _) ->
+            | (_, None) ->
+              update_loc lexbuf None 1 false 0;
+              store_lexeme lexbuf;
+              comment lexbuf
+            | ([], Some _) ->
+              line_comment_start_loc := None; Location.curr lexbuf
+            | (_, Some _) ->
               let start = List.hd (List.rev !comment_start_loc) in
               comment_start_loc := [];
               raise (Error (Unmatched_nested_comment start, Location.curr lexbuf))
-        )
       }
   | _
       { store_lexeme lexbuf; comment lexbuf }

src/reason_toolchain.ml

@@ -129,9 +129,12 @@ module Create_parse_entrypoint (Toolchain_impl: Toolchain_spec) :Toolchain = str
         | _ ->
           let modified_comments =
             List.map (fun (str, is_line_comment, loc) ->
-              let comment_length = (loc.loc_end.pos_cnum - loc.loc_start.pos_cnum - 4) in
-              let original_comment_contents = String.sub !chan_input (loc.loc_start.pos_cnum + 2) comment_length in
-              (original_comment_contents, is_line_comment, loc)
+              if not is_line_comment then
+                let comment_length = (loc.loc_end.pos_cnum - loc.loc_start.pos_cnum - 4) in
+                let original_comment_contents = String.sub !chan_input (loc.loc_start.pos_cnum + 2) comment_length in
+                (original_comment_contents, is_line_comment, loc)
+              else
+                (str, is_line_comment, loc)
             )
             unmodified_comments
           in