Improve conversion of quoted strings from Reason. (rescript-lang#238)

* Improve conversion of quoted strings from Reason. Fixes rescript-lang/syntax#237 The conversion from Reason to ReScript naively picks up `//` as comments inside quoted strings.. input: ```reason let x = {j|https://www.apple.com|j}; let x = {|https://www.apple.com|}; let x = {js|https://www.apple.com|js}; ``` output: ```rescript let x = j`https://www.apple.com` //www.apple.com|j}; let x = `https://www.apple.com` //www.apple.com|}; let x = `https://www.apple.com` //www.apple.com|js}; ``` This patch fixes this behaviour and doesn't produce any extra comments * Update CHANGELOG Co-authored-by: Iwan <[email protected]>
kevinbarabash · Jan 17, 2021 · 80a5df1 · 80a5df1
1 parent 4b97d08
commit 80a5df1
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 0 deletions.
diff --git a/syntax/CHANGELOG.md b/syntax/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Unreleased
 
+* Improve conversion of quoted strings from Reason in [#238](https://github.com/rescript-lang/syntax/pull/238)
 * Print attributes/extension without bs prefix where possible in [#230](https://github.com/rescript-lang/syntax/pull/230)
 * Cleanup gentype attribute printing [fe05e1051aa94b16f6993ddc5ba9651f89e86907](https://github.com/rescript-lang/syntax/commit/fe05e1051aa94b16f6993ddc5ba9651f89e86907)
 * Implement light weight syntax for poly-variants [f84c5760b3f743f65e934195c87fc06bf88bff75](https://github.com/rescript-lang/syntax/commit/f84c5760b3f743f65e934195c87fc06bf88bff75)

diff --git a/syntax/src/res_driver_reason_binary.ml b/syntax/src/res_driver_reason_binary.ml
@@ -30,6 +30,11 @@ let extractConcreteSyntax filename =
       let len = endPos.pos_cnum - startPos.pos_cnum in
       let txt = (String.sub [@doesNotRaise]) src startPos.pos_cnum len in
       stringData := (txt, loc)::(!stringData);
+      next endPos scanner;
+    | Lbrace ->
+      (* handle {| |} or {sql||sql} quoted strings. We don't care about its contents.
+         Why? // abcdef inside the quoted string would otherwise be picked up as an extra comment *)
+      Res_scanner.tryAdvanceQuotedString scanner;
       next endPos scanner
     | _ ->
       next endPos scanner

diff --git a/syntax/src/res_scanner.ml b/syntax/src/res_scanner.ml
@@ -735,3 +735,53 @@ let isBinaryOp src startCnum endCnum =
       c == CharacterCodes.eof
     in
     leftOk && rightOk
+
+(* Assume `{` consumed, advances the scanner towards the ends of Reason quoted strings. (for conversion)
+ * In {| foo bar |} the scanner will be advanced until after the `|}` *)
+let tryAdvanceQuotedString scanner =
+  let rec scanContents tag () =
+    if scanner.ch == CharacterCodes.eof then (
+      ()
+    ) else if scanner.ch == CharacterCodes.bar then (
+      next scanner;
+      if CharacterCodes.Lower.a <= scanner.ch && scanner.ch <= CharacterCodes.Lower.z then (
+        let startOff = scanner.offset in
+        while CharacterCodes.Lower.a <= scanner.ch && scanner.ch <= CharacterCodes.Lower.z do
+          next scanner
+        done;
+        let suffix = Bytes.sub_string scanner.src startOff (scanner.offset - startOff) in
+        if tag = suffix then (
+          if scanner.ch = CharacterCodes.rbrace then
+            next scanner
+          else
+            scanContents tag ()
+        ) else
+          scanContents tag ()
+      ) else if CharacterCodes.rbrace = scanner.ch then (
+        next scanner
+      ) else (
+        scanContents tag ()
+      )
+    ) else (
+      if CharacterCodes.isLineBreak scanner.ch then (
+        scanner.lineOffset <- scanner.offset + 1;
+        scanner.lnum <- scanner.lnum + 1;
+      );
+      next scanner;
+      scanContents tag ()
+    )
+  in
+  if CharacterCodes.Lower.a <= scanner.ch && scanner.ch <= CharacterCodes.Lower.z then (
+    let startOff = scanner.offset in
+    while CharacterCodes.Lower.a <= scanner.ch && scanner.ch <= CharacterCodes.Lower.z do
+      next scanner
+    done;
+    let tag = Bytes.sub_string scanner.src startOff (scanner.offset - startOff) in
+    if scanner.ch = CharacterCodes.bar then
+      scanContents tag ()
+    else
+      ()
+  ) else if scanner.ch = CharacterCodes.bar then
+    scanContents "" ()
+  else
+    ()
diff --git a/syntax/src/res_scanner.mli b/syntax/src/res_scanner.mli
@@ -30,3 +30,5 @@ val popMode: t -> mode -> unit
 val reconsiderLessThan: t -> Res_token.t
 
 val scanTemplateLiteralToken: t -> (Lexing.position * Lexing.position * Res_token.t)
+
+val tryAdvanceQuotedString: t -> unit
diff --git a/syntax/tests/conversion/reason/__snapshots__/render.spec.js.snap b/syntax/tests/conversion/reason/__snapshots__/render.spec.js.snap
@@ -1341,6 +1341,22 @@ let x = \\"\\\\\\"\\"
 let y = \\"\\\\n\\"
 
 (<> {\\"\\\\n\\"->React.string} </>)
+
+// The \`//\` should not result into an extra comment
+let x = j\`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = sql\`https://www.apple.com\`
+
+// /* */ should not result in an extra comments
+let x = j\`/* https://www.apple.com */\`
+let x = \`/* https://www.apple.com*/\`
+let x = \`/*https://www.apple.com*/\`
+let x = \`/*https://www.apple.com*/\`
+let x = sql\`/*https://www.apple.com*/\`
+
+let x = \`\\\\\`https://\\\\\${appleWebsite}\\\\\`\`
 "
 `;
 

diff --git a/syntax/tests/conversion/reason/string.re b/syntax/tests/conversion/reason/string.re
@@ -11,3 +11,19 @@ let x = "\"";
 let y = "\n";
 
 <> {"\n"->React.string} </>;
+
+// The `//` should not result into an extra comment
+let x = {j|https://www.apple.com|j};
+let x = {|https://www.apple.com|};
+let x = {js|https://www.apple.com|js};
+let x = {|https://www.apple.com|};
+let x = {sql|https://www.apple.com|sql};
+
+// /* */ should not result in an extra comments
+let x = {j|/* https://www.apple.com */|j};
+let x = {|/* https://www.apple.com*/|};
+let x = {js|/*https://www.apple.com*/|js};
+let x = {|/*https://www.apple.com*/|};
+let x = {sql|/*https://www.apple.com*/|sql};
+
+let x = {js|`https://${appleWebsite}`|js};
diff --git a/syntax/tests/printer/other/__snapshots__/render.spec.js.snap b/syntax/tests/printer/other/__snapshots__/render.spec.js.snap
@@ -518,6 +518,23 @@ let x = \\"foo\\\\x0Abar\\"
 let x = \\"foo\\\\o012bar\\"
 
 let x = \\"😁 this works now 😆\\"
+let x = \`😁 this works now 😆\`
+
+/* The \`//\` should not result into an extra comment */
+let x = j\`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = \`https://www.apple.com\`
+let x = sql\`https://www.apple.com\`
+
+/* /* */ should not result in an extra comments */
+let x = j\`/* https://www.apple.com */\`
+let x = \`/* https://www.apple.com*/\`
+let x = \`/*https://www.apple.com*/\`
+let x = \`/*https://www.apple.com*/\`
+let x = sql\`/*https://www.apple.com*/\`
+
+let x = \`\\\\\`https://\\\\\${appleWebsite}\\\\\`\`
 "
 `;
 

diff --git a/syntax/tests/printer/other/ocamlString.ml b/syntax/tests/printer/other/ocamlString.ml
@@ -7,3 +7,21 @@ let x = "foo\x0Abar"
 let x = "foo\o012bar"
 
 let x = "😁 this works now 😆"
+let x = {|😁 this works now 😆|}
+
+
+(* The `//` should not result into an extra comment *)
+let x = {j|https://www.apple.com|j}
+let x = {|https://www.apple.com|}
+let x = {js|https://www.apple.com|js}
+let x = {|https://www.apple.com|}
+let x = {sql|https://www.apple.com|sql}
+
+(* /* */ should not result in an extra comments *)
+let x = {j|/* https://www.apple.com */|j}
+let x = {|/* https://www.apple.com*/|}
+let x = {js|/*https://www.apple.com*/|js}
+let x = {|/*https://www.apple.com*/|}
+let x = {sql|/*https://www.apple.com*/|sql}
+
+let x = {js|`https://${appleWebsite}`|js}