Clean up unused IVariableResolver functionality.

Handle escaped bind-time variable references.

5 files changed, 72 insertions, 18 deletions

diff --git a/src/WixToolset.Core/Bind/ResolveFieldsCommand.cs b/src/WixToolset.Core/Bind/ResolveFieldsCommand.cs
index 5cb2524d..5db878a1 100644
--- a/src/WixToolset.Core/Bind/ResolveFieldsCommand.cs
+++ b/src/WixToolset.Core/Bind/ResolveFieldsCommand.cs
@@ -62,7 +62,7 @@ namespace WixToolset.Core.Bind
                                 var original = field.AsString();
                                 if (!String.IsNullOrEmpty(original))
                                 {
-                                    var resolution = this.VariableResolver.ResolveVariables(tuple.SourceLineNumbers, original, false);
+                                    var resolution = this.VariableResolver.ResolveVariables(tuple.SourceLineNumbers, original);
                                     if (resolution.UpdatedValue)
                                     {
                                         field.Set(resolution.Value);
diff --git a/src/WixToolset.Core/Compiler.cs b/src/WixToolset.Core/Compiler.cs
index 6fd0f30b..de718c84 100644
--- a/src/WixToolset.Core/Compiler.cs
+++ b/src/WixToolset.Core/Compiler.cs
@@ -251,7 +251,7 @@ namespace WixToolset.Core
                                 var data = field.AsString();
                                 if (!String.IsNullOrEmpty(data))
                                 {
-                                    var resolved = this.componentIdPlaceholdersResolver.ResolveVariables(tuple.SourceLineNumbers, data, false, false);
+                                    var resolved = this.componentIdPlaceholdersResolver.ResolveVariables(tuple.SourceLineNumbers, data, errorOnUnknown: false);
                                     if (resolved.UpdatedValue)
                                     {
                                         field.Set(resolved.Value);
diff --git a/src/WixToolset.Core/Librarian.cs b/src/WixToolset.Core/Librarian.cs
index 5c0fb302..b6be73e9 100644
--- a/src/WixToolset.Core/Librarian.cs
+++ b/src/WixToolset.Core/Librarian.cs
@@ -95,7 +95,7 @@ namespace WixToolset.Core
 
                         if (pathField != null && !String.IsNullOrEmpty(pathField.Path))
                         {
-                            var resolution = variableResolver.ResolveVariables(tuple.SourceLineNumbers, pathField.Path, false);
+                            var resolution = variableResolver.ResolveVariables(tuple.SourceLineNumbers, pathField.Path);
 
                             var file = fileResolver.Resolve(tuple.SourceLineNumbers, tuple.Definition, resolution.Value);
 
diff --git a/src/WixToolset.Core/VariableResolver.cs b/src/WixToolset.Core/VariableResolver.cs
index 4a65bec2..b1a5defc 100644
--- a/src/WixToolset.Core/VariableResolver.cs
+++ b/src/WixToolset.Core/VariableResolver.cs
@@ -66,9 +66,9 @@ namespace WixToolset.Core
             }
         }
 
-        public IVariableResolution ResolveVariables(SourceLineNumber sourceLineNumbers, string value, bool localizationOnly)
+        public IVariableResolution ResolveVariables(SourceLineNumber sourceLineNumbers, string value)
         {
-            return this.ResolveVariables(sourceLineNumbers, value, localizationOnly, true);
+            return this.ResolveVariables(sourceLineNumbers, value, errorOnUnknown: true);
         }
 
         public bool TryGetLocalizedControl(string dialog, string control, out LocalizedControl localizedControl)
@@ -82,25 +82,27 @@ namespace WixToolset.Core
         /// </summary>
         /// <param name="sourceLineNumbers">The source line information for the value.</param>
         /// <param name="value">The value to resolve.</param>
-        /// <param name="localizationOnly">true to only resolve localization variables; false otherwise.</param>
         /// <param name="errorOnUnknown">true if unknown variables should throw errors.</param>
         /// <returns>The resolved value.</returns>
-        internal IVariableResolution ResolveVariables(SourceLineNumber sourceLineNumbers, string value, bool localizationOnly, bool errorOnUnknown)
+        internal IVariableResolution ResolveVariables(SourceLineNumber sourceLineNumbers, string value, bool errorOnUnknown)
         {
             var matches = Common.WixVariableRegex.Matches(value);
 
-            // the value is the default unless its substituted further down
+            // the value is the default unless it's substituted further down
             var result = this.ServiceProvider.GetService<IVariableResolution>();
             result.IsDefault = true;
             result.Value = value;
 
-            while (!this.Messaging.EncounteredError && !result.DelayedResolve && matches.Count > 0)
+            var finalizeEscapes = false;
+
+            while (matches.Count > 0)
             {
+                var updatedResultThisPass = false;
                 var sb = new StringBuilder(value);
 
                 // notice how this code walks backward through the list
                 // because it modifies the string as we move through it
-                for (int i = matches.Count - 1; 0 <= i; i--)
+                for (var i = matches.Count - 1; 0 <= i; i--)
                 {
                     var variableNamespace = matches[i].Groups["namespace"].Value;
                     var variableId = matches[i].Groups["fullname"].Value;
@@ -130,12 +132,16 @@ namespace WixToolset.Core
                     // 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 (!localizationOnly)
+                        if (finalizeEscapes)
                         {
                             sb.Remove(matches[i].Index - 1, 1);
 
                             result.UpdatedValue = true;
                         }
+                        else
+                        {
+                            continue;
+                        }
                     }
                     else
                     {
@@ -154,7 +160,7 @@ namespace WixToolset.Core
                                 resolvedValue = bindVariable.Value;
                             }
                         }
-                        else if (!localizationOnly && "wix" == variableNamespace)
+                        else if ("wix" == variableNamespace)
                         {
                             // illegal syntax of $(wix.var)
                             if ('$' == sb[matches[i].Index])
@@ -189,12 +195,13 @@ namespace WixToolset.Core
                                 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 (!localizationOnly && "wix" == variableNamespace && errorOnUnknown) // unresolved wix variable
+                            else if ("wix" == variableNamespace && errorOnUnknown) // unresolved wix variable
                             {
                                 this.Messaging.Write(ErrorMessages.WixVariableUnknown(sourceLineNumbers, variableId));
                             }
@@ -204,7 +211,22 @@ namespace WixToolset.Core
 
                 result.Value = sb.ToString();
                 value = result.Value;
-                matches = Common.WixVariableRegex.Matches(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;
diff --git a/src/test/WixToolsetTest.CoreIntegration/VariableResolverFixture.cs b/src/test/WixToolsetTest.CoreIntegration/VariableResolverFixture.cs
index eaeb4724..abf1bd43 100644
--- a/src/test/WixToolsetTest.CoreIntegration/VariableResolverFixture.cs
+++ b/src/test/WixToolsetTest.CoreIntegration/VariableResolverFixture.cs
@@ -29,10 +29,42 @@ namespace WixToolsetTest.CoreIntegration
 
             variableResolver.AddLocalization(localization);
 
-            Assert.Equal("Welcome to Localized Product Name", variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductName)", false).Value);
-            Assert.Equal("Welcome to Localized Product Name Enterprise Edition", variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductNameEdition)", false).Value);
-            Assert.Equal("Welcome to Localized Product Name Enterprise Edition v1.2.3", variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductNameEditionVersion)", false).Value);
-            Assert.Throws<WixException>(() => variableResolver.ResolveVariables(null, "Welcome to !(loc.UnknownLocalizationVariable)", false));
+            var result = variableResolver.ResolveVariables(null, "These are not the loc strings you're looking for.");
+            Assert.Equal("These are not the loc strings you're looking for.", result.Value);
+            Assert.False(result.UpdatedValue);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductName)");
+            Assert.Equal("Welcome to Localized Product Name", result.Value);
+            Assert.True(result.UpdatedValue);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductNameEdition)");
+            Assert.Equal("Welcome to Localized Product Name Enterprise Edition", result.Value);
+            Assert.True(result.UpdatedValue);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductNameEditionVersion)");
+            Assert.Equal("Welcome to Localized Product Name Enterprise Edition v1.2.3", result.Value);
+            Assert.True(result.UpdatedValue);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !(bind.property.ProductVersion)");
+            Assert.Equal("Welcome to !(bind.property.ProductVersion)", result.Value);
+            Assert.False(result.UpdatedValue);
+            Assert.True(result.DelayedResolve);
+
+            Assert.Throws<WixException>(() => variableResolver.ResolveVariables(null, "Welcome to !(loc.UnknownLocalizationVariable)"));
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !!(loc.UnknownLocalizationVariable)");
+            Assert.Equal("Welcome to !(loc.UnknownLocalizationVariable)", result.Value);
+            Assert.True(result.UpdatedValue);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !!(loc.UnknownLocalizationVariable) v!(bind.property.ProductVersion)");
+            Assert.Equal("Welcome to !(loc.UnknownLocalizationVariable) v!(bind.property.ProductVersion)", result.Value);
+            Assert.True(result.UpdatedValue);
+            Assert.True(result.DelayedResolve);
+
+            result = variableResolver.ResolveVariables(null, "Welcome to !(loc.ProductNameEditionVersion) !!(loc.UnknownLocalizationVariable) v!(bind.property.ProductVersion)");
+            Assert.Equal("Welcome to Localized Product Name Enterprise Edition v1.2.3 !(loc.UnknownLocalizationVariable) v!(bind.property.ProductVersion)", result.Value);
+            Assert.True(result.UpdatedValue);
+            Assert.True(result.DelayedResolve);
         }
     }
 }