From d26157531381aba81d2cac15e424b7e5c738253a Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Sun, 2 Aug 2020 08:32:54 -0600 Subject: WIXFEAT:4763 Change "string" variable type to literal and add "formatted". --- .../Bind/SetVariableSearchFacade.cs | 19 ++++- .../Bundles/CreateBurnManifestCommand.cs | 19 ++++- src/WixToolset.Core/Compiler_Bundle.cs | 93 +++++++++++++--------- .../BadInputFixture.cs | 23 ++++++ .../TestData/BadInput/BundleVariable.wxs | 7 ++ .../WixToolsetTest.CoreIntegration.csproj | 1 + 6 files changed, 122 insertions(+), 40 deletions(-) create mode 100644 src/test/WixToolsetTest.CoreIntegration/TestData/BadInput/BundleVariable.wxs (limited to 'src') diff --git a/src/WixToolset.Core.Burn/Bind/SetVariableSearchFacade.cs b/src/WixToolset.Core.Burn/Bind/SetVariableSearchFacade.cs index fb6f72dd..e88f26ef 100644 --- a/src/WixToolset.Core.Burn/Bind/SetVariableSearchFacade.cs +++ b/src/WixToolset.Core.Burn/Bind/SetVariableSearchFacade.cs @@ -21,10 +21,25 @@ namespace WixToolset.Core.Burn base.WriteXml(writer); - if (this.SetVariableSymbol.Type != null) + if (this.SetVariableSymbol.Type != WixBundleVariableType.Unknown) { writer.WriteAttributeString("Value", this.SetVariableSymbol.Value); - writer.WriteAttributeString("Type", this.SetVariableSymbol.Type); + + switch (this.SetVariableSymbol.Type) + { + case WixBundleVariableType.Formatted: + writer.WriteAttributeString("Type", "formatted"); + break; + case WixBundleVariableType.Numeric: + writer.WriteAttributeString("Type", "numeric"); + break; + case WixBundleVariableType.String: + writer.WriteAttributeString("Type", "string"); + break; + case WixBundleVariableType.Version: + writer.WriteAttributeString("Type", "version"); + break; + } } writer.WriteEndElement(); diff --git a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs index 05b15ab6..6eafcdd9 100644 --- a/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs +++ b/src/WixToolset.Core.Burn/Bundles/CreateBurnManifestCommand.cs @@ -131,10 +131,25 @@ namespace WixToolset.Core.Burn.Bundles { writer.WriteStartElement("Variable"); writer.WriteAttributeString("Id", variable.Id.Id); - if (null != variable.Type) + if (variable.Type != WixBundleVariableType.Unknown) { writer.WriteAttributeString("Value", variable.Value); - writer.WriteAttributeString("Type", variable.Type); + + switch (variable.Type) + { + case WixBundleVariableType.Formatted: + writer.WriteAttributeString("Type", "formatted"); + break; + case WixBundleVariableType.Numeric: + writer.WriteAttributeString("Type", "numeric"); + break; + case WixBundleVariableType.String: + writer.WriteAttributeString("Type", "string"); + break; + case WixBundleVariableType.Version: + writer.WriteAttributeString("Type", "version"); + break; + } } writer.WriteAttributeString("Hidden", variable.Hidden ? "yes" : "no"); writer.WriteAttributeString("Persisted", variable.Persisted ? "yes" : "no"); diff --git a/src/WixToolset.Core/Compiler_Bundle.cs b/src/WixToolset.Core/Compiler_Bundle.cs index d73db84d..33467dda 100644 --- a/src/WixToolset.Core/Compiler_Bundle.cs +++ b/src/WixToolset.Core/Compiler_Bundle.cs @@ -2141,7 +2141,7 @@ namespace WixToolset.Core case "": break; default: - this.Core.Write(ErrorMessages.IllegalAttributeValue(sourceLineNumbers, node.Name.LocalName, attrib.Name.LocalName, value, "button", "yes", "no")); + this.Core.Write(ErrorMessages.IllegalAttributeValue(sourceLineNumbers, node.Name.LocalName, attrib.Name.LocalName, value, "always", "yes", "no")); break; } break; @@ -3043,7 +3043,7 @@ namespace WixToolset.Core string condition = null; string after = null; string value = null; - string type = null; + string typeValue = null; foreach (var attrib in node.Attributes()) { @@ -3067,7 +3067,7 @@ namespace WixToolset.Core value = this.Core.GetAttributeValue(sourceLineNumbers, attrib); break; case "Type": - type = this.Core.GetAttributeValue(sourceLineNumbers, attrib); + typeValue = this.Core.GetAttributeValue(sourceLineNumbers, attrib); break; default: @@ -3081,13 +3081,13 @@ namespace WixToolset.Core } } - type = this.ValidateVariableTypeWithValue(sourceLineNumbers, type, value); + var type = this.ValidateVariableTypeWithValue(sourceLineNumbers, node, typeValue, value); this.Core.ParseForExtensionElements(node); if (id == null) { - id = this.Core.CreateIdentifier("sbv", variable, condition, after, value, type); + id = this.Core.CreateIdentifier("sbv", variable, condition, after, value, type.ToString()); } this.Core.CreateWixSearchSymbol(sourceLineNumbers, node.Name.LocalName, id, variable, condition, after); @@ -3113,7 +3113,7 @@ namespace WixToolset.Core string name = null; var persisted = false; string value = null; - string type = null; + string typeValue = null; foreach (var attrib in node.Attributes()) { @@ -3140,7 +3140,7 @@ namespace WixToolset.Core value = this.Core.GetAttributeValue(sourceLineNumbers, attrib, EmptyRule.CanBeEmpty); break; case "Type": - type = this.Core.GetAttributeValue(sourceLineNumbers, attrib); + typeValue = this.Core.GetAttributeValue(sourceLineNumbers, attrib); break; default: this.Core.UnexpectedAttribute(node, attrib); @@ -3162,7 +3162,7 @@ namespace WixToolset.Core this.Core.Write(ErrorMessages.ReservedNamespaceViolation(sourceLineNumbers, node.Name.LocalName, "Name", "Wix")); } - type = this.ValidateVariableTypeWithValue(sourceLineNumbers, type, value); + var type = this.ValidateVariableTypeWithValue(sourceLineNumbers, node, typeValue, value); this.Core.ParseForExtensionElements(node); @@ -3178,46 +3178,67 @@ namespace WixToolset.Core } } - private string ValidateVariableTypeWithValue(SourceLineNumber sourceLineNumbers, string type, string value) + private WixBundleVariableType ValidateVariableTypeWithValue(SourceLineNumber sourceLineNumbers, XElement node, string typeValue, string value) { - var newType = type; - if (newType == null && value != null) + WixBundleVariableType type; + switch (typeValue) + { + case "formatted": + type = WixBundleVariableType.Formatted; + break; + case "numeric": + type = WixBundleVariableType.Numeric; + break; + case "string": + type = WixBundleVariableType.String; + break; + case "version": + type = WixBundleVariableType.Version; + break; + case null: + type = WixBundleVariableType.Unknown; + break; + default: + this.Core.Write(ErrorMessages.IllegalAttributeValue(sourceLineNumbers, node.Name.LocalName, "Type", typeValue, "formatted", "numeric", "string", "version")); + return WixBundleVariableType.Unknown; + } + + if (type != WixBundleVariableType.Unknown) { - // Infer the type from the current value... - if (value.StartsWith("v", StringComparison.OrdinalIgnoreCase)) + if (value == null) { - // Version constructor does not support simple "v#" syntax so check to see if the value is - // non-negative real quick. - if (Int32.TryParse(value.Substring(1), NumberStyles.None, CultureInfo.InvariantCulture.NumberFormat, out var _)) - { - newType = "version"; - } - else if (Version.TryParse(value.Substring(1), out var _)) - { - newType = "version"; - } + this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, "Variable", "Value", "Type")); } - // Not a version, check for numeric. - if (newType == null) + return type; + } + else if (value == null) + { + return type; + } + + // Infer the type from the current value... + if (value.StartsWith("v", StringComparison.OrdinalIgnoreCase)) + { + // Version constructor does not support simple "v#" syntax so check to see if the value is + // non-negative real quick. + if (Int32.TryParse(value.Substring(1), NumberStyles.None, CultureInfo.InvariantCulture.NumberFormat, out var _)) { - if (Int64.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture.NumberFormat, out var _)) - { - newType = "numeric"; - } - else - { - newType = "string"; - } + return WixBundleVariableType.Version; + } + else if (Version.TryParse(value.Substring(1), out var _)) + { + return WixBundleVariableType.Version; } } - if (value == null && newType != null) + // Not a version, check for numeric. + if (Int64.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture.NumberFormat, out var _)) { - this.Core.Write(ErrorMessages.ExpectedAttribute(sourceLineNumbers, "Variable", "Value", "Type")); + return WixBundleVariableType.Numeric; } - return newType; + return WixBundleVariableType.String; } private class RemotePayload diff --git a/src/test/WixToolsetTest.CoreIntegration/BadInputFixture.cs b/src/test/WixToolsetTest.CoreIntegration/BadInputFixture.cs index d6c7b091..7a630a36 100644 --- a/src/test/WixToolsetTest.CoreIntegration/BadInputFixture.cs +++ b/src/test/WixToolsetTest.CoreIntegration/BadInputFixture.cs @@ -32,5 +32,28 @@ namespace WixToolsetTest.CoreIntegration Assert.InRange(result.ExitCode, 2, Int32.MaxValue); } } + + [Fact] + public void BundleVariableWithBadTypeIsRejected() + { + var folder = TestData.Get(@"TestData\BadInput"); + + using (var fs = new DisposableFileSystem()) + { + var baseFolder = fs.GetFolder(); + var intermediateFolder = Path.Combine(baseFolder, "obj"); + var wixlibPath = Path.Combine(intermediateFolder, @"test.wixlib"); + + var result = WixRunner.Execute(new[] + { + "build", + Path.Combine(folder, "BundleVariable.wxs"), + "-intermediateFolder", intermediateFolder, + "-o", wixlibPath, + }); + + Assert.Equal(21, result.ExitCode); + } + } } } diff --git a/src/test/WixToolsetTest.CoreIntegration/TestData/BadInput/BundleVariable.wxs b/src/test/WixToolsetTest.CoreIntegration/TestData/BadInput/BundleVariable.wxs new file mode 100644 index 00000000..293b379f --- /dev/null +++ b/src/test/WixToolsetTest.CoreIntegration/TestData/BadInput/BundleVariable.wxs @@ -0,0 +1,7 @@ + + + + + + diff --git a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj index 601a66e7..f4aab391 100644 --- a/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj +++ b/src/test/WixToolsetTest.CoreIntegration/WixToolsetTest.CoreIntegration.csproj @@ -22,6 +22,7 @@ + -- cgit v1.2.3-55-g6feb