Fix fable-compiler#403: Unescaped Chars in XML Comments (2nd try) (fa…

…ble-compiler#408)

Previous fix escaped chars before converting markdown/jsdoc into xml.
That included a test for xml tag (-> simple summary doesn't need a
`<summary>` tag -> doesn't need escaping).
In Combination with an incorrect test, that resulted in `<` and `&` not
beeing escaped most of the time.

Additional: `>` gets escaped too. `&lt;tag>` just looks to strange. But
disadvantage: Start of Markdown Quote (`> ...`) or arrow (`->`) looks
strange in xml doc comment code.

src/syntax.fs

@@ -122,6 +122,11 @@ module FsComment =
     let isSummary v = match v with | FsComment.Summary _ -> true | _ -> false
     let asSummary v = match v with | FsComment.Summary o -> Some o | _ -> None
 
+    let justSummary comments =
+        match comments with
+        | [ FsComment.Summary s ] -> Some s
+        | _ -> None
+
     /// Checks if passed xml comment text contains xml comment tags
     let containsXml (text: string) =
         //cannot test for just < or > -> might not be xml tags like `the return value should be Option<string>`

src/transformComments.fs

@@ -227,7 +227,7 @@ module private Text =
         text
             .Replace("&", "&")
             .Replace("<", "&lt;")
-            // .Replace(">", "&gt;")
+            .Replace(">", "&gt;")
             // .Replace("\"", "&quot;")
             // .Replace("'", "&apos;")
 
@@ -515,12 +515,9 @@ let private transformTags (comments: FsComment list): FsComment list =
 
 /// MUST be called before transformation of text into xml tags!
 let private escapeXmlChars (comments: FsComment list): FsComment list =
-    // don't escape when only summary without any xml tags (-> no xml tags)
-    match comments with
-    | [ FsComment.Summary lines ] when lines |> List.forall (not << FsComment.containsXml) ->
-        // no escape necessary: not inside xml (`<summary>`)
-        comments
-    | _ ->
+    match () with
+    | Markdown -> comments
+    | Xml ->
         let escape = List.map Text.escapeXmlChars
         comments
         |> List.map (
@@ -664,8 +661,26 @@ let transform (f: FsFile): FsFile =
         comments
         |> mergeSummaries
         |> transformTags
-        |> escapeXmlChars
-        |> transformContent
+        |> fun comments ->
+            // don't escape when only summary without any xml tags (-> no xml tags)
+            // BUT: markdown & jsdoc aren't yet converted into xml -> unknown if there are xml tags
+            // BUT: escape xml char must happen before conversion to xml, because otherwise those xml tags would be escaped too
+            match comments |> FsComment.justSummary with
+            | Some _ ->
+                let transformed = comments |> transformContent
+                match transformed |> FsComment.justSummary with
+                | Some lines when lines |> List.forall (not << FsComment.containsXml) ->
+                    // just summary without any doc xml
+                    transformed
+                | _ ->
+                    // escape necessary
+                    comments
+                    |> escapeXmlChars
+                    |> transformContent
+            | _ ->
+                comments
+                |> escapeXmlChars
+                |> transformContent
 
     let fix ns =
         function

test/fragments/custom/comments/transformToXml.expected.fs

@@ -6,7 +6,7 @@ open Fable.Core.JS
 
 
 /// <summary>
-/// Testing all text transformations (links & code)
+/// Testing all text transformations (links &amp; code)
 /// 
 /// Simple link: <see href="https://github.com/fable-compiler/ts2fable" />
 /// 
@@ -35,7 +35,7 @@ open Fable.Core.JS
 /// * href: <see href="https://github.com/fable-compiler/ts2fable">ts2fable</see> and <see href="https://github.com">GitHub</see> 
 /// * cref: <see cref="B">Thingy B</see> and <see cref="B">Thingy B</see>
 /// 
-/// CRef with <c>#</c> (instance member) and <c>~</c> (inner member) -> convert all into <c>.</c>:
+/// CRef with <c>#</c> (instance member) and <c>~</c> (inner member) -&gt; convert all into <c>.</c>:
 /// <c>Namespace.Class.method</c> = <see cref="Namespace.Class.method" />
 /// <c>Namespace.Class#method</c> = <see cref="Namespace.Class.method" />
 /// <c>Namespace.Class~method</c> = <see cref="Namespace.Class.method" />
@@ -78,7 +78,7 @@ type [<AllowNullLiteral>] AllTextTransformations =
 type [<AllowNullLiteral>] AllTags =
     interface end
 
-/// <summary>Testing <c>@see</c> tag (-> <c>&lt;seealso...</c>)</summary>
+/// <summary>Testing <c>@see</c> tag (-&gt; <c>&lt;seealso...</c>)</summary>
 /// <seealso cref="SomeType1">should be cref</seealso>
 /// <seealso cref="Module.SomeType2">should be cref</seealso>
 /// <seealso href="https://github.com/fable-compiler/ts2fable">should be href</seealso>
@@ -113,7 +113,7 @@ type [<AllowNullLiteral>] SeeTag =
 type [<AllowNullLiteral>] Separator =
     interface end
 
-/// <summary>Exception Type in <c>@throws</c> is optional, but required in <c>&lt;exception></c></summary>
+/// <summary>Exception Type in <c>@throws</c> is optional, but required in <c>&lt;exception&gt;</c></summary>
 /// <exception cref="SomeException"> exception with type</exception>
 /// <exception cref="Module.SomeException"> exception with type in module</exception>
 /// <exception cref="">exception without type</exception>

test/fragments/regressions/#393-mutable-variables-become-immutable.expected.fs

@@ -7,41 +7,40 @@ open Fable.Core.JS
 let [<Import("M","#393-mutable-variables-become-immutable")>] m: M.IExports = jsNative
 let [<Import("N","#393-mutable-variables-become-immutable")>] n: N.IExports = jsNative
 /// <summary>
-/// <c>var</c> -> mutable 
+/// <c>var</c> -&gt; mutable 
 /// 
 /// BUT: 
-/// > error FS0874: Mutable 'let' bindings can't be recursive or defined in recursive modules or namespaces
-/// -> keep immutable
+/// &gt; error FS0874: Mutable 'let' bindings can't be recursive or defined in recursive modules or namespaces
+/// -&gt; keep immutable
 /// </summary>
 let [<Import("v2","#393-mutable-variables-become-immutable")>] v2: float = jsNative
 /// <summary>
-/// <c>let</c> -> mutable 
+/// <c>let</c> -&gt; mutable 
 /// 
 /// BUT: 
-/// > error FS0874: Mutable 'let' bindings can't be recursive or defined in recursive modules or namespaces
-/// -> keep immutable
+/// &gt; error FS0874: Mutable 'let' bindings can't be recursive or defined in recursive modules or namespaces
+/// -&gt; keep immutable
 /// </summary>
 let [<Import("l1","#393-mutable-variables-become-immutable")>] l1: float = jsNative
-/// <summary><c>const</c> -> immutable</summary>
+/// <summary><c>const</c> -&gt; immutable</summary>
 let [<Import("c1","#393-mutable-variables-become-immutable")>] c1: float = jsNative
 
-
 module M =
 
     type [<AllowNullLiteral>] IExports =
-        /// <summary><c>var</c> -> mutable</summary>
+        /// <summary><c>var</c> -&gt; mutable</summary>
         abstract v2: float with get, set
-        /// <summary><c>let</c> -> mutable</summary>
+        /// <summary><c>let</c> -&gt; mutable</summary>
         abstract l1: float with get, set
-        /// <summary><c>const</c> -> immutable</summary>
+        /// <summary><c>const</c> -&gt; immutable</summary>
         abstract c1: float
 
 module N =
 
     type [<AllowNullLiteral>] IExports =
-        /// <summary><c>var</c> -> mutable</summary>
+        /// <summary><c>var</c> -&gt; mutable</summary>
         abstract v2: float with get, set
-        /// <summary><c>let</c> -> mutable</summary>
+        /// <summary><c>let</c> -&gt; mutable</summary>
         abstract l1: float with get, set
-        /// <summary><c>const</c> -> immutable</summary>
+        /// <summary><c>const</c> -&gt; immutable</summary>
         abstract c1: float

test/fragments/regressions/#403-xml-comment-escape-chars.d.ts

@@ -2,7 +2,7 @@
  * Chars to escape: & <
  * Chars that might be reasonable to escape, but not needed: > ' "
  * 
- * In code environment: <c> & < > ' " </c>
+ * In code environment: ` & < > ' " `
  * 
  * In link: [search fsharp & ts2fable](https://duckduckgo.com/?q=fsharp+ts2fable&ia=web)
  */
@@ -26,4 +26,14 @@ export interface I3 { }
  * 
  * @deprecated Ok: &; not "ok"!
  */
-export interface I4 { }
+export interface I4 { }
+
+/**
+ * A & and link: [link](target)
+ */
+export interface I5 { }
+
+/**
+ * `And` in code: `&`
+ */
+export interface I6 { }

test/fragments/regressions/#403-xml-comment-escape-chars.expected.fs

@@ -7,9 +7,9 @@ open Fable.Core.JS
 
 /// <summary>
 /// Chars to escape: &amp; &lt;
-/// Chars that might be reasonable to escape, but not needed: > ' "
+/// Chars that might be reasonable to escape, but not needed: &gt; ' "
 /// 
-/// In code environment: &lt;c> &amp; &lt; > ' " &lt;/c>
+/// In code environment: <c> &amp; &lt; &gt; ' " </c>
 /// 
 /// In link: <see href="https://duckduckgo.com/?q=fsharp+ts2fable&amp;ia=web">search fsharp &amp; ts2fable</see>
 /// </summary>
@@ -23,7 +23,7 @@ type [<AllowNullLiteral>] I2 =
     interface end
 
 /// <summary>Escape chars here: some extra tag</summary>
-/// <remarks>Remarks: Stuff &amp; Stuff: &amp; &lt; > ' "</remarks>
+/// <remarks>Remarks: Stuff &amp; Stuff: &amp; &lt; &gt; ' "</remarks>
 type [<AllowNullLiteral>] I3 =
     interface end
 
@@ -33,3 +33,11 @@ type [<AllowNullLiteral>] I3 =
 [<Obsolete("Ok: &; not \"ok\"!")>]
 type [<AllowNullLiteral>] I4 =
     interface end
+
+/// <summary>A &amp; and link: <see cref="target">link</see></summary>
+type [<AllowNullLiteral>] I5 =
+    interface end
+
+/// <summary><c>And</c> in code: <c>&amp;</c></summary>
+type [<AllowNullLiteral>] I6 =
+    interface end