Improve compiler performance by removing regex and other string fixes

11 files changed, 454 insertions, 374 deletions

diff --git a/src/WixToolset.Core.WindowsInstaller/Unbind/UnbindDatabaseCommand.cs b/src/WixToolset.Core.WindowsInstaller/Unbind/UnbindDatabaseCommand.cs
index d5601fad..36172b5e 100644
--- a/src/WixToolset.Core.WindowsInstaller/Unbind/UnbindDatabaseCommand.cs
+++ b/src/WixToolset.Core.WindowsInstaller/Unbind/UnbindDatabaseCommand.cs
@@ -226,11 +226,12 @@ namespace WixToolset.Core.WindowsInstaller.Unbind
                                                 value = value.Replace("$(", "$$(");
 
                                                 // escape things that look like wix variables
-                                                var matches = Common.WixVariableRegex.Matches(value);
-                                                for (var j = matches.Count - 1; 0 <= j; j--)
-                                                {
-                                                    value = value.Insert(matches[j].Index, "!");
-                                                }
+                                                // TODO: Evaluate this requirement.
+                                                //var matches = Common.WixVariableRegex.Matches(value);
+                                                //for (var j = matches.Count - 1; 0 <= j; j--)
+                                                //{
+                                                //    value = value.Insert(matches[j].Index, "!");
+                                                //}
 
                                                 row[i] = value;
                                                 break;
diff --git a/src/WixToolset.Core/Bind/ResolveDelayedFieldsCommand.cs b/src/WixToolset.Core/Bind/ResolveDelayedFieldsCommand.cs
index a10b98dc..ebabed47 100644
--- a/src/WixToolset.Core/Bind/ResolveDelayedFieldsCommand.cs
+++ b/src/WixToolset.Core/Bind/ResolveDelayedFieldsCommand.cs
@@ -69,11 +69,11 @@ namespace WixToolset.Core.Bind
             }
 
             // add specialization for ProductVersion fields
-            string keyProductVersion = "property.ProductVersion";
+            var keyProductVersion = "property.ProductVersion";
             if (this.VariableCache.TryGetValue(keyProductVersion, out var versionValue) && Version.TryParse(versionValue, out Version productVersion))
             {
                 // Don't add the variable if it already exists (developer defined a property with the same name).
-                string fieldKey = String.Concat(keyProductVersion, ".Major");
+                var fieldKey = String.Concat(keyProductVersion, ".Major");
                 if (!this.VariableCache.ContainsKey(fieldKey))
                 {
                     this.VariableCache[fieldKey] = productVersion.Major.ToString(CultureInfo.InvariantCulture);
@@ -115,68 +115,45 @@ namespace WixToolset.Core.Bind
 
         private static string ResolveDelayedVariables(SourceLineNumber sourceLineNumbers, string value, IDictionary<string, string> resolutionData)
         {
-            var matches = Common.WixVariableRegex.Matches(value);
+            var start = 0;
 
-            if (0 < matches.Count)
+            while (Common.TryParseWixVariable(value, start, out var parsed))
             {
-                var sb = new StringBuilder(value);
-
-                // notice how this code walks backward through the list
-                // because it modifies the string as we go through it
-                for (int i = matches.Count - 1; 0 <= i; i--)
+                if (parsed.Namespace == "bind")
                 {
-                    string variableNamespace = matches[i].Groups["namespace"].Value;
-                    string variableId = matches[i].Groups["fullname"].Value;
-                    string variableDefaultValue = null;
-                    string variableScope = null;
+                    var key = String.Concat(parsed.Name, ".", parsed.Scope);
 
-                    // get the default value if one was specified
-                    if (matches[i].Groups["value"].Success)
+                    if (!resolutionData.TryGetValue(key, out var resolvedValue))
                     {
-                        variableDefaultValue = matches[i].Groups["value"].Value;
+                        resolvedValue = parsed.DefaultValue;
                     }
 
-                    // get the scope if one was specified
-                    if (matches[i].Groups["scope"].Success)
+                    // insert the resolved value if it was found or display an error
+                    if (null != resolvedValue)
                     {
-                        variableScope = matches[i].Groups["scope"].Value;
-                        if ("bind" == variableNamespace)
+                        if (parsed.Index == 0 && parsed.Length == value.Length)
                         {
-                            variableId = matches[i].Groups["name"].Value;
+                            value = resolvedValue;
+                        }
+                        else
+                        {
+                            var sb = new StringBuilder(value);
+                            sb.Remove(parsed.Index, parsed.Length);
+                            sb.Insert(parsed.Index, resolvedValue);
+                            value = sb.ToString();
                         }
-                    }
 
-                    // check for an escape sequence of !! indicating the match is not a variable expression
-                    if (0 < matches[i].Index && '!' == sb[matches[i].Index - 1])
-                    {
-                        sb.Remove(matches[i].Index - 1, 1);
+                        start = parsed.Index;
                     }
                     else
                     {
-                        var key = String.Format(CultureInfo.InvariantCulture, "{0}.{1}", variableId, variableScope).ToLower(CultureInfo.InvariantCulture);
-
-                        if (!resolutionData.TryGetValue(key, out var resolvedValue))
-                        {
-                            resolvedValue = variableDefaultValue;
-                        }
-
-                        if ("bind" == variableNamespace)
-                        {
-                            // insert the resolved value if it was found or display an error
-                            if (null != resolvedValue)
-                            {
-                                sb.Remove(matches[i].Index, matches[i].Length);
-                                sb.Insert(matches[i].Index, resolvedValue);
-                            }
-                            else
-                            {
-                                throw new WixException(ErrorMessages.UnresolvedBindReference(sourceLineNumbers, value));
-                            }
-                        }
+                        throw new WixException(ErrorMessages.UnresolvedBindReference(sourceLineNumbers, value));
                     }
                 }
-
-                value = sb.ToString();
+                else
+                {
+                    start = parsed.Index + parsed.Length;
+                }
             }
 
             return value;
diff --git a/src/WixToolset.Core/Common.cs b/src/WixToolset.Core/Common.cs
index a9fc9538..19c77368 100644
--- a/src/WixToolset.Core/Common.cs
+++ b/src/WixToolset.Core/Common.cs
@@ -9,7 +9,6 @@ namespace WixToolset.Core
     using System.Linq;
     using System.Security.Cryptography;
     using System.Text;
-    using System.Text.RegularExpressions;
     using System.Xml;
     using System.Xml.Linq;
     using WixToolset.Data;
@@ -102,18 +101,14 @@ namespace WixToolset.Core
         // TODO: Find a place to put this that it doesn't have to be public and exposed by WixToolset.Core.dll
         public static readonly string[] FilePermissions = { "Read", "Write", "Append", "ReadExtendedAttributes", "WriteExtendedAttributes", "Execute", "FileAllRights", "ReadAttributes", "WriteAttributes" };
 
-        public static readonly Regex WixVariableRegex = new Regex(@"(\!|\$)\((?<namespace>loc|wix|bind|bindpath)\.(?<fullname>(?<name>[_A-Za-z][0-9A-Za-z_]+)(\.(?<scope>[_A-Za-z][0-9A-Za-z_\.]*))?)(\=(?<value>.+?))?\)", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.ExplicitCapture);
+        internal static readonly char[] IllegalLongFilenameCharacters = new[] { '\\', '/', '?', '*', '|', '>', '<', ':', '\"' }; // illegal: \ / ? | > < : / * "
+        internal static readonly char[] IllegalRelativeLongFilenameCharacters = new[] { '?', '*', '|', '>', '<', ':', '\"' }; // like illegal, but we allow '\' and '/'
+        internal static readonly char[] IllegalWildcardLongFilenameCharacters = new[] { '\\', '/', '|', '>', '<', ':', '\"' };   // like illegal: but we allow '*' and '?'
 
-        private static readonly Regex PropertySearch = new Regex(@"\[[#$!]?[a-zA-Z_][a-zA-Z0-9_\.]*]", RegexOptions.Singleline);
-        private static readonly Regex AddPrefix = new Regex(@"^[^a-zA-Z_]", RegexOptions.Compiled);
-        private static readonly Regex LegalIdentifierCharacters = new Regex(@"^[_A-Za-z][0-9A-Za-z_\.]*$", RegexOptions.Compiled);
-        private static readonly Regex IllegalIdentifierCharacters = new Regex(@"[^A-Za-z0-9_\.]|\.{2,}", RegexOptions.Compiled); // non 'words' and assorted valid characters
+        private static readonly char[] IllegalShortFilenameCharacters = new[] { '\\', '?', '|', '>', '<', ':', '/', '*', '\"', '+', ',', ';', '=', '[', ']', '.', ' ' };
+        private static readonly char[] IllegalWildcardShortFilenameCharacters = new[] { '\\', '|', '>', '<', ':', '/', '\"', '+', ',', ';', '=', '[', ']', '.', ' ' };
 
-        private const string LegalShortFilenameCharacters = @"[^\\\?|><:/\*""\+,;=\[\]\. ]"; // illegal: \ ? | > < : / * " + , ; = [ ] . (space)
-        private static readonly Regex LegalShortFilename = new Regex(String.Concat("^", LegalShortFilenameCharacters, @"{1,8}(\.", LegalShortFilenameCharacters, "{0,3})?$"), RegexOptions.Compiled);
 
-        private const string LegalWildcardShortFilenameCharacters = @"[^\\|><:/""\+,;=\[\]\. ]"; // illegal: \ | > < : / " + , ; = [ ] . (space)
-        private static readonly Regex LegalWildcardShortFilename = new Regex(String.Concat("^", LegalWildcardShortFilenameCharacters, @"{1,16}(\.", LegalWildcardShortFilenameCharacters, "{0,6})?$"));
 
         /// <summary>
         /// Gets a valid code page from the given web name or integer value.
@@ -197,79 +192,95 @@ namespace WixToolset.Core
 
             if (allowWildcards)
             {
-                if (Common.LegalWildcardShortFilename.IsMatch(filename))
+                var expectedDot = filename.IndexOfAny(IllegalWildcardShortFilenameCharacters);
+                if (expectedDot == -1)
                 {
-                    bool foundPeriod = false;
-                    int beforePeriod = 0;
-                    int afterPeriod = 0;
+                }
+                else if (filename[expectedDot] != '.')
+                {
+                    return false;
+                }
+                else if (expectedDot < filename.Length)
+                {
+                    var extensionInvalids = filename.IndexOfAny(IllegalShortFilenameCharacters, expectedDot + 1);
+                    if (extensionInvalids != -1)
+                    {
+                        return false;
+                    }
+                }
 
-                    // count the number of characters before and after the period
-                    // '*' is not counted because it may represent zero characters
-                    foreach (char character in filename)
+                var foundPeriod = false;
+                var beforePeriod = 0;
+                var afterPeriod = 0;
+
+                // count the number of characters before and after the period
+                // '*' is not counted because it may represent zero characters
+                foreach (var character in filename)
+                {
+                    if ('.' == character)
+                    {
+                        foundPeriod = true;
+                    }
+                    else if ('*' != character)
                     {
-                        if ('.' == character)
+                        if (foundPeriod)
                         {
-                            foundPeriod = true;
+                            afterPeriod++;
                         }
-                        else if ('*' != character)
+                        else
                         {
-                            if (foundPeriod)
-                            {
-                                afterPeriod++;
-                            }
-                            else
-                            {
-                                beforePeriod++;
-                            }
+                            beforePeriod++;
                         }
                     }
+                }
 
-                    if (8 >= beforePeriod && 3 >= afterPeriod)
-                    {
-                        return true;
-                    }
+                if (8 >= beforePeriod && 3 >= afterPeriod)
+                {
+                    return true;
                 }
 
                 return false;
             }
             else
             {
-                return Common.LegalShortFilename.IsMatch(filename);
+                if (filename.Length > 12)
+                {
+                    return false;
+                }
+
+                var expectedDot = filename.IndexOfAny(IllegalShortFilenameCharacters);
+                if (expectedDot == -1)
+                {
+                    return filename.Length < 9;
+                }
+                else if (expectedDot > 8 || filename[expectedDot] != '.')
+                {
+                    return false;
+                }
+
+                var validExtension = filename.IndexOfAny(IllegalShortFilenameCharacters, expectedDot + 1);
+                return validExtension == -1;
             }
         }
 
         /// <summary>
         /// Verifies if an identifier is a valid binder variable name.
         /// </summary>
-        /// <param name="name">Binder variable name to verify.</param>
+        /// <param name="variable">Binder variable name to verify.</param>
         /// <returns>True if the identifier is a valid binder variable name.</returns>
-        public static bool IsValidBinderVariable(string name)
+        public static bool IsValidBinderVariable(string variable)
         {
-            if (String.IsNullOrEmpty(name))
-            {
-                return false;
-            }
-
-            Match match = Common.WixVariableRegex.Match(name);
-
-            return (match.Success && ("bind" == match.Groups["namespace"].Value || "wix" == match.Groups["namespace"].Value) && 0 == match.Index && name.Length == match.Length);
+            return TryParseWixVariable(variable, 0, out var parsed) && parsed.Index == 0 && parsed.Length == variable.Length && (parsed.Namespace == "bind" || parsed.Namespace == "wix");
         }
 
         /// <summary>
         /// Verifies if a string contains a valid binder variable name.
         /// </summary>
-        /// <param name="name">String to verify.</param>
+        /// <param name="verify">String to verify.</param>
         /// <returns>True if the string contains a valid binder variable name.</returns>
-        public static bool ContainsValidBinderVariable(string name)
+        public static bool ContainsValidBinderVariable(string verify)
         {
-            if (String.IsNullOrEmpty(name))
-            {
-                return false;
-            }
-
-            Match match = Common.WixVariableRegex.Match(name);
-
-            return match.Success && ("bind" == match.Groups["namespace"].Value || "wix" == match.Groups["namespace"].Value);
+            return TryParseWixVariable(verify, 0, out var parsed) && (parsed.Namespace == "bind" || parsed.Namespace == "wix");
         }
 
         /// <summary>
@@ -281,7 +292,7 @@ namespace WixToolset.Core
         {
             if (!Common.IsValidBinderVariable(version))
             {
-                Version ver = null;
+                Version ver;
                 
                 try
                 {
@@ -344,16 +355,31 @@ namespace WixToolset.Core
         /// <returns>A version of the name that is a legal identifier.</returns>
         internal static string GetIdentifierFromName(string name)
         {
-            string result = IllegalIdentifierCharacters.Replace(name, "_"); // replace illegal characters with "_".
+            StringBuilder sb = null;
+            var offset = 0;
 
             // MSI identifiers must begin with an alphabetic character or an
             // underscore. Prefix all other values with an underscore.
-            if (AddPrefix.IsMatch(name))
+            if (!ValidIdentifierChar(name[0], true))
             {
-                result = String.Concat("_", result);
+                sb = new StringBuilder("_" + name);
+                offset = 1;
             }
 
-            return result;
+            for (var i = 0; i < name.Length; ++i)
+            {
+                if (!ValidIdentifierChar(name[i], false))
+                {
+                    if (sb == null)
+                    {
+                        sb = new StringBuilder(name);
+                    }
+
+                    sb[i + offset] = '_';
+                }
+            }
+
+            return sb?.ToString() ?? name;
         }
 
         /// <summary>
@@ -363,7 +389,28 @@ namespace WixToolset.Core
         /// <returns>True if a property is found in the string.</returns>
         internal static bool ContainsProperty(string possibleProperty)
         {
-            return PropertySearch.IsMatch(possibleProperty);
+            var start = possibleProperty.IndexOf('[');
+            if (start != -1 && start < possibleProperty.Length - 2)
+            {
+                var end = possibleProperty.IndexOf(']', start + 1);
+                if (end > start + 1)
+                {
+                    // Skip supported property modifiers.
+                    if (possibleProperty[start + 1] == '#' || possibleProperty[start + 1] == '$' || possibleProperty[start + 1] == '!')
+                    {
+                        ++start;
+                    }
+
+                    var id = possibleProperty.Substring(start + 1, end - 1);
+
+                    if (Common.IsIdentifier(id))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
         }
 
         /// <summary>
@@ -436,7 +483,7 @@ namespace WixToolset.Core
         public static string[] GetNames(string value)
         {
             string[] names = new string[4];
-            int targetSeparator = value.IndexOf(":", StringComparison.Ordinal);
+            int targetSeparator = value.IndexOf(':');
 
             // split source and target
             string sourceName = null;
@@ -451,7 +498,7 @@ namespace WixToolset.Core
             string sourceLongName = null;
             if (null != sourceName)
             {
-                int sourceLongNameSeparator = sourceName.IndexOf("|", StringComparison.Ordinal);
+                int sourceLongNameSeparator = sourceName.IndexOf('|');
                 if (0 <= sourceLongNameSeparator)
                 {
                     sourceLongName = sourceName.Substring(sourceLongNameSeparator + 1);
@@ -460,7 +507,7 @@ namespace WixToolset.Core
             }
 
             // split the target short and long names
-            int targetLongNameSeparator = targetName.IndexOf("|", StringComparison.Ordinal);
+            int targetLongNameSeparator = targetName.IndexOf('|');
             string targetLongName = null;
             if (0 <= targetLongNameSeparator)
             {
@@ -555,7 +602,7 @@ namespace WixToolset.Core
         /// <returns>The attribute's value.</returns>
         internal static string GetAttributeValue(IMessaging messaging, SourceLineNumber sourceLineNumbers, XAttribute attribute, EmptyRule emptyRule)
         {
-            string value = attribute.Value;
+            var value = attribute.Value;
 
             if ((emptyRule == EmptyRule.MustHaveNonWhitespaceCharacters && String.IsNullOrEmpty(value.Trim())) ||
                 (emptyRule == EmptyRule.CanBeWhitespaceOnly && String.IsNullOrEmpty(value)))
@@ -574,15 +621,20 @@ namespace WixToolset.Core
         /// <returns>true if the value is an identifier; false otherwise.</returns>
         public static bool IsIdentifier(string value)
         {
-            if (!String.IsNullOrEmpty(value))
+            if (String.IsNullOrEmpty(value))
+            {
+                return false;
+            }
+
+            for (var i = 0; i < value.Length; ++i)
             {
-                if (LegalIdentifierCharacters.IsMatch(value))
+                if (!ValidIdentifierChar(value[i], i == 0))
                 {
-                    return true;
+                    return false;
                 }
             }
 
-            return false;
+            return true;
         }
 
         /// <summary>
@@ -698,6 +750,85 @@ namespace WixToolset.Core
             return text?.Value;
         }
 
+        internal static bool TryParseWixVariable(string value, int start, out ParsedWixVariable parsedVariable)
+        {
+            parsedVariable = null;
+
+            if (String.IsNullOrEmpty(value) || start >= value.Length)
+            {
+                return false;
+            }
+
+            var startWixVariable = value.IndexOf("!(", start, StringComparison.Ordinal);
+            if (startWixVariable == -1)
+            {
+                return false;
+            }
+
+            var firstDot = value.IndexOf('.', startWixVariable + 1);
+            if (firstDot == -1)
+            {
+                return false;
+            }
+
+            var ns = value.Substring(startWixVariable + 2, firstDot - startWixVariable - 2);
+            if (ns != "loc" && ns != "bind" && ns != "wix")
+            {
+                return false;
+            }
+
+            var closeParen = value.IndexOf(')', firstDot);
+            if (closeParen == -1)
+            {
+                return false;
+            }
+
+            string name;
+            string scope = null;
+            string defaultValue = null;
+
+            var secondDot = value.IndexOf('.', firstDot + 1, closeParen - firstDot);
+            var equalsDefaultValue = value.IndexOf('=', firstDot + 1, closeParen - firstDot);
+            var end = equalsDefaultValue == -1 ? closeParen : equalsDefaultValue;
+
+            if (secondDot == -1)
+            {
+                name = value.Substring(firstDot + 1, end - firstDot - 1);
+            }
+            else
+            {
+                name = value.Substring(firstDot + 1, secondDot - firstDot - 1);
+                scope = value.Substring(secondDot + 1, end - secondDot - 1);
+
+                if (!Common.IsIdentifier(scope))
+                {
+                    return false;
+                }
+            }
+
+            if (!Common.IsIdentifier(name))
+            {
+                return false;
+            }
+
+            if (equalsDefaultValue != -1 && equalsDefaultValue < closeParen)
+            {
+                defaultValue = value.Substring(equalsDefaultValue + 1, end - equalsDefaultValue - 1);
+            }
+
+            parsedVariable = new ParsedWixVariable
+            {
+                Index = startWixVariable,
+                Length = closeParen - startWixVariable + 1,
+                Namespace = ns,
+                Name = name,
+                Scope = scope,
+                DefaultValue = defaultValue
+            };
+
+            return true;
+        }
+
         /// <summary>
         /// Display an unexpected attribute error.
         /// </summary>
@@ -727,5 +858,11 @@ namespace WixToolset.Core
                 messaging.Write(ErrorMessages.UnsupportedExtensionAttribute(sourceLineNumbers, extensionAttribute.Parent.Name.LocalName, extensionAttribute.Name.LocalName));
             }
         }
+
+        private static bool ValidIdentifierChar(char c, bool firstChar)
+        {
+            return ('A' <= c && 'Z' >= c) || ('a' <= c && 'z' >= c) || '_' == c ||
+                   (!firstChar && (Char.IsDigit(c) || '.' == c));
+        }
     }
 }
diff --git a/src/WixToolset.Core/Compiler.cs b/src/WixToolset.Core/Compiler.cs
index b29821b0..020e35fe 100644
--- a/src/WixToolset.Core/Compiler.cs
+++ b/src/WixToolset.Core/Compiler.cs
@@ -9,7 +9,6 @@ namespace WixToolset.Core
     using System.Globalization;
     using System.IO;
     using System.Linq;
-    using System.Text.RegularExpressions;
     using System.Xml.Linq;
     using WixToolset.Data;
     using WixToolset.Data.Symbols;
@@ -26,8 +25,8 @@ namespace WixToolset.Core
         private const int MinValueOfMaxCabSizeForLargeFileSplitting = 20; // 20 MB
         private const int MaxValueOfMaxCabSizeForLargeFileSplitting = 2 * 1024; // 2048 MB (i.e. 2 GB)
 
-        private const string DefaultComponentIdPlaceholderFormat = "WixComponentIdPlaceholder{0}";
-        private const string DefaultComponentIdPlaceholderWixVariableFormat = "!(wix.{0})";
+        private const string DefaultComponentIdPlaceholderPrefix = "WixComponentIdPlaceholder";
+        private const string DefaultComponentIdPlaceholderWixVariablePrefix = "!(wix.";
         // If these are true you know you are building a module or product
         // but if they are false you cannot not be sure they will not end
         // up a product or module.  Use these flags carefully.
@@ -342,16 +341,22 @@ namespace WixToolset.Core
 
             if (!String.IsNullOrEmpty(value))
             {
-                var regex = new Regex(@"\[(?<identifier>[a-zA-Z_][a-zA-Z0-9_\.]*)]", RegexOptions.Compiled | RegexOptions.Singleline | RegexOptions.ExplicitCapture);
-                var matches = regex.Matches(value);
-
-                foreach (Match match in matches)
+                var start = value.IndexOf('[');
+                while (start != -1 && start < value.Length)
                 {
-                    var group = match.Groups["identifier"];
-                    if (group.Success)
+                    var end = value.IndexOf(']', start + 1);
+                    if (end == -1)
+                    {
+                        break;
+                    }
+
+                    var id = value.Substring(start + 1, end - 1);
+                    if (Common.IsIdentifier(id))
                     {
-                        this.Core.Write(WarningMessages.PropertyValueContainsPropertyReference(sourceLineNumbers, propertyId.Id, group.Value));
+                        this.Core.Write(WarningMessages.PropertyValueContainsPropertyReference(sourceLineNumbers, propertyId.Id, id));
                     }
+
+                    start = (end < value.Length) ? value.IndexOf('[', end + 1) : -1;
                 }
             }
 
@@ -2091,8 +2096,8 @@ namespace WixToolset.Core
             var encounteredODBCDataSource = false;
             var files = 0;
             var guid = "*";
-            var componentIdPlaceholder = String.Format(Compiler.DefaultComponentIdPlaceholderFormat, this.componentIdPlaceholdersResolver.VariableCount); // placeholder id for defaulting Component/@Id to keypath id.
-            var componentIdPlaceholderWixVariable = String.Format(Compiler.DefaultComponentIdPlaceholderWixVariableFormat, componentIdPlaceholder);
+            var componentIdPlaceholder = Compiler.DefaultComponentIdPlaceholderPrefix + this.componentIdPlaceholdersResolver.VariableCount; // placeholder id for defaulting Component/@Id to keypath id.
+            var componentIdPlaceholderWixVariable = Compiler.DefaultComponentIdPlaceholderWixVariablePrefix + componentIdPlaceholder + ")";
             var id = new Identifier(AccessModifier.Private, componentIdPlaceholderWixVariable);
             var keyFound = false;
             string keyPath = null;
@@ -4080,7 +4085,7 @@ namespace WixToolset.Core
                         break;
                     case "Name":
                         nameHasValue = true;
-                        if (attrib.Value.Equals("."))
+                        if (attrib.Value == ".")
                         {
                             name = attrib.Value;
                         }
@@ -4225,7 +4230,7 @@ namespace WixToolset.Core
                 defaultDir = String.Concat(defaultDir, ":", String.IsNullOrEmpty(shortSourceName) ? sourceName : String.Concat(shortSourceName, "|", sourceName));
             }
 
-            if ("TARGETDIR".Equals(id.Id) && !"SourceDir".Equals(defaultDir))
+            if ("TARGETDIR".Equals(id.Id, StringComparison.Ordinal) && !"SourceDir".Equals(defaultDir, StringComparison.Ordinal))
             {
                 this.Core.Write(ErrorMessages.IllegalTargetDirDefaultDir(sourceLineNumbers, defaultDir));
             }
@@ -7147,13 +7152,9 @@ namespace WixToolset.Core
                 else // external cabinet file
                 {
                     // external cabinet files must use 8.3 filenames
-                    if (!String.IsNullOrEmpty(cabinet) && !this.Core.IsValidShortFilename(cabinet, false))
+                    if (!String.IsNullOrEmpty(cabinet) && !this.Core.IsValidLongFilename(cabinet) && !Common.ContainsValidBinderVariable(cabinet))
                     {
-                        // WiX variables in the name will trip the "not a valid 8.3 name" switch, so let them through
-                        if (!Common.WixVariableRegex.Match(cabinet).Success)
-                        {
-                            this.Core.Write(WarningMessages.MediaExternalCabinetFilenameIllegal(sourceLineNumbers, node.Name.LocalName, "Cabinet", cabinet));
-                        }
+                        this.Core.Write(WarningMessages.MediaExternalCabinetFilenameIllegal(sourceLineNumbers, node.Name.LocalName, "Cabinet", cabinet));
                     }
                 }
             }
@@ -7286,12 +7287,12 @@ namespace WixToolset.Core
                         if (!this.Core.IsValidLocIdentifier(exampleCabinetName))
                         {
                             // The example name should not match the authored template since that would nullify the
-                            // reason for having multiple cabients. External cabinet files must also be valid file names.
-                            if (exampleCabinetName.Equals(authoredCabinetTemplateValue) || !this.Core.IsValidLongFilename(exampleCabinetName, false))
+                            // reason for having multiple cabinets. External cabinet files must also be valid file names.
+                            if (exampleCabinetName.Equals(authoredCabinetTemplateValue, StringComparison.OrdinalIgnoreCase) || !this.Core.IsValidLongFilename(exampleCabinetName, false))
                             {
                                 this.Core.Write(ErrorMessages.InvalidCabinetTemplate(sourceLineNumbers, cabinetTemplate));
                             }
-                            else if (!this.Core.IsValidShortFilename(exampleCabinetName, false) && !Common.WixVariableRegex.Match(exampleCabinetName).Success) // ignore short names with wix variables because it rarely works out.
+                            else if (!this.Core.IsValidLongFilename(exampleCabinetName) && !Common.ContainsValidBinderVariable(exampleCabinetName)) // ignore short names with wix variables because it rarely works out.
                             {
                                 this.Core.Write(WarningMessages.MediaExternalCabinetFilenameIllegal(sourceLineNumbers, node.Name.LocalName, "CabinetTemplate", cabinetTemplate));
                             }
diff --git a/src/WixToolset.Core/CompilerCore.cs b/src/WixToolset.Core/CompilerCore.cs
index c2724f6b..2f65db7a 100644
--- a/src/WixToolset.Core/CompilerCore.cs
+++ b/src/WixToolset.Core/CompilerCore.cs
@@ -10,7 +10,6 @@ namespace WixToolset.Core
     using System.Globalization;
     using System.Reflection;
     using System.Text;
-    using System.Text.RegularExpressions;
     using System.Xml.Linq;
     using WixToolset.Data;
     using WixToolset.Data.Symbols;
@@ -45,16 +44,8 @@ namespace WixToolset.Core
         internal static readonly XNamespace W3SchemaPrefix = "http://www.w3.org/";
         internal static readonly XNamespace WixNamespace = "http://wixtoolset.org/schemas/v4/wxs";
 
-        private static readonly Regex AmbiguousFilename = new Regex(@"^.{6}\~\d", RegexOptions.Compiled);
-
-        private const string IllegalLongFilenameCharacters = @"[\\\?|><:/\*""]"; // illegal: \ ? | > < : / * "
-        private static readonly Regex IllegalLongFilename = new Regex(IllegalLongFilenameCharacters, RegexOptions.Compiled);
-
-        //public const int DefaultMaximumUncompressedMediaSize = 200; // Default value is 200 MB
-
-
         // Built-in variables (from burn\engine\variable.cpp, "vrgBuiltInVariables", around line 113)
-        private static readonly List<String> BuiltinBundleVariables = new List<string>(
+        private static readonly List<string> BuiltinBundleVariables = new List<string>(
             new string[] {
                 "AdminToolsFolder",
                 "AppDataFolder",
@@ -221,7 +212,13 @@ namespace WixToolset.Core
         /// <returns>true if the filename is ambiguous; false otherwise.</returns>
         public static bool IsAmbiguousFilename(string filename)
         {
-            return String.IsNullOrEmpty(filename) ? false : CompilerCore.AmbiguousFilename.IsMatch(filename);
+            if (String.IsNullOrEmpty(filename))
+            {
+                return false;
+            }
+
+            var tilde = filename.IndexOf('~');
+            return (tilde > 0 && tilde < filename.Length) && Char.IsNumber(filename[tilde + 1]);
         }
 
         /// <summary>
@@ -273,9 +270,29 @@ namespace WixToolset.Core
         /// <param name="filename">Filename to make valid.</param>
         /// <param name="replace">Replacement string for invalid characters in filename.</param>
         /// <returns>Valid filename.</returns>
-        public static string MakeValidLongFileName(string filename, string replace)
+        public static string MakeValidLongFileName(string filename, char replace)
         {
-            return CompilerCore.IllegalLongFilename.Replace(filename, replace);
+            if (String.IsNullOrEmpty(filename))
+            {
+                return filename;
+            }
+
+            StringBuilder sb = null;
+
+            var found = filename.IndexOfAny(Common.IllegalLongFilenameCharacters);
+            while (found != -1)
+            {
+                if (sb == null)
+                {
+                    sb = new StringBuilder(filename);
+                }
+
+                sb[found] = replace;
+
+                found = (found + 1 < filename.Length) ? filename.IndexOfAny(Common.IllegalLongFilenameCharacters, found + 1) : -1;
+            }
+
+            return sb?.ToString() ?? filename;
         }
 
         /// <summary>
@@ -717,7 +734,7 @@ namespace WixToolset.Core
                 throw new ArgumentNullException("attribute");
             }
 
-            string value = this.GetAttributeValue(sourceLineNumbers, attribute);
+            var value = this.GetAttributeValue(sourceLineNumbers, attribute);
 
             if (0 < value.Length)
             {
@@ -1039,16 +1056,6 @@ namespace WixToolset.Core
             return this.parseHelper.ScheduleActionSymbol(this.ActiveSection, sourceLineNumbers, access, sequence, actionName, condition, beforeAction, afterAction, overridable);
         }
 
-        /// <summary>
-        /// Finds a compiler extension by namespace URI.
-        /// </summary>
-        /// <param name="ns">Namespace the extension supports.</param>
-        /// <returns>True if found compiler extension or false if nothing matches namespace URI.</returns>
-        private bool TryFindExtension(XNamespace ns, out ICompilerExtension extension)
-        {
-            return this.extensions.TryGetValue(ns, out extension);
-        }
-
         private static string CreateValueList(ValueListKind kind, IEnumerable<string> values)
         {
             // Ideally, we could denote the list kind (and the list itself) directly in the
diff --git a/src/WixToolset.Core/Compiler_2.cs b/src/WixToolset.Core/Compiler_2.cs
index 7e2485e1..29f240f4 100644
--- a/src/WixToolset.Core/Compiler_2.cs
+++ b/src/WixToolset.Core/Compiler_2.cs
@@ -4182,30 +4182,6 @@ namespace WixToolset.Core
                             this.Core.Write(ErrorMessages.IllegalAttributeValue(sourceLineNumbers, node.Name.LocalName, "Show", showValue, "normal", "maximized", "minimized"));
                             break;
                         }
-                        //if (showValue.Length == 0)
-                        //{
-                        //    show = CompilerConstants.IllegalInteger;
-                        //}
-                        //else
-                        //{
-                        //    var showType = Wix.Shortcut.ParseShowType(showValue);
-                        //    switch (showType)
-                        //    {
-                        //    case Wix.Shortcut.ShowType.normal:
-                        //        show = 1;
-                        //        break;
-                        //    case Wix.Shortcut.ShowType.maximized:
-                        //        show = 3;
-                        //        break;
-                        //    case Wix.Shortcut.ShowType.minimized:
-                        //        show = 7;
-                        //        break;
-                        //    default:
-                        //        this.Core.Write(ErrorMessages.IllegalAttributeValue(sourceLineNumbers, node.Name.LocalName, "Show", showValue, "normal", "maximized", "minimized"));
-                        //        show = CompilerConstants.IllegalInteger;
-                        //        break;
-                        //    }
-                        //}
                         break;
                     case "Target":
                         target = this.Core.GetAttributeValue(sourceLineNumbers, attrib);
@@ -4325,11 +4301,11 @@ namespace WixToolset.Core
                 }
                 else if ("Component" == parentElementLocalName || "CreateFolder" == parentElementLocalName)
                 {
-                    target = String.Format(CultureInfo.InvariantCulture, "[{0}]", defaultTarget);
+                    target = "[" + defaultTarget + "]";
                 }
                 else if ("File" == parentElementLocalName)
                 {
-                    target = String.Format(CultureInfo.InvariantCulture, "[#{0}]", defaultTarget);
+                    target = "[#" + defaultTarget + "]";
                 }
 
                 this.Core.AddSymbol(new ShortcutSymbol(sourceLineNumbers, id)
diff --git a/src/WixToolset.Core/Compiler_Bundle.cs b/src/WixToolset.Core/Compiler_Bundle.cs
index 578c7dcd..2089f037 100644
--- a/src/WixToolset.Core/Compiler_Bundle.cs
+++ b/src/WixToolset.Core/Compiler_Bundle.cs
@@ -236,7 +236,7 @@ namespace WixToolset.Core
             else
             {
                 // Ensure only allowable path characters are in "name" (and change spaces to underscores).
-                fileSystemSafeBundleName = CompilerCore.MakeValidLongFileName(name.Replace(' ', '_'), "_");
+                fileSystemSafeBundleName = CompilerCore.MakeValidLongFileName(name.Replace(' ', '_'), '_');
                 logVariablePrefixAndExtension = String.Concat("WixBundleLog:", fileSystemSafeBundleName, ":log");
             }
 
diff --git a/src/WixToolset.Core/Compiler_UI.cs b/src/WixToolset.Core/Compiler_UI.cs
index 36d2e4e9..9353d966 100644
--- a/src/WixToolset.Core/Compiler_UI.cs
+++ b/src/WixToolset.Core/Compiler_UI.cs
@@ -1740,7 +1740,7 @@ namespace WixToolset.Core
             {
                 // if we're not looking at a standard action or a formatted string then create a reference
                 // to the custom action.
-                if (!WindowsInstallerStandard.IsStandardAction(argument) && !Common.ContainsProperty(argument))
+                if (!WindowsInstallerStandard.IsStandardAction(argument) && !this.Core.ContainsProperty(argument))
                 {
                     this.Core.CreateSimpleReference(sourceLineNumbers, SymbolDefinitions.CustomAction, argument);
                 }
diff --git a/src/WixToolset.Core/ExtensibilityServices/ParseHelper.cs b/src/WixToolset.Core/ExtensibilityServices/ParseHelper.cs
index db465354..81a18e24 100644
--- a/src/WixToolset.Core/ExtensibilityServices/ParseHelper.cs
+++ b/src/WixToolset.Core/ExtensibilityServices/ParseHelper.cs
@@ -9,7 +9,6 @@ namespace WixToolset.Core.ExtensibilityServices
     using System.IO;
     using System.Security.Cryptography;
     using System.Text;
-    using System.Text.RegularExpressions;
     using System.Xml.Linq;
     using WixToolset.Data;
     using WixToolset.Data.Symbols;
@@ -20,19 +19,6 @@ namespace WixToolset.Core.ExtensibilityServices
 
     internal class ParseHelper : IParseHelper
     {
-        private const string LegalLongFilenameCharacters = @"[^\\\?|><:/\*""]";  // opposite of illegal above.
-        private static readonly Regex LegalLongFilename = new Regex(String.Concat("^", LegalLongFilenameCharacters, @"{1,259}$"), RegexOptions.Compiled);
-
-        private const string LegalRelativeLongFilenameCharacters = @"[^\?|><:/\*""]"; // (like legal long, but we allow '\') illegal: ? | > < : / * "
-        private static readonly Regex LegalRelativeLongFilename = new Regex(String.Concat("^", LegalRelativeLongFilenameCharacters, @"{1,259}$"), RegexOptions.Compiled);
-
-        private const string LegalWildcardLongFilenameCharacters = @"[^\\|><:/""]"; // illegal: \ | > < : / "
-        private static readonly Regex LegalWildcardLongFilename = new Regex(String.Concat("^", LegalWildcardLongFilenameCharacters, @"{1,259}$"));
-
-        private static readonly Regex LegalIdentifierWithAccess = new Regex(@"^((?<access>public|internal|protected|private)\s+)?(?<id>[_A-Za-z][0-9A-Za-z_\.]*)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture);
-
-        private static readonly Regex PutGuidHere = new Regex(@"PUT\-GUID\-(?:\d+\-)?HERE", RegexOptions.Singleline);
-
         public ParseHelper(IWixToolsetServiceProvider serviceProvider)
         {
             this.ServiceProvider = serviceProvider;
@@ -119,7 +105,7 @@ namespace WixToolset.Core.ExtensibilityServices
                     id = parentId;
 
                     var pathStartsAt = 0;
-                    if (inlineSyntax[0].EndsWith(":"))
+                    if (inlineSyntax[0].EndsWith(":", StringComparison.Ordinal))
                     {
                         // TODO: should overriding the parent identifier with a specific id be an error or a warning or just let it slide?
                         //if (null != parentId)
@@ -415,44 +401,34 @@ namespace WixToolset.Core.ExtensibilityServices
             var emptyRule = canBeEmpty ? EmptyRule.CanBeEmpty : EmptyRule.CanBeWhitespaceOnly;
             var value = this.GetAttributeValue(sourceLineNumbers, attribute, emptyRule);
 
-            if (String.IsNullOrEmpty(value) && canBeEmpty)
-            {
-                return String.Empty;
-            }
-            else if (!String.IsNullOrEmpty(value))
+            if (String.IsNullOrEmpty(value))
             {
-                // If the value starts and ends with braces or parenthesis, accept that and strip them off.
-                if ((value.StartsWith("{", StringComparison.Ordinal) && value.EndsWith("}", StringComparison.Ordinal))
-                    || (value.StartsWith("(", StringComparison.Ordinal) && value.EndsWith(")", StringComparison.Ordinal)))
+                if (canBeEmpty)
                 {
-                    value = value.Substring(1, value.Length - 2);
+                    return String.Empty;
                 }
-
-                if (generatable && "*".Equals(value, StringComparison.Ordinal))
+            }
+            else
+            {
+                if (generatable && value == "*")
                 {
                     return value;
                 }
 
-                if (ParseHelper.PutGuidHere.IsMatch(value))
+                if (Guid.TryParse(value, out var guid))
                 {
-                    this.Messaging.Write(ErrorMessages.ExampleGuid(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
-                    return CompilerConstants.IllegalGuid;
+                    return guid.ToString("B").ToUpperInvariant();
                 }
-                else if (value.StartsWith("!(loc", StringComparison.Ordinal) || value.StartsWith("$(loc", StringComparison.Ordinal) || value.StartsWith("!(wix", StringComparison.Ordinal))
+
+                if (value.StartsWith("!(loc", StringComparison.Ordinal) || value.StartsWith("$(loc", StringComparison.Ordinal) || value.StartsWith("!(wix", StringComparison.Ordinal))
                 {
                     return value;
                 }
-                else if (Guid.TryParse(value, out var guid))
-                {
-                    var uppercaseGuid = guid.ToString().ToUpperInvariant();
 
-                    // TODO: This used to be a pedantic error, what should it be now?
-                    //if (uppercaseGuid != value)
-                    //{
-                    //    this.Messaging.Write(WixErrors.GuidContainsLowercaseLetters(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
-                    //}
-
-                    return String.Concat("{", uppercaseGuid, "}");
+                if (value.StartsWith("PUT-GUID-", StringComparison.OrdinalIgnoreCase) ||
+                    value.StartsWith("{PUT-GUID-", StringComparison.OrdinalIgnoreCase))
+                {
+                    this.Messaging.Write(ErrorMessages.ExampleGuid(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
                 }
                 else
                 {
@@ -468,19 +444,45 @@ namespace WixToolset.Core.ExtensibilityServices
             var access = AccessModifier.Public;
             var value = Common.GetAttributeValue(this.Messaging, sourceLineNumbers, attribute, EmptyRule.CanBeEmpty);
 
-            var match = ParseHelper.LegalIdentifierWithAccess.Match(value);
-            if (!match.Success)
+            var separator = value.IndexOf(' ');
+            if (separator > 0)
             {
-                return null;
+                var prefix = value.Substring(0, separator);
+                switch (prefix)
+                {
+                    case "public":
+                    case "package":
+                        access = AccessModifier.Public;
+                        break;
+
+                    case "internal":
+                    case "library":
+                        access = AccessModifier.Internal;
+                        break;
+
+                    case "protected":
+                    case "file":
+                        access = AccessModifier.Protected;
+                        break;
+
+                    case "private":
+                    case "fragment":
+                        access = AccessModifier.Private;
+                        break;
+
+                    default:
+                        return null;
+                }
+
+                value = value.Substring(separator + 1).Trim();
             }
-            else if (match.Groups["access"].Success)
+
+            if (!Common.IsIdentifier(value))
             {
-                access = (AccessModifier)Enum.Parse(typeof(AccessModifier), match.Groups["access"].Value, true);
+                this.Messaging.Write(ErrorMessages.IllegalIdentifier(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
+                return null;
             }
-
-            value = match.Groups["id"].Value;
-
-            if (Common.IsIdentifier(value) && 72 < value.Length)
+            else if (72 < value.Length)
             {
                 this.Messaging.Write(WarningMessages.IdentifierTooLong(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
             }
@@ -520,7 +522,7 @@ namespace WixToolset.Core.ExtensibilityServices
                 }
                 else if (resultUsedToCreateReference && 1 == result.Length)
                 {
-                    if (value.EndsWith("\\"))
+                    if (value.EndsWith("\\", StringComparison.Ordinal))
                     {
                         if (!this.IsValidLongFilename(result[0], false, false))
                         {
@@ -547,7 +549,7 @@ namespace WixToolset.Core.ExtensibilityServices
                     }
                 }
 
-                if (1 < result.Length && !value.EndsWith("\\"))
+                if (1 < result.Length && !value.EndsWith("\\", StringComparison.Ordinal))
                 {
                     this.Messaging.Write(WarningMessages.BackslashTerminateInlineDirectorySyntax(sourceLineNumbers, attribute.Parent.Name.LocalName, attribute.Name.LocalName, value));
                 }
@@ -776,14 +778,7 @@ namespace WixToolset.Core.ExtensibilityServices
 
         public bool IsValidLocIdentifier(string identifier)
         {
-            if (String.IsNullOrEmpty(identifier))
-            {
-                return false;
-            }
-
-            var match = Common.WixVariableRegex.Match(identifier);
-
-            return (match.Success && "loc" == match.Groups["namespace"].Value && 0 == match.Index && identifier.Length == match.Length);
+            return Common.TryParseWixVariable(identifier, 0, out var parsed) && parsed.Index == 0 && parsed.Length == identifier.Length && parsed.Namespace == "loc";
         }
 
         public bool IsValidLongFilename(string filename, bool allowWildcards, bool allowRelative)
@@ -792,29 +787,38 @@ namespace WixToolset.Core.ExtensibilityServices
             {
                 return false;
             }
+            else if (filename.Length > 259)
+            {
+                return false;
+            }
 
             // Check for a non-period character (all periods is not legal)
-            var nonPeriodFound = false;
+            var allPeriods = true;
             foreach (var character in filename)
             {
                 if ('.' != character)
                 {
-                    nonPeriodFound = true;
+                    allPeriods = false;
                     break;
                 }
             }
 
+            if (allPeriods)
+            {
+                return false;
+            }
+
             if (allowWildcards)
             {
-                return (nonPeriodFound && ParseHelper.LegalWildcardLongFilename.IsMatch(filename));
+                return filename.IndexOfAny(Common.IllegalWildcardLongFilenameCharacters) == -1;
             }
             else if (allowRelative)
             {
-                return (nonPeriodFound && ParseHelper.LegalRelativeLongFilename.IsMatch(filename));
+                return filename.IndexOfAny(Common.IllegalRelativeLongFilenameCharacters) == -1; 
             }
             else
             {
-                return (nonPeriodFound && ParseHelper.LegalLongFilename.IsMatch(filename));
+                return filename.IndexOfAny(Common.IllegalLongFilenameCharacters) == -1;
             }
         }
 
diff --git a/src/WixToolset.Core/ParsedWixVariable.cs b/src/WixToolset.Core/ParsedWixVariable.cs
new file mode 100644
index 00000000..9d308b77
--- /dev/null
+++ b/src/WixToolset.Core/ParsedWixVariable.cs
@@ -0,0 +1,19 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
+
+namespace WixToolset.Core
+{
+    internal class ParsedWixVariable
+    {
+        public int Index { get; set; }
+
+        public int Length { get; set; }
+
+        public string Namespace { get; set; }
+
+        public string Name { get; set; }
+
+        public string Scope { get; set; }
+
+        public string DefaultValue { get; set; }
+    }
+}
diff --git a/src/WixToolset.Core/VariableResolver.cs b/src/WixToolset.Core/VariableResolver.cs
index 88067673..140e7def 100644
--- a/src/WixToolset.Core/VariableResolver.cs
+++ b/src/WixToolset.Core/VariableResolver.cs
@@ -79,150 +79,108 @@ namespace WixToolset.Core
 
         public IVariableResolution ResolveVariables(SourceLineNumber sourceLineNumbers, string value, bool errorOnUnknown)
         {
-            var matches = Common.WixVariableRegex.Matches(value);
+            var start = 0;
+            var defaulted = true;
+            var delayed = false;
+            var updated = false;
 
-            // the value is the default unless it's substituted further down
-            var result = this.ServiceProvider.GetService<IVariableResolution>();
-            result.IsDefault = true;
-            result.Value = value;
-
-            var finalizeEscapes = false;
-
-            while (matches.Count > 0)
+            while (Common.TryParseWixVariable(value, start, out var parsed))
             {
-                var updatedResultThisPass = false;
-                var sb = new StringBuilder(value);
+                var variableNamespace = parsed.Namespace;
+                var variableId = parsed.Name;
+                var variableDefaultValue = parsed.DefaultValue;
 
-                // notice how this code walks backward through the list
-                // because it modifies the string as we move through it
-                for (var i = matches.Count - 1; 0 <= i; i--)
+                // check for an escape sequence of !! indicating the match is not a variable expression
+                if (0 < parsed.Index && '!' == value[parsed.Index - 1])
                 {
-                    var variableNamespace = matches[i].Groups["namespace"].Value;
-                    var variableId = matches[i].Groups["fullname"].Value;
-                    string variableDefaultValue = null;
+                    var sb = new StringBuilder(value);
+                    sb.Remove(parsed.Index - 1, 1);
+                    value = sb.ToString();
 
-                    // get the default value if one was specified
-                    if (matches[i].Groups["value"].Success)
-                    {
-                        variableDefaultValue = matches[i].Groups["value"].Value;
+                    updated = true;
+                    start = parsed.Index + parsed.Length - 1;
 
-                        // localization variables do not support inline default values
-                        if ("loc" == variableNamespace)
-                        {
-                            this.Messaging.Write(ErrorMessages.IllegalInlineLocVariable(sourceLineNumbers, variableId, variableDefaultValue));
-                        }
+                    continue;
+                }
+
+                string resolvedValue = null;
+
+                if ("loc" == variableNamespace)
+                {
+                    // localization variables do not support inline default values
+                    if (variableDefaultValue != null)
+                    {
+                        this.Messaging.Write(ErrorMessages.IllegalInlineLocVariable(sourceLineNumbers, variableId, variableDefaultValue));
+                        continue;
                     }
 
-                    // get the scope if one was specified
-                    if (matches[i].Groups["scope"].Success)
+                    if (this.locVariables.TryGetValue(variableId, out var bindVariable))
                     {
-                        if ("bind" == variableNamespace)
-                        {
-                            variableId = matches[i].Groups["name"].Value;
-                        }
+                        resolvedValue = bindVariable.Value;
+                    }
+                }
+                else if ("wix" == variableNamespace)
+                {
+                    if (this.wixVariables.TryGetValue(variableId, out var bindVariable))
+                    {
+                        resolvedValue = bindVariable.Value ?? String.Empty;
+                        defaulted = false;
                     }
+                    else if (null != variableDefaultValue) // default the resolved value to the inline value if one was specified
+                    {
+                        resolvedValue = variableDefaultValue;
+                    }
+                }
 
-                    // check for an escape sequence of !! indicating the match is not a variable expression
-                    if (0 < matches[i].Index && '!' == sb[matches[i].Index - 1])
+                if ("bind" == variableNamespace)
+                {
+                    // Can't resolve these yet, but keep track of where we find them so they can be resolved later with less effort.
+                    delayed = true;
+                    start = parsed.Index + parsed.Length - 1;
+                }
+                else
+                {
+                    // insert the resolved value if it was found or display an error
+                    if (null != resolvedValue)
                     {
-                        if (finalizeEscapes)
+                        if (parsed.Index == 0 && parsed.Length == value.Length)
                         {
-                            sb.Remove(matches[i].Index - 1, 1);
-
-                            result.UpdatedValue = true;
+                            value = resolvedValue;
                         }
                         else
                         {
-                            continue;
+                            var sb = new StringBuilder(value);
+                            sb.Remove(parsed.Index, parsed.Length);
+                            sb.Insert(parsed.Index, resolvedValue);
+                            value = sb.ToString();
                         }
+
+                        updated = true;
+                        start = parsed.Index;
                     }
                     else
                     {
-                        string resolvedValue = null;
-
-                        if ("loc" == variableNamespace)
+                        if ("loc" == variableNamespace && errorOnUnknown) // unresolved loc variable
                         {
-                            // warn about deprecated syntax of $(loc.var)
-                            if ('$' == sb[matches[i].Index])
-                            {
-                                this.Messaging.Write(WarningMessages.DeprecatedLocalizationVariablePrefix(sourceLineNumbers, variableId));
-                            }
-
-                            if (this.locVariables.TryGetValue(variableId, out var bindVariable))
-                            {
-                                resolvedValue = bindVariable.Value;
-                            }
+                            this.Messaging.Write(ErrorMessages.LocalizationVariableUnknown(sourceLineNumbers, variableId));
                         }
-                        else if ("wix" == variableNamespace)
+                        else if ("wix" == variableNamespace && errorOnUnknown) // unresolved wix variable
                         {
-                            // illegal syntax of $(wix.var)
-                            if ('$' == sb[matches[i].Index])
-                            {
-                                this.Messaging.Write(ErrorMessages.IllegalWixVariablePrefix(sourceLineNumbers, variableId));
-                            }
-                            else
-                            {
-                                if (this.wixVariables.TryGetValue(variableId, out var bindVariable))
-                                {
-                                    resolvedValue = bindVariable.Value ?? String.Empty;
-                                    result.IsDefault = false;
-                                }
-                                else if (null != variableDefaultValue) // default the resolved value to the inline value if one was specified
-                                {
-                                    resolvedValue = variableDefaultValue;
-                                }
-                            }
+                            this.Messaging.Write(ErrorMessages.WixVariableUnknown(sourceLineNumbers, variableId));
                         }
 
-                        if ("bind" == variableNamespace)
-                        {
-                            // can't resolve these yet, but keep track of where we find them so they can be resolved later with less effort
-                            result.DelayedResolve = true;
-                        }
-                        else
-                        {
-                            // insert the resolved value if it was found or display an error
-                            if (null != resolvedValue)
-                            {
-                                sb.Remove(matches[i].Index, matches[i].Length);
-                                sb.Insert(matches[i].Index, resolvedValue);
-
-                                result.UpdatedValue = true;
-                                updatedResultThisPass = true;
-                            }
-                            else if ("loc" == variableNamespace && errorOnUnknown) // unresolved loc variable
-                            {
-                                this.Messaging.Write(ErrorMessages.LocalizationVariableUnknown(sourceLineNumbers, variableId));
-                            }
-                            else if ("wix" == variableNamespace && errorOnUnknown) // unresolved wix variable
-                            {
-                                this.Messaging.Write(ErrorMessages.WixVariableUnknown(sourceLineNumbers, variableId));
-                            }
-                        }
+                        start = parsed.Index + parsed.Length;
                     }
                 }
-
-                result.Value = sb.ToString();
-                value = result.Value;
-
-                if (finalizeEscapes)
-                {
-                    // escaped references have been un-escaped, so we're done
-                    break;
-                }
-                else if (updatedResultThisPass)
-                {
-                    // we substituted loc strings, so make another pass to see if that brought in more loc strings
-                    matches = Common.WixVariableRegex.Matches(value);
-                }
-                else
-                {
-                    // make one final pass to un-escape any escaped references
-                    finalizeEscapes = true;
-                }
             }
 
-            return result;
+            return new VariableResolution
+            {
+                DelayedResolve = delayed,
+                IsDefault = defaulted,
+                UpdatedValue = updated,
+                Value = value,
+            };
         }
 
         private static bool TryAddWixVariable(IDictionary<string, BindVariable> variables, BindVariable variable)