From 0574b6fd002941783eab35c60564bea457e0f2bd Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 10 Jan 2025 17:17:04 +0100 Subject: [PATCH 1/5] SLVS-1600 CaYC: cleanup touched files on save --- .../XamlGenerator/DiffTranslatorTests.cs | 8 ++++---- src/Education/XamlGenerator/DiffTranslator.cs | 1 - src/Education/XamlGenerator/RuleHelpXamlTranslator.cs | 11 ++--------- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs b/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs index b5daf3131c..16f4adf14c 100644 --- a/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs +++ b/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using FluentAssertions; -using Microsoft.VisualStudio.TestTools.UnitTesting; using SonarLint.VisualStudio.Education.XamlGenerator; using SonarLint.VisualStudio.TestInfrastructure; @@ -123,8 +121,10 @@ public void GetDiffXaml_LineHasNoSubPieces_StillCorrectResult() (string result1, string result2) = testSubject.GetDiffXaml(input1, input2); - result1.Should().BeEquivalentTo("\n } \n if\n (\n condition2\n ) {\n\n"); - result2.Should().BeEquivalentTo("\n }\n\nif (condition2) {"); + result1.Should().BeEquivalentTo( + "\n } \n if\n (\n condition2\n ) {\n\n"); + result2.Should().BeEquivalentTo( + "\n }\n\nif (condition2) {"); } private static DiffTranslator CreateTestSubject(IXamlWriterFactory xamlWriterFactory = null) diff --git a/src/Education/XamlGenerator/DiffTranslator.cs b/src/Education/XamlGenerator/DiffTranslator.cs index 9d2ddc5555..67b6f5111f 100644 --- a/src/Education/XamlGenerator/DiffTranslator.cs +++ b/src/Education/XamlGenerator/DiffTranslator.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Generic; using System.ComponentModel.Composition; using System.Text; using System.Xml; diff --git a/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs b/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs index 25f0f3543e..3b94d734f0 100644 --- a/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs +++ b/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs @@ -18,10 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Collections.Generic; using System.ComponentModel.Composition; -using System.Diagnostics; using System.IO; using System.Text; using System.Web; @@ -387,11 +384,7 @@ private void ProcessElement() private static XmlReader CreateXmlReader(string data) { - var settings = new XmlReaderSettings - { - ConformanceLevel = ConformanceLevel.Fragment, - IgnoreWhitespace = false - }; + var settings = new XmlReaderSettings { ConformanceLevel = ConformanceLevel.Fragment, IgnoreWhitespace = false }; var stream = new StringReader(data); return XmlReader.Create(stream, settings); @@ -420,7 +413,7 @@ private void WriteText(string text) } Debug.Assert(textToken is IRuleCrossRef || textToken is ISimpleText, - $"Unknown text token type. Type: {textToken.GetType().FullName}"); + $"Unknown text token type. Type: {textToken.GetType().FullName}"); } } From eed465fb0ce56d43f5788780b330c2661c50fedf Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 10 Jan 2025 17:28:20 +0100 Subject: [PATCH 2/5] SLVS-1600 Group all consecutive changes sub-pieces in the same span This is needed to prevent html escape characters (e.g. ') which end up in different sup-pieces to be split into different spans (because they won't be rendered correctly in the UI) --- .../XamlGenerator/DiffTranslatorTests.cs | 22 ++++++++++- src/Education/XamlGenerator/DiffTranslator.cs | 37 +++++++++++++++++-- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs b/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs index 16f4adf14c..b8c74ce7e1 100644 --- a/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs +++ b/src/Education.UnitTests/XamlGenerator/DiffTranslatorTests.cs @@ -121,12 +121,30 @@ public void GetDiffXaml_LineHasNoSubPieces_StillCorrectResult() (string result1, string result2) = testSubject.GetDiffXaml(input1, input2); - result1.Should().BeEquivalentTo( - "\n } \n if\n (\n condition2\n ) {\n\n"); + result1.Should().BeEquivalentTo("\n } if (condition2) {\n\n"); result2.Should().BeEquivalentTo( "\n }\n\nif (condition2) {"); } + /// + /// Making sure that all consecutive changed sub-pieces are grouped together is needed to ensure that html escape characters (e.g. ') + /// that end up in different sup-pieces will be rendered correctly in the UI + /// + [TestMethod] + public void GetDiffXaml_LineHasHasConsecutiveChangedSubPieces_GroupsConsecutiveSubPiecesInOneSpan() + { + var input1 = "var a; // Noncompliant var b=2;"; + var input2 = "var a; "; + + var testSubject = CreateTestSubject(); + + (string result1, string result2) = testSubject.GetDiffXaml(input1, input2); + + result1.Should().BeEquivalentTo( + "var a; // Noncompliant var b=2;"); + result2.Should().BeEquivalentTo("var a; "); + } + private static DiffTranslator CreateTestSubject(IXamlWriterFactory xamlWriterFactory = null) { xamlWriterFactory ??= new XamlWriterFactory(); diff --git a/src/Education/XamlGenerator/DiffTranslator.cs b/src/Education/XamlGenerator/DiffTranslator.cs index 67b6f5111f..53eee4517b 100644 --- a/src/Education/XamlGenerator/DiffTranslator.cs +++ b/src/Education/XamlGenerator/DiffTranslator.cs @@ -68,7 +68,10 @@ private string HighlightLines(List lines, StyleResourceNames style, S { var line = lines[i]; - if (string.IsNullOrEmpty(line.Text)) continue; + if (string.IsNullOrEmpty(line.Text)) + { + continue; + } if (line.Type != ChangeType.Unchanged) { @@ -99,7 +102,7 @@ private string HighlightLines(List lines, StyleResourceNames style, S return sb.ToString(); } - private void WriteSubPieces(XmlWriter writer, StyleResourceNames style, DiffPiece line) + private static void WriteSubPieces(XmlWriter writer, StyleResourceNames style, DiffPiece line) { if (line.SubPieces.Count == 0) { @@ -107,15 +110,29 @@ private void WriteSubPieces(XmlWriter writer, StyleResourceNames style, DiffPiec return; } - foreach (var subPiece in line.SubPieces) + for (var i = 0; i < line.SubPieces.Count; i++) { - if (string.IsNullOrEmpty(subPiece.Text)) continue; + var subPiece = line.SubPieces[i]; + if (IsSubPieceEmpty(subPiece)) + { + continue; + } if (subPiece.Type != ChangeType.Unchanged) { writer.WriteStartElement("Span"); writer.ApplyStyleToElement(style); writer.WriteString(subPiece.Text); + // group all consecutive changed sub-pieces in the same span + // this is needed to prevent html escape characters (e.g. ') which end up in different sup-pieces to be split into different spans, + // which then won't be rendered correctly in the UI + while (IsNextSubPieceChanged(line, i)) + { + i++; + var nextSupPiece = line.SubPieces[i]; + writer.WriteString(nextSupPiece.Text); + } + writer.WriteEndElement(); } else @@ -124,5 +141,17 @@ private void WriteSubPieces(XmlWriter writer, StyleResourceNames style, DiffPiec } } } + + private static bool IsSubPieceEmpty(DiffPiece subPiece) => string.IsNullOrEmpty(subPiece.Text); + + private static bool IsNextSubPieceChanged(DiffPiece line, int currentPosition) + { + if (currentPosition >= line.SubPieces.Count - 1) + { + return false; + } + var nextSupPiece = line.SubPieces[currentPosition + 1]; + return !IsSubPieceEmpty(nextSupPiece) && nextSupPiece.Type != ChangeType.Unchanged; + } } } From 1304fab6e50537db4535c09806be31bd5e6e4138 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 10 Jan 2025 18:04:43 +0100 Subject: [PATCH 3/5] SLVS-1600 Fix single quotation marks in diff view are not displayed correctly in the rule help description. This was causes by the fact that the single quotation mark was both html encoded and escaped by the XmlWriter. Which means that when & from the html escape character ' was escaped by the XmlWriter, it ended up to be amp;#39;, which couldn't be interpreted as the single quote character --- .../RuleHelpXamlTranslatorTests.cs | 37 ++++++++++++++++--- src/Education/XamlGenerator/DiffTranslator.cs | 19 +++++++--- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs index edcab3d350..7dce4fbb47 100644 --- a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs +++ b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs @@ -19,8 +19,6 @@ */ using System.Text; -using FluentAssertions; -using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using SonarLint.VisualStudio.Education.XamlGenerator; using SonarLint.VisualStudio.TestInfrastructure; @@ -34,7 +32,7 @@ public class RuleHelpXamlTranslatorTests public void Factory_MefCtor_CheckExports() { MefTestHelpers.CheckTypeCanBeImported - (MefTestHelpers.CreateExport(), + (MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); } @@ -220,7 +218,8 @@ public void TranslateHtmlToXaml_TwoCompliantCode_DoesNotHighlightCode() diffTranslator.Setup(d => d.GetDiffXaml(nonCompliantText1, compliantText)).Returns((noncompliantXaml, compliantXaml)); - var htmlText = $"
{compliantText}
\n
{nonCompliantText1}
\n
{nonCompliantText2}
"; + var htmlText + = $"
{compliantText}
\n
{nonCompliantText1}
\n
{nonCompliantText2}
"; var expectedText = @"
Same text 1 @@ -270,7 +269,8 @@ public void TranslateHtmlToXaml_TwoDiffs_HighlightsCodeCorrectly() diffTranslator.Setup(d => d.GetDiffXaml(noncompliantText1, compliantText1)).Returns((noncompliantXaml1, compliantXaml1)); diffTranslator.Setup(d => d.GetDiffXaml(noncompliantText2, compliantText2)).Returns((noncompliantXaml2, compliantXaml2)); - var htmlText = $"
{compliantText1}
{compliantText2}
{noncompliantText1}
{noncompliantText2}
"; + var htmlText + = $"
{compliantText1}
{compliantText2}
{noncompliantText1}
{noncompliantText2}
"; var expectedText = @"
Same text 1 @@ -379,6 +379,33 @@ public void TranslateHtmlToXaml_DataDiffWithAngleBracket_XMLParsable() result.Replace("\r\n", "\n").Should().Be(expectedText.Replace("\r\n", "\n")); } + [TestMethod] + public void TranslateHtmlToXaml_SingleQuotesPresentInDiffCode_AreEscapedCorrectly() + { + var testSubject = CreateTestSubject(); + + var htmlText = @" +
+    function f(a, g){} // Noncompliant: 'f' returns 'b' on two different return statements
+
+
+function f(a, g){}
+
+"; + + var expectedText + = @" +
function f(a, g){} // Noncompliant: 'f' returns 'b' on two different return statements +
+
function f(a, g){} +
+"; + + var result = testSubject.TranslateHtmlToXaml(htmlText); + + result.Should().Be(expectedText); + } + private static IRuleHelpXamlTranslator CreateTestSubject(IXamlWriterFactory xamlWriterFactory = null, IDiffTranslator diffTranslator = null) { xamlWriterFactory ??= new XamlWriterFactory(); diff --git a/src/Education/XamlGenerator/DiffTranslator.cs b/src/Education/XamlGenerator/DiffTranslator.cs index 53eee4517b..4d7edacfa0 100644 --- a/src/Education/XamlGenerator/DiffTranslator.cs +++ b/src/Education/XamlGenerator/DiffTranslator.cs @@ -84,7 +84,7 @@ private string HighlightLines(List lines, StyleResourceNames style, S } else { - writer.WriteString(line.Text); + writer.WriteRaw(line.Text); } // The differ removes the next line element as it returns a list of lines. @@ -106,7 +106,7 @@ private static void WriteSubPieces(XmlWriter writer, StyleResourceNames style, D { if (line.SubPieces.Count == 0) { - writer.WriteString(line.Text); + writer.WriteRaw(line.Text); return; } @@ -122,7 +122,7 @@ private static void WriteSubPieces(XmlWriter writer, StyleResourceNames style, D { writer.WriteStartElement("Span"); writer.ApplyStyleToElement(style); - writer.WriteString(subPiece.Text); + WriteSubPiece(writer, subPiece); // group all consecutive changed sub-pieces in the same span // this is needed to prevent html escape characters (e.g. ') which end up in different sup-pieces to be split into different spans, // which then won't be rendered correctly in the UI @@ -130,18 +130,27 @@ private static void WriteSubPieces(XmlWriter writer, StyleResourceNames style, D { i++; var nextSupPiece = line.SubPieces[i]; - writer.WriteString(nextSupPiece.Text); + WriteSubPiece(writer, nextSupPiece); } writer.WriteEndElement(); } else { - writer.WriteString(subPiece.Text); + WriteSubPiece(writer, subPiece); } } } + /// + /// When writing a sub-piece, it's important to use the WriteRaw method to avoid escaping the special characters. + /// The string is expected to already be html encoded, which means that when special characters (e.g. &) are escaped in html escape characters (e.g. '), + /// we end up with invalid characters that can't be interpreted (e.g. amp;#39;). + /// + /// + /// + private static void WriteSubPiece(XmlWriter writer, DiffPiece subPiece) => writer.WriteRaw(subPiece.Text); + private static bool IsSubPieceEmpty(DiffPiece subPiece) => string.IsNullOrEmpty(subPiece.Text); private static bool IsNextSubPieceChanged(DiffPiece line, int currentPosition) From d5f4c21c2614f4779115f4a1f74d08d2bb5eb206 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 10 Jan 2025 18:37:32 +0100 Subject: [PATCH 4/5] SLVS-1600 Move html encoding inside DiffTranslator to have more control on the received html content and for better testability (e.g. html encoding and XML writer encoding sometimes lead to invalid characters, when XmlWriter encodes special characters from html escape characters) --- .../XamlGenerator/RuleHelpXamlTranslatorTests.cs | 8 ++------ src/Education/XamlGenerator/DiffTranslator.cs | 5 ++++- src/Education/XamlGenerator/RuleHelpXamlTranslator.cs | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs index 7dce4fbb47..e9f2e4f0f3 100644 --- a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs +++ b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs @@ -354,9 +354,7 @@ same 1 [TestMethod] public void TranslateHtmlToXaml_DataDiffWithAngleBracket_XMLParsable() { - var diffTranslator = new Mock(); - - IRuleHelpXamlTranslator testSubject = CreateTestSubject(diffTranslator: diffTranslator.Object); + IRuleHelpXamlTranslator testSubject = CreateTestSubject(); var compliantText = "#include <vector>"; var nonCompliantText = "#include <vector>"; @@ -365,8 +363,6 @@ public void TranslateHtmlToXaml_DataDiffWithAngleBracket_XMLParsable() var noncompliantXaml = @"#include <vector>"; - diffTranslator.Setup(d => d.GetDiffXaml(compliantText, nonCompliantText)).Returns((noncompliantXaml, compliantXaml)); - var htmlText = $"
{compliantText}
\n
{nonCompliantText}
"; var expectedText = @"
@@ -386,7 +382,7 @@ public void TranslateHtmlToXaml_SingleQuotesPresentInDiffCode_AreEscapedCorrectl var htmlText = @" 
-    function f(a, g){} // Noncompliant: 'f' returns 'b' on two different return statements
+function f(a, g){} // Noncompliant: 'f' returns 'b' on two different return statements
 
 function f(a, g){}
diff --git a/src/Education/XamlGenerator/DiffTranslator.cs b/src/Education/XamlGenerator/DiffTranslator.cs
index 4d7edacfa0..607099d9ed 100644
--- a/src/Education/XamlGenerator/DiffTranslator.cs
+++ b/src/Education/XamlGenerator/DiffTranslator.cs
@@ -20,6 +20,7 @@
 
 using System.ComponentModel.Composition;
 using System.Text;
+using System.Web;
 using System.Xml;
 using DiffPlex.DiffBuilder;
 using DiffPlex.DiffBuilder.Model;
@@ -51,7 +52,9 @@ public DiffTranslator(IXamlWriterFactory xamlWriterFactory)
 
         public (string noncompliantXaml, string compliantXaml) GetDiffXaml(string noncompliantHtml, string compliantHtml)
         {
-            var resultDiff = SideBySideDiffBuilder.Diff(oldText: noncompliantHtml, newText: compliantHtml, ignoreWhiteSpace: false);
+            var encodedNonCompliantHtml = HttpUtility.HtmlEncode(noncompliantHtml);
+            var encodedCompliantHtml = HttpUtility.HtmlEncode(compliantHtml);
+            var resultDiff = SideBySideDiffBuilder.Diff(oldText: encodedNonCompliantHtml, newText: encodedCompliantHtml, ignoreWhiteSpace: false);
 
             var highlightedNonCompliant = HighlightLines(resultDiff.OldText.Lines, StyleResourceNames.NonCompliant_Diff, StyleResourceNames.Sub_NonCompliant_Diff);
             var highlightedCompliant = HighlightLines(resultDiff.NewText.Lines, StyleResourceNames.Compliant_Diff, StyleResourceNames.Sub_Compliant_Diff);
diff --git a/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs b/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs
index 3b94d734f0..1f22cd3df3 100644
--- a/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs
+++ b/src/Education/XamlGenerator/RuleHelpXamlTranslator.cs
@@ -601,7 +601,7 @@ private void ReplaceDiffs(StringBuilder sb)
                         continue;
                     }
 
-                    var diffXaml = diffTranslator.GetDiffXaml(HttpUtility.HtmlEncode(diffCodes[noncompliantKey]), HttpUtility.HtmlEncode(diffCodes[compliantKey]));
+                    var diffXaml = diffTranslator.GetDiffXaml(diffCodes[noncompliantKey], diffCodes[compliantKey]);
 
                     sb.Replace(noncompliantKey, diffXaml.noncompliantXaml);
                     sb.Replace(compliantKey, diffXaml.compliantXaml);

From 47ada12fec4fabf3bf0d38b8878668fb8a38bd09 Mon Sep 17 00:00:00 2001
From: Gabriela Trutan 
Date: Fri, 10 Jan 2025 18:57:06 +0100
Subject: [PATCH 5/5] SLVS-1600 Remove no longer used variables

---
 .../XamlGenerator/RuleHelpXamlTranslatorTests.cs          | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs
index e9f2e4f0f3..d4c2d431d5 100644
--- a/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs
+++ b/src/Education.UnitTests/XamlGenerator/RuleHelpXamlTranslatorTests.cs
@@ -354,15 +354,9 @@ same 1
         [TestMethod]
         public void TranslateHtmlToXaml_DataDiffWithAngleBracket_XMLParsable()
         {
-            IRuleHelpXamlTranslator testSubject = CreateTestSubject();
-
+            var testSubject = CreateTestSubject();
             var compliantText = "#include <vector>";
             var nonCompliantText = "#include <vector>";
-
-            var compliantXaml = @"#include <vector>";
-
-            var noncompliantXaml = @"#include <vector>";
-
             var htmlText = $"
{compliantText}
\n
{nonCompliantText}
";
 
             var expectedText = @"