add new tests and add is_line_comment boolean to lexer COMMENT

definition

Author: dxu

formatTest/unit_tests/expected_output/comments.re

@@ -1,50 +1,60 @@
 /*
  * Multiline comment
+ */
+// Empty comments, or comments with whitespaces:
+//
+//
+//
+//a
+/*inlinecommentwithnospaces*/
+/*
+
+
  */
 /*
  * 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
-        "matched thingy x";
-      let zz = 10;
-      // post line single line comment
-      zz
-    }
-  | `Other x => {
-      //  single line comment above
-      print_string "matched other x";
-      x
-    }
-  };
-
-//  single line comment below
-// short comment
+  | `Thingy x =>
+    print_string
+      /* multiline comment should be fine */
+      "matched thingy x";
+    let zz = 10;
+    // post line single line comment
+    zz
+  | `Other x =>
+    //  single line comment above
+    print_string "matched other x";
+    x
+  }
+//  single line comment below;
+
+/* 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 +63,120 @@ let a = 10;
 
 let b = 20;
 
-//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;
-
-//C
+/*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;
+
+/*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

@@ -1,6 +1,14 @@
 /*
  * Multiline comment
  */
+
+// Empty comments, or comments with whitespaces:
+//
+// 
+// 
+//a
+/*inlinecommentwithnospaces*/
+
 /*
 
 
@@ -185,5 +193,4 @@ let x = [
 ];
 
 
-
-
+// File ends with a comment

src/reason_lexer.mll

@@ -431,7 +431,7 @@ rule token = parse
         let end_loc = comment lexbuf in
         let s = get_stored_string () in
         reset_string_buffer ();
-        COMMENT (s, { start_loc with
+        COMMENT (s, true, { start_loc with
                       Location.loc_end = end_loc.Location.loc_end })
       }
   | "/*"
@@ -441,7 +441,7 @@ rule token = parse
         let end_loc = comment lexbuf in
         let s = get_stored_string () in
         reset_string_buffer ();
-        COMMENT (s, { start_loc with
+        COMMENT (s, false, { start_loc with
                       Location.loc_end = end_loc.Location.loc_end })
       }
   | "/*/"
@@ -453,7 +453,7 @@ rule token = parse
         let end_loc = comment lexbuf in
         let s = get_stored_string () in
         reset_string_buffer ();
-        COMMENT (s, { loc with Location.loc_end = end_loc.Location.loc_end })
+        COMMENT (s, false, { loc with Location.loc_end = end_loc.Location.loc_end })
       }
   | "*/"
       { let loc = Location.curr lexbuf in
@@ -646,7 +646,20 @@ and comment = parse
       { store_lexeme lexbuf; comment lexbuf }
   | eof
       { match !comment_start_loc with
-        | [] -> assert false
+        | [] ->
+          (* 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 :: _ ->
           let start = List.hd (List.rev !comment_start_loc) in
           comment_start_loc := [];
@@ -764,8 +777,8 @@ and skip_sharp_bang = parse
   let last_comments = ref []
   let rec token lexbuf =
     match token_with_comments lexbuf with
-        COMMENT (s, comment_loc) ->
-          last_comments := (s, comment_loc) :: !last_comments;
+        COMMENT (s, is_line_comment, comment_loc) ->
+          last_comments := (s, is_line_comment, comment_loc) :: !last_comments;
           token lexbuf
       | tok -> tok
   let comments () = List.rev !last_comments

src/reason_parser.mly

@@ -915,7 +915,7 @@ let built_in_explicit_arity_constructors = ["Some"; "Assert_failure"; "Match_fai
 %token WHEN
 %token WHILE
 %token WITH
-%token <string * Location.t> COMMENT
+%token <string * bool * Location.t> COMMENT
 
 %token EOL
 

src/reason_pprint_ast.ml

@@ -1221,16 +1221,14 @@ let rec extractComments comments tester =
   let open Lexing in
   (* There might be an issue here - we shouldn't have to split "up to and including".
      Up to should be sufficient. Comments' end position might be off by one (too large) *)
-  comments |> List.partition (fun (str, attLoc, physLoc) ->
+  comments |> List.partition (fun (str, isLineComment, attLoc, physLoc) ->
     let oneGreaterThanAttachmentLocEnd = attLoc.loc_end.pos_cnum in
     let attachmentLocLastChar = oneGreaterThanAttachmentLocEnd - 1 in
     let oneGreaterThanPhysLocEnd = physLoc.loc_end.pos_cnum in
     let physLastChar = oneGreaterThanPhysLocEnd - 1 in
     tester attLoc.loc_start.pos_cnum attachmentLocLastChar physLoc.loc_start.pos_cnum physLastChar
   )
 
-
-
 let space = " "
 (* Can't you tell the difference? *)
 let tab = "	"
@@ -1270,17 +1268,15 @@ let smallestLeadingSpaces strs =
   is part of a list that could potentially break to prevent inline
   comments from being rendered as line comments
 *)
-let formatItemComment ?(forceMultiline=false) (str, commLoc, physCommLoc) =
+let formatItemComment ?(forceMultiline=false) (str, is_line_comment, commLoc, physCommLoc) =
   let commLines = Str.split_delim (Str.regexp "\n") ("/*" ^ str ^ "*/") in
-  match commLines with
-  | [] -> easyAtom ""
-  | [hd] ->
-    if forceMultiline then
-      makeEasyList ~inline:(true, true) ~postSpace:true ~preSpace:true ~indent:0 ~break:IfNeed [easyAtom hd]
-    (* caveat: single line comment . *)
-    else
-      makeEasyList ~inline:(true, true) ~postSpace:true ~preSpace:true ~indent:0 ~break:IfNeed [easyAtom ("//" ^ str)]
-  | zero::one::tl ->
+  match (commLines, is_line_comment, forceMultiline) with
+  | (_, true, false) ->
+    makeEasyList ~inline:(true, true) ~postSpace:true ~preSpace:true ~indent:0 ~break:IfNeed [easyAtom ("//" ^ str)]
+  | ([], _, _) -> easyAtom ""
+  | ([hd], _, _) ->
+    makeEasyList ~inline:(true, true) ~postSpace:true ~preSpace:true ~indent:0 ~break:IfNeed [easyAtom hd]
+  | (zero::one::tl, _, _) ->
     let lineZero = List.nth commLines 0 in
     let lineOne = List.nth commLines 1 in
     let hasMeaningfulContentOnLineZero = lineZeroHasMeaningfulContent lineZero in
@@ -1321,7 +1317,7 @@ let removeSepFromListConfig listSettings =
    1. Before-line items break in unison with list items.
    2. End of line comments are placed *after* separators.  *)
 let rec interleaveComments ?endMaxChar listConfig layoutListItems comments =
-  let isDocComment (c, _, _) = String.length c > 0 && c.[0] == '*' in
+  let isDocComment (c, _, _, _) = String.length c > 0 && c.[0] == '*' in
   match (layoutListItems, endMaxChar) with
     | ([], None)-> ([], comments)
     | ([], Some endMax)->