From cbf3d3b235dda1d94b076917026fa3fac3ff4fcb Mon Sep 17 00:00:00 2001 From: axunonb Date: Sat, 10 Apr 2021 23:23:46 +0200 Subject: [PATCH] Fix parsing issues (#152) * Reference to issues #148, #147, #143 * Updated README.md * Fix for https://github.com/axuno/SmartFormat/pull/149#issuecomment-816622584 * Update CHANGES.md --- CHANGES.md | 6 + src/SmartFormat.Tests/Core/ParserTests.cs | 233 +++++++++++++++--- .../SmartFormat.Tests.csproj | 9 +- src/SmartFormat/Core/Parsing/LiteralText.cs | 1 + src/SmartFormat/Core/Parsing/Parser.cs | 68 +++-- src/SmartFormat/Core/Parsing/ParsingErrors.cs | 22 +- 6 files changed, 252 insertions(+), 87 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 037798c4..c2569dcf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,12 @@ Latest Changes ==== +v2.7.0 +=== +* **Fixed** broken backward compatibilty introduced in v2.6.2 (issues referenced in [#148](https://github.com/axuno/SmartFormat/issues/148), [#147](https://github.com/axuno/SmartFormat/issues/147), [#143](https://github.com/axuno/SmartFormat/issues/143)). +* **Fixed**: Take an erroneous format string like `"this is {uncomplete"` (missing closing brace). Before v2.7.0 the parser handled `{uncomplete` as a `TextLiteral`, not as an erroneous `Placeholder`. +* **Fixed**: Since v1.6.1 there was an undiscovered issue: If the `Parser` encountered a `ParsingError.TooManyClosingBraces`, this closing brace was simply "swallowed-up". This way, the result with `Parser.ErrorAction.MaintainTokens` differs from the original format string. From v2.7.0, the redundant closing brace is handled as a `TextLiteral`. + v2.6.2 === * Fix: Fully implemented all `Settings.ParseErrorAction`, see [#143](https://github.com/axuno/SmartFormat/pull/143) - Thanks to [Anders Jonsson](https://github.com/andersjonsson) diff --git a/src/SmartFormat.Tests/Core/ParserTests.cs b/src/SmartFormat.Tests/Core/ParserTests.cs index 2638b06f..5d2ef691 100644 --- a/src/SmartFormat.Tests/Core/ParserTests.cs +++ b/src/SmartFormat.Tests/Core/ParserTests.cs @@ -14,7 +14,7 @@ public class ParserTests [Test] public void TestParser() { - var parser = new SmartFormatter() {Settings = { ParseErrorAction = ErrorAction.ThrowError}}.Parser; + var parser = new SmartFormatter {Settings = { ParseErrorAction = ErrorAction.ThrowError}}.Parser; parser.AddAlphanumericSelectors(); parser.AddAdditionalSelectorChars("_"); parser.AddOperators("."); @@ -35,32 +35,26 @@ public void TestParser() results.TryAll(r => Assert.AreEqual(r.format, r.parsed.ToString())).ThrowIfNotEmpty(); } - [Test] - public void Parser_Throws_Exceptions() + [TestCase("{")] + [TestCase("{0")] + [TestCase("}")] + [TestCase("0}")] + [TestCase("{{{")] + [TestCase("}}}")] + [TestCase("{.}")] + [TestCase("{.:}")] + [TestCase("{..}")] + [TestCase("{..:}")] + [TestCase("{0.}")] + [TestCase("{0.:}")] + public void Parser_Throws_Exceptions(string format) { // Let's set the "ErrorAction" to "Throw": var formatter = Smart.CreateDefaultSmartFormat(); formatter.Settings.ParseErrorAction = ErrorAction.ThrowError; var args = new object[] { TestFactory.GetPerson() }; - var invalidFormats = new[] { - "{", - "{0", - "}", - "0}", - "{{{", - "}}}", - "{.}", - "{.:}", - "{..}", - "{..:}", - "{0.}", - "{0.:}", - }; - foreach (var format in invalidFormats) - { - Assert.Throws(() => formatter.Test(format, args, "Error")); - } + Assert.Throws(() => formatter.Test(format, args, "Error")); } [Test] @@ -119,42 +113,201 @@ public void Parser_Ignores_Exceptions() [Test] public void Parser_Error_Action_Ignore() { - var invalidTemplate = "Hello, I'm {Name from {City}"; + // | Literal | Erroneous | | Okay | + var invalidTemplate = "Hello, I'm {Name from {City} {Street}"; var smart = Smart.CreateDefaultSmartFormat(); smart.Settings.ParseErrorAction = ErrorAction.Ignore; + + var parser = GetRegularParser(); + parser.Settings.ParseErrorAction = ErrorAction.Ignore; + var parsed = parser.ParseFormat(invalidTemplate, new[] { Guid.NewGuid().ToString("N") }); + + Assert.That(parsed.Items.Count, Is.EqualTo(4), "Number of parsed items"); + Assert.That(parsed.Items[0].RawText, Is.EqualTo("Hello, I'm "), "Literal text"); + Assert.That(parsed.Items[1].RawText, Is.EqualTo(string.Empty), "Erroneous placeholder"); + Assert.That(parsed.Items[2].RawText, Is.EqualTo(" ")); + Assert.That(parsed.Items[3], Is.TypeOf(typeof(Placeholder))); + Assert.That(parsed.Items[3].RawText, Does.Contain("{Street}"), "Correct placeholder"); + } - var result = smart.Format(invalidTemplate, new { Name = "John", City = "Oklahoma" }); - - Assert.AreEqual(string.Empty, result); + // | Literal | Erroneous | | Okay | + // Hello, I'm {Name from {City} {Street} + [TestCase("Hello, I'm {Name from {City} {Street}", true)] + // | Literal | Erroneous | | Erroneous + // Hello, I'm {Name from {City} {Street + [TestCase("Hello, I'm {Name from {City} {Street", false)] + public void Parser_Error_Action_MaintainTokens(string invalidTemplate, bool lastItemIsPlaceholder) + { + var parser = GetRegularParser(); + parser.Settings.ParseErrorAction = ErrorAction.MaintainTokens; + var parsed = parser.ParseFormat(invalidTemplate, new[] { Guid.NewGuid().ToString("N") }); + + Assert.That(parsed.Items.Count, Is.EqualTo(4), "Number of parsed items"); + Assert.That(parsed.Items[0].RawText, Is.EqualTo("Hello, I'm ")); + Assert.That(parsed.Items[1].RawText, Is.EqualTo("{Name from {City}")); + Assert.That(parsed.Items[2].RawText, Is.EqualTo(" ")); + if (lastItemIsPlaceholder) + { + Assert.That(parsed.Items[3], Is.TypeOf(typeof(Placeholder)), "Last item should be Placeholder"); + Assert.That(parsed.Items[3].RawText, Does.Contain("{Street}")); + } + else + { + Assert.That(parsed.Items[3], Is.TypeOf(typeof(LiteralText)), "Last item should be LiteralText"); + Assert.That(parsed.Items[3].RawText, Does.Contain("{Street")); + } } [Test] - public void Parser_Error_Action_MaintainTokens() + public void Parser_Error_Action_OutputErrorInResult() { + // | Literal | Erroneous | var invalidTemplate = "Hello, I'm {Name from {City}"; + + var parser = GetRegularParser(); + parser.Settings.ParseErrorAction = ErrorAction.OutputErrorInResult; + var parsed = parser.ParseFormat(invalidTemplate, new[] { Guid.NewGuid().ToString("N") }); - var smart = Smart.CreateDefaultSmartFormat(); - smart.Settings.ParseErrorAction = ErrorAction.MaintainTokens; + Assert.That(parsed.Items.Count, Is.EqualTo(1)); + Assert.That(parsed.Items[0].RawText, Does.StartWith("The format string has 3 issues")); + } - var result = smart.Format(invalidTemplate, new { Name = "John", City = "Oklahoma" }); + /// + /// SmartFormat is not designed for processing JavaScript because of interfering usage of {}[]. + /// This example shows that even a comment can lead to parsing will work or not. + /// + [TestCase("/* The comment with this '}{' makes it fail */", "############### {TheVariable} ###############", false)] + [TestCase("", "############### {TheVariable} ###############", true)] + public void Parse_JavaScript_May_Succeed_Or_Fail(string var0, string var1, bool shouldSucceed) + { + var js = @" +(function(exports) { + 'use strict'; + /** + * Searches for specific element in a given array using + * the interpolation search algorithm.

+ * Time complexity: O(log log N) when elements are uniformly + * distributed, and O(N) in the worst case + * + * @example + * + * var search = require('path-to-algorithms/src/searching/'+ + * 'interpolation-search').interpolationSearch; + * console.log(search([1, 2, 3, 4, 5], 4)); // 3 + * + * @public + * @module searching/interpolation-search + * @param {Array} sortedArray Input array. + * @param {Number} seekIndex of the element which index should be found. + * @returns {Number} Index of the element or -1 if not found. + */ + function interpolationSearch(sortedArray, seekIndex) { + let leftIndex = 0; + let rightIndex = sortedArray.length - 1; + + while (leftIndex <= rightIndex) { + const rangeDiff = sortedArray[rightIndex] - sortedArray[leftIndex]; + const indexDiff = rightIndex - leftIndex; + const valueDiff = seekIndex - sortedArray[leftIndex]; + + if (valueDiff < 0) { + return -1; + } + + if (!rangeDiff) { + return sortedArray[leftIndex] === seekIndex ? leftIndex : -1; + } + + const middleIndex = + leftIndex + Math.floor((valueDiff * indexDiff) / rangeDiff); + + if (sortedArray[middleIndex] === seekIndex) { + return middleIndex; + } + + if (sortedArray[middleIndex] < seekIndex) { + leftIndex = middleIndex + 1; + } else { + rightIndex = middleIndex - 1; + } + } + " + var0 + @" + /* " + var1 + @" */ + return -1; + } + exports.interpolationSearch = interpolationSearch; +})(typeof window === 'undefined' ? module.exports : window); +"; + var parser = GetRegularParser(); + parser.Settings.ParseErrorAction = ErrorAction.MaintainTokens; + var parsed = parser.ParseFormat(js, new[] { Guid.NewGuid().ToString("N") }); - Assert.AreEqual("Hello, I'm {Name from {City}", result); - } + // No characters should get lost compared to the format string, + // no matter if a Placeholder can be identified or not + Assert.That(parsed.Items.Sum(i => i.RawText.Length), Is.EqualTo(js.Length), "No characters lost"); - [Test] - public void Parser_Error_Action_OutputErrorInResult() + if (shouldSucceed) + { + Assert.That(parsed.Items.Count(i => i.GetType() == typeof(Placeholder)), Is.EqualTo(1), + "One placeholders"); + Assert.That(parsed.Items.First(i => i.GetType() == typeof(Placeholder)).RawText, + Is.EqualTo("{TheVariable}")); + } + else + { + Assert.That(parsed.Items.Count(i => i.GetType() == typeof(Placeholder)), Is.EqualTo(0), + "NO placeholder"); + } + } + + /// + /// SmartFormat is not designed for processing CSS because of interfering usage of {}[]. + /// This example shows that even a comment can lead to parsing will work or not. + /// + [TestCase("", "############### {TheVariable} ###############", false)] + [TestCase("/* This '}' in the comment makes it succeed */", "############### {TheVariable} ###############", true)] + public void Parse_Css_May_Succeed_Or_Fail(string var0, string var1, bool shouldSucceed) { - var invalidTemplate = "Hello, I'm {Name from {City}"; + var css = @" +.media { + display: grid; + grid-template-columns: 1fr 3fr; +} - var smart = Smart.CreateDefaultSmartFormat(); - smart.Settings.ParseErrorAction = ErrorAction.OutputErrorInResult; +.media .content { + font-size: .8rem; +} + +.comment img { + border: 1px solid grey; " + var0 + @" + anything: '" + var1 + @"' +} + +.list-item { + border-bottom: 1px solid grey; +} +"; + var parser = GetRegularParser(); + parser.Settings.ParseErrorAction = ErrorAction.MaintainTokens; + var parsed = parser.ParseFormat(css, new[] { Guid.NewGuid().ToString("N") }); - var result = smart.Format(invalidTemplate, new { Name = "John", City = "Oklahoma" }); + // No characters should get lost compared to the format string, + // no matter if a Placeholder can be identified or not + Assert.That(parsed.Items.Sum(i => i.RawText.Length), Is.EqualTo(css.Length), "No characters lost"); - Assert.IsTrue( - result.StartsWith("The format string has") - ); + if (shouldSucceed) + { + Assert.That(parsed.Items.Count(i => i.GetType() == typeof(Placeholder)), Is.EqualTo(1), + "One placeholders"); + Assert.That(parsed.Items.First(i => i.GetType() == typeof(Placeholder)).RawText, + Is.EqualTo("{TheVariable}")); + } + else + { + Assert.That(parsed.Items.Count(i => i.GetType() == typeof(Placeholder)), Is.EqualTo(0), + "NO placeholder"); + } } [Test] diff --git a/src/SmartFormat.Tests/SmartFormat.Tests.csproj b/src/SmartFormat.Tests/SmartFormat.Tests.csproj index 6ee5a2ab..8fc00cb8 100644 --- a/src/SmartFormat.Tests/SmartFormat.Tests.csproj +++ b/src/SmartFormat.Tests/SmartFormat.Tests.csproj @@ -3,14 +3,15 @@ Unit tests for SmartFormat SmartFormat.Test - net462;net5.0 + axuno gGmbH, Scott Rippey, Bernhard Millauer and other contributors. + net462;net5.0 $(DefineConstants) false SmartFormat.Tests ../SmartFormat/SmartFormat.snk - false + false true - false + false enable @@ -23,7 +24,7 @@ - + diff --git a/src/SmartFormat/Core/Parsing/LiteralText.cs b/src/SmartFormat/Core/Parsing/LiteralText.cs index 971f751d..ff5450a1 100644 --- a/src/SmartFormat/Core/Parsing/LiteralText.cs +++ b/src/SmartFormat/Core/Parsing/LiteralText.cs @@ -33,6 +33,7 @@ public override string ToString() private string ConvertCharacterLiteralsToUnicode() { var source = baseString.Substring(startIndex, endIndex - startIndex); + if (source.Length == 0) return source; // No character literal escaping - nothing to do if (source[0] != Parser.CharLiteralEscapeChar) diff --git a/src/SmartFormat/Core/Parsing/Parser.cs b/src/SmartFormat/Core/Parsing/Parser.cs index 48ee39e7..679ccaa6 100644 --- a/src/SmartFormat/Core/Parsing/Parser.cs +++ b/src/SmartFormat/Core/Parsing/Parser.cs @@ -243,6 +243,8 @@ public Format ParseFormat(string format, string[] formatterExtensionNames) // Make sure that this is a nested placeholder before we un-nest it: if (current.parent == null) { + // Don't swallow-up redundant closing braces, but treat them as literals + current.Items.Add(new LiteralText(Settings, current, i) {endIndex = i + 1}); parsingErrors.AddIssue(current, parsingErrorText[ParsingError.TooManyClosingBraces], i, i + 1); continue; @@ -399,7 +401,7 @@ public Format ParseFormat(string format, string[] formatterExtensionNames) currentPlaceholder.Selectors.Add(new Selector(Settings, format, lastI, i, operatorIndex, selectorIndex)); else if (operatorIndex != i) - parsingErrors.AddIssue(current, parsingErrorText[ParsingError.TrailingOperatorsInSelector], + parsingErrors.AddIssue(current, $"'0x{Convert.ToByte(c):X}': " + parsingErrorText[ParsingError.TrailingOperatorsInSelector], operatorIndex, i); lastI = i + 1; @@ -418,7 +420,7 @@ public Format ParseFormat(string format, string[] formatterExtensionNames) currentPlaceholder.Selectors.Add(new Selector(Settings, format, lastI, i, operatorIndex, selectorIndex)); else if (operatorIndex != i) - parsingErrors.AddIssue(current, parsingErrorText[ParsingError.TrailingOperatorsInSelector], + parsingErrors.AddIssue(current, $"'0x{Convert.ToByte(c):X}': " + parsingErrorText[ParsingError.TrailingOperatorsInSelector], operatorIndex, i); lastI = i + 1; @@ -437,27 +439,32 @@ public Format ParseFormat(string format, string[] formatterExtensionNames) || _alphanumericSelectors && ('a' <= c && c <= 'z' || 'A' <= c && c <= 'Z') || _allowedSelectorChars.IndexOf(c) != -1; if (!isValidSelectorChar) - parsingErrors.AddIssue(current, parsingErrorText[ParsingError.InvalidCharactersInSelector], + parsingErrors.AddIssue(current, $"'0x{Convert.ToByte(c):X}': " + parsingErrorText[ParsingError.InvalidCharactersInSelector], i, i + 1); } } } - // finish the last text item: - if (lastI != format.Length) - current.Items.Add(new LiteralText(Settings, current, lastI) {endIndex = format.Length}); - - // Check that the format is finished: + // We're at the end of the input string + + // 1. Is the last item a placeholder, that is not finished yet? if (current.parent != null || currentPlaceholder != null) { parsingErrors.AddIssue(current, parsingErrorText[ParsingError.MissingClosingBrace], format.Length, format.Length); current.endIndex = format.Length; - while (current.parent != null) - { - current = current.parent.parent; - current.endIndex = format.Length; - } + } + else if (lastI != format.Length) + { + // 2. The last item must be a literal, so add it + current.Items.Add(new LiteralText(Settings, current, lastI) {endIndex = format.Length}); + } + + // Todo v2.7.0: There is no unit test for this condition! + while (current.parent != null) + { + current = current.parent.parent; + current.endIndex = format.Length; } // Check for any parsing errors: @@ -533,12 +540,11 @@ internal ParsingErrorText() } /// - /// Handles s as defined in , - /// which leads to results similar to s + /// Handles s as defined in . /// /// /// - /// The which will be further processed with formatting. + /// The which will be further processed by the formatter. private Format HandleParsingErrors(ParsingErrors parsingErrors, Format currentResult) { switch (Settings.ParseErrorAction) @@ -546,23 +552,35 @@ private Format HandleParsingErrors(ParsingErrors parsingErrors, Format currentRe case ErrorAction.ThrowError: throw parsingErrors; case ErrorAction.MaintainTokens: - var fmt = new Format(Settings, currentResult.baseString) { - startIndex = 0, - endIndex = currentResult.baseString.Length - }; - fmt.Items.Add(new LiteralText(Settings, fmt)); - return fmt; + // Replace erroneous Placeholders with tokens as LiteralText + // Placeholder without issues are left unmodified + for (var i = 0; i < currentResult.Items.Count; i++) + { + if (currentResult.Items[i] is Placeholder ph && parsingErrors.Issues.Any(errItem => errItem.Index >= currentResult.Items[i].startIndex && errItem.Index <= currentResult.Items[i].endIndex)) + { + currentResult.Items[i] = new LiteralText(Settings, ph.Format ?? new Format(Settings, ph.baseString), ph.startIndex){endIndex = ph.endIndex}; + } + } + return currentResult; case ErrorAction.Ignore: - return new Format(Settings, string.Empty); + // Replace erroneous Placeholders with an empty LiteralText + for (var i = 0; i < currentResult.Items.Count; i++) + { + if (currentResult.Items[i] is Placeholder ph && parsingErrors.Issues.Any(errItem => errItem.Index >= currentResult.Items[i].startIndex && errItem.Index <= currentResult.Items[i].endIndex)) + { + currentResult.Items[i] = new LiteralText(Settings, ph.Format ?? new Format(Settings, ph.baseString), ph.startIndex){endIndex = ph.startIndex}; + } + } + return currentResult; case ErrorAction.OutputErrorInResult: - fmt = new Format(Settings, parsingErrors.Message) { + var fmt = new Format(Settings, parsingErrors.Message) { startIndex = 0, endIndex = parsingErrors.Message.Length }; fmt.Items.Add(new LiteralText(Settings, fmt)); return fmt; default: - return currentResult; + throw new ArgumentException("Illegal type for ParsingErrors", parsingErrors); } } diff --git a/src/SmartFormat/Core/Parsing/ParsingErrors.cs b/src/SmartFormat/Core/Parsing/ParsingErrors.cs index 387bc9e7..c785e07e 100644 --- a/src/SmartFormat/Core/Parsing/ParsingErrors.cs +++ b/src/SmartFormat/Core/Parsing/ParsingErrors.cs @@ -26,17 +26,8 @@ public ParsingErrors(Format result) public List Issues { get; } public bool HasIssues => Issues.Count > 0; - public string MessageShort - { - get - { - return string.Format("The format string has {0} issue{1}: {2}", - Issues.Count, - Issues.Count == 1 ? "" : "s", - string.Join(", ", Issues.Select(i => i.Issue).ToArray()) - ); - } - } + public string MessageShort => + $"The format string has {Issues.Count} issue{(Issues.Count == 1 ? "" : "s")}: {string.Join(", ", Issues.Select(i => i.Issue).ToArray())}"; public override string Message { @@ -59,13 +50,8 @@ public override string Message } } - return string.Format("The format string has {0} issue{1}:\n{2}\nIn: \"{3}\"\nAt: {4} ", - Issues.Count, - Issues.Count == 1 ? "" : "s", - string.Join(", ", Issues.Select(i => i.Issue).ToArray()), - result.baseString, - arrows - ); + return + $"The format string has {Issues.Count} issue{(Issues.Count == 1 ? "" : "s")}:\n{string.Join(", ", Issues.Select(i => i.Issue).ToArray())}\nIn: \"{result.baseString}\"\nAt: {arrows} "; } }