Add support to escape/unescape testcase filter strings #1627
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| namespace Microsoft.VisualStudio.TestPlatform.Common.Filtering | ||
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Globalization; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using Microsoft.VisualStudio.TestPlatform.Common.Resources; | ||
|
|
||
| public static class FilterHelpers | ||
|
There was a problem hiding this comment. Can this be internal? LUT has dependency on There was a problem hiding this comment. Yes, made this public for LUT. LUT has dependency on TranslationLayer, which pulls in Common as well. There was a problem hiding this comment. Got it. However not all clients(VS code, Rider,..) uses TranslationLayer. We should rename this to FilterEscapeHelper and move this to OM? There was a problem hiding this comment. I guess the tokenizer code should remain private to FilterExpression/Condition in this case, don't see a point of exposing them. |
||
| { | ||
| private const char EscapePrefix = '\\'; | ||
|
There was a problem hiding this comment. Nit: |
||
| private static readonly char[] SpecialCharacters = { '\\', '(', ')', '&', '|', '=', '!', '~' }; | ||
|
|
||
| /// <summary> | ||
| /// Escapes a set of special characters for filter (%, (, ), &, |, =, !, ~) by replacing them with their escape sequences. | ||
| /// </summary> | ||
| /// <param name="str">The input string that contains the text to convert.</param> | ||
| /// <returns>A string of characters with special characters converted to their escaped form.</returns> | ||
| public static string Escape(string str) | ||
|
There was a problem hiding this comment. I am guessing. we are just going to replicate this code on the IDE side. If yes, it is problematic, maintenance issues and what not. There was a problem hiding this comment. I made the helper class public, so it can be used by other components. But I'm not sure if this is the right place for public helpers. |
||
| { | ||
| if (str == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(str)); | ||
| } | ||
|
|
||
| if (str.IndexOfAny(SpecialCharacters) < 0) | ||
| { | ||
| return str; | ||
| } | ||
|
|
||
| var builder = new StringBuilder(); | ||
| for (int i = 0; i < str.Length; ++i) | ||
| { | ||
| var currentChar = str[i]; | ||
| if (SpecialCharacters.Contains(currentChar)) | ||
|
There was a problem hiding this comment. Can we use |
||
| { | ||
| builder.Append(EscapePrefix); | ||
| } | ||
| builder.Append(currentChar); | ||
| } | ||
|
|
||
| return builder.ToString(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Converts any escaped characters in the input filter string. | ||
| /// </summary> | ||
| /// <param name="str">The input string that contains the text to convert.</param> | ||
| /// <returns>A filter string of characters with any escaped characters converted to their unescaped form.</returns> | ||
| public static string Unescape(string str) | ||
| { | ||
| if (str == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(str)); | ||
| } | ||
|
|
||
| if (str.IndexOf(EscapePrefix) < 0) | ||
| { | ||
| return str; | ||
| } | ||
|
|
||
| var builder = new StringBuilder(); | ||
| for (int i = 0; i < str.Length; ++i) | ||
| { | ||
| var currentChar = str[i]; | ||
| if (currentChar == EscapePrefix) | ||
| { | ||
| if (++i == str.Length || !SpecialCharacters.Contains(currentChar = str[i])) | ||
| { | ||
| // "\" should be followed by a special character. | ||
|
There was a problem hiding this comment. Show the position and character in error message and log same error message to EqtTrace. |
||
| throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.TestCaseFilterEscapeException, str)); | ||
| } | ||
| } | ||
|
|
||
| // Strictly speaking, string to be unescaped shouldn't contain any of the special characters, | ||
| // other than being part of escape sequence, but we will ignore that to avoid additional overhead. | ||
|
|
||
| builder.Append(currentChar); | ||
| } | ||
|
|
||
| return builder.ToString(); | ||
| } | ||
|
|
||
| internal static IEnumerable<string> TokenizeFilterExpressionString(string str) | ||
| { | ||
| if (str == null) | ||
|
There was a problem hiding this comment. Use string.IsNullOrEmpty() instead everywhere. There was a problem hiding this comment. This is to ensure the exact same behavior as before (with regex). we might be able to do further adjustment in |
||
| { | ||
| throw new ArgumentNullException(nameof(str)); | ||
| } | ||
|
|
||
| StringBuilder tokenBuilder = new StringBuilder(); | ||
|
|
||
| var last = '\0'; | ||
| for (int i = 0; i < str.Length; ++i) | ||
| { | ||
| var current = str[i]; | ||
|
|
||
| if (last == EscapePrefix) | ||
| { | ||
| // Don't check if `current` is one of the special characters here. | ||
| // Instead, we blindly let any character follows '\' pass though and | ||
| // relies on `FiltetrHelpers.Unescape` to report such errors. | ||
| tokenBuilder.Append(current); | ||
|
|
||
| if (current == EscapePrefix) | ||
| { | ||
| // We just encountered "\\" (escaped '\'), this will set last to '\0' | ||
| // so the next char will not be treated as a suffix of escape sequence. | ||
| current = '\0'; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| switch (current) | ||
| { | ||
| case '(': | ||
| case ')': | ||
| case '&': | ||
| case '|': | ||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| tokenBuilder.Clear(); | ||
| } | ||
| yield return current.ToString(); | ||
| break; | ||
|
|
||
| default: | ||
| tokenBuilder.Append(current); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| last = current; | ||
| } | ||
|
|
||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| } | ||
| } | ||
|
|
||
| internal static IEnumerable<string> TokenizeFilterConditionString(string str) | ||
|
There was a problem hiding this comment. Nit: Please add documentation for these as well. |
||
| { | ||
| if (str == null) | ||
| { | ||
| throw new ArgumentNullException(nameof(str)); | ||
| } | ||
|
|
||
| StringBuilder tokenBuilder = new StringBuilder(); | ||
|
|
||
| var last = '\0'; | ||
| for (int i = 0; i < str.Length; ++i) | ||
| { | ||
| var current = str[i]; | ||
|
|
||
| if (last == FilterHelpers.EscapePrefix) | ||
| { | ||
| // Don't check if `current` is one of the special characters here. | ||
| // Instead, we blindly let any character follows '\' pass though and | ||
| // relies on `FiltetrHelpers.Unescape` to report such errors. | ||
|
There was a problem hiding this comment. Nit: pass through*/ FilterHelpers* (Same in the other method) |
||
| tokenBuilder.Append(current); | ||
|
|
||
| if (current == FilterHelpers.EscapePrefix) | ||
| { | ||
| // We just encountered "\\" (escaped '\'), this will set last to '\0' | ||
| // so the next char will not be treated as a suffix of escape sequence. | ||
| current = '\0'; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| switch (current) | ||
|
There was a problem hiding this comment. Just put a comment telling the allowed special tokens, eg "=", "!=", "!", "~" There was a problem hiding this comment. When do we expect just "!" ? Can you share an example, also add to tests. There was a problem hiding this comment. We don't expect '!', but it's not handled in tokenizer. Instead, an exception is thrown in |
||
| { | ||
| case '=': | ||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| tokenBuilder.Clear(); | ||
| } | ||
| yield return "="; | ||
| break; | ||
|
|
||
| case '!': | ||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| tokenBuilder.Clear(); | ||
| } | ||
| // Determine if this is a "!=" or just a single "!". | ||
| var next = str[i + 1]; | ||
|
There was a problem hiding this comment. This will cause |
||
| if (next == '=') | ||
| { | ||
| ++i; | ||
| current = next; | ||
| yield return "!="; | ||
| } | ||
| else | ||
| { | ||
| yield return "!"; | ||
| } | ||
| break; | ||
|
|
||
| case '~': | ||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| tokenBuilder.Clear(); | ||
| } | ||
| yield return "~"; | ||
| break; | ||
|
|
||
| default: | ||
| tokenBuilder.Append(current); | ||
| break; | ||
| } | ||
| } | ||
| last = current; | ||
| } | ||
|
|
||
| if (tokenBuilder.Length > 0) | ||
| { | ||
| yield return tokenBuilder.ToString(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete this please