diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/KeyValuePairListEncoding.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/KeyValuePairListEncoding.cs index ba0afb4d2c1..84049c49aeb 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/KeyValuePairListEncoding.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Debug/KeyValuePairListEncoding.cs @@ -4,32 +4,39 @@ namespace Microsoft.VisualStudio.ProjectSystem.Debug; internal static class KeyValuePairListEncoding { - public static IEnumerable<(string Name, string Value)> Parse(string input) + /// + /// Parses the input string into a collection of key-value pairs with the given separator. + /// + /// The input string to parse. + /// Indicates whether empty keys are allowed. If this is true, a pair will be returned if an empty key has a non-empty value. ie, =4 + /// The character used to separate entries in the input string. + public static IEnumerable<(string Name, string Value)> Parse(string input, bool allowsEmptyKey = false, char separator = ',') { if (string.IsNullOrWhiteSpace(input)) { yield break; } - foreach (var entry in ReadEntries(input)) + foreach (var entry in ReadEntries(input, separator)) { - var (entryKey, entryValue) = SplitEntry(entry); + var (entryKey, entryValue) = SplitEntry(entry, allowsEmptyKey); var decodedEntryKey = Decode(entryKey); var decodedEntryValue = Decode(entryValue); - - if (!string.IsNullOrEmpty(decodedEntryKey)) + + if ((allowsEmptyKey && !string.IsNullOrEmpty(decodedEntryValue)) + || !string.IsNullOrEmpty(decodedEntryKey) || !string.IsNullOrEmpty(decodedEntryValue)) { yield return (decodedEntryKey, decodedEntryValue); } } - static IEnumerable ReadEntries(string rawText) + static IEnumerable ReadEntries(string rawText, char separator) { bool escaped = false; int entryStart = 0; for (int i = 0; i < rawText.Length; i++) { - if (rawText[i] == ',' && !escaped) + if (rawText[i] == separator && !escaped) { yield return rawText.Substring(entryStart, i - entryStart); entryStart = i + 1; @@ -48,12 +55,12 @@ static IEnumerable ReadEntries(string rawText) yield return rawText.Substring(entryStart); } - static (string EncodedKey, string EncodedValue) SplitEntry(string entry) + static (string EncodedKey, string EncodedValue) SplitEntry(string entry, bool allowsEmptyKey) { bool escaped = false; for (int i = 0; i < entry.Length; i++) { - if (entry[i] == '=' && !escaped) + if (entry[i] == '=' && !escaped && (allowsEmptyKey || i > 0)) { return (entry.Substring(0, i), entry.Substring(i + 1)); } @@ -67,7 +74,7 @@ static IEnumerable ReadEntries(string rawText) } } - return (string.Empty, string.Empty); + return (entry, string.Empty); } static string Decode(string value) @@ -76,12 +83,15 @@ static string Decode(string value) } } - public static string Format(IEnumerable<(string Name, string Value)> pairs) + public static string Format(IEnumerable<(string Name, string Value)> pairs, char separator = ',') { // Copied from ActiveLaunchProfileEnvironmentVariableValueProvider in the .NET Project System. // In future, EnvironmentVariablesNameValueListEncoding should be exported from that code base and imported here. - - return string.Join(",", pairs.Select(kvp => $"{Encode(kvp.Name)}={Encode(kvp.Value)}")); + return string.Join( + separator.ToString(), + pairs.Select(kvp => string.IsNullOrEmpty(kvp.Value) + ? Encode(kvp.Name) + : $"{Encode(kvp.Name)}={Encode(kvp.Value)}")); static string Encode(string value) { diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCSharpValueProvider.cs b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCAndFSharpValueProvider.cs similarity index 62% rename from src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCSharpValueProvider.cs rename to src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCAndFSharpValueProvider.cs index 3aaef154158..c5d8cb93864 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCSharpValueProvider.cs +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Properties/InterceptedProjectProperties/BuildPropertyPage/DefineConstantsCAndFSharpValueProvider.cs @@ -7,25 +7,18 @@ namespace Microsoft.VisualStudio.ProjectSystem.Properties; [ExportInterceptingPropertyValueProvider(ConfiguredBrowseObject.DefineConstantsProperty, ExportInterceptingPropertyValueProviderFile.ProjectFile)] [AppliesTo(ProjectCapability.CSharpOrFSharp)] -internal class DefineConstantsValueProvider : InterceptingPropertyValueProviderBase +[method: ImportingConstructor] +internal class DefineConstantsCAndFSharpValueProvider(IProjectAccessor projectAccessor, ConfiguredProject project) : InterceptingPropertyValueProviderBase { - private readonly IProjectAccessor _projectAccessor; - private readonly ConfiguredProject _project; + private const string DefineConstantsRecursivePrefix = "$(DefineConstants)"; - internal const string DefineConstantsRecursivePrefix = "$(DefineConstants)"; - - [ImportingConstructor] - public DefineConstantsValueProvider(IProjectAccessor projectAccessor, ConfiguredProject project) + private static IEnumerable ParseDefinedConstantsFromUnevaluatedValue(string unevaluatedValue) { - _projectAccessor = projectAccessor; - _project = project; - } + string substring = unevaluatedValue.Length <= DefineConstantsRecursivePrefix.Length || !unevaluatedValue.StartsWith(DefineConstantsRecursivePrefix) + ? unevaluatedValue + : unevaluatedValue.Substring(DefineConstantsRecursivePrefix.Length); - internal static IEnumerable ParseDefinedConstantsFromUnevaluatedValue(string unevaluatedValue) - { - return unevaluatedValue.Length <= DefineConstantsRecursivePrefix.Length || !unevaluatedValue.StartsWith(DefineConstantsRecursivePrefix) - ? Array.Empty() - : unevaluatedValue.Substring(DefineConstantsRecursivePrefix.Length).Split(';').Where(x => x.Length > 0); + return substring.Split(';').Where(x => x.Length > 0); } public override async Task OnGetUnevaluatedPropertyValueAsync(string propertyName, string unevaluatedPropertyValue, IProjectProperties defaultProperties) @@ -38,10 +31,11 @@ public override async Task OnGetUnevaluatedPropertyValueAsync(string pro return string.Empty; } - return KeyValuePairListEncoding.Format( - ParseDefinedConstantsFromUnevaluatedValue(unevaluatedDefineConstantsValue) - .Select(symbol => (Key: symbol, Value: bool.FalseString)) - ); + var pairs = KeyValuePairListEncoding.Parse(unevaluatedDefineConstantsValue, separator: ';').Select(pair => pair.Name) + .Where(symbol => !string.IsNullOrEmpty(symbol)) + .Select(symbol => (symbol, bool.FalseString)).ToList(); + + return KeyValuePairListEncoding.Format(pairs, separator: ','); } // We cannot rely on the unevaluated property value as obtained through Project.GetProperty.UnevaluatedValue - the reason is that for a recursively-defined @@ -51,11 +45,12 @@ public override async Task OnGetUnevaluatedPropertyValueAsync(string pro // 2. to override IsValueDefinedInContextAsync, as this will always return false private async Task GetUnevaluatedDefineConstantsPropertyValueAsync() { - await ((ConfiguredProject2)_project).EnsureProjectEvaluatedAsync(); - return await _projectAccessor.OpenProjectForReadAsync(_project, project => + await ((ConfiguredProject2)project).EnsureProjectEvaluatedAsync(); + + return await projectAccessor.OpenProjectForReadAsync(project, msbuildProject => { - project.ReevaluateIfNecessary(); - ProjectProperty defineConstantsProperty = project.GetProperty(ConfiguredBrowseObject.DefineConstantsProperty); + msbuildProject.ReevaluateIfNecessary(); + ProjectProperty defineConstantsProperty = msbuildProject.GetProperty(ConfiguredBrowseObject.DefineConstantsProperty); while (defineConstantsProperty.IsImported && defineConstantsProperty.Predecessor is not null) { defineConstantsProperty = defineConstantsProperty.Predecessor; @@ -76,18 +71,21 @@ public override async Task IsValueDefinedInContextAsync(string propertyNam IEnumerable innerConstants = ParseDefinedConstantsFromUnevaluatedValue(await defaultProperties.GetUnevaluatedPropertyValueAsync(ConfiguredBrowseObject.DefineConstantsProperty) ?? string.Empty); - IEnumerable constantsToWrite = KeyValuePairListEncoding.Parse(unevaluatedPropertyValue) + var foundConstants = KeyValuePairListEncoding.Parse(unevaluatedPropertyValue, separator: ',') .Select(pair => pair.Name) - .Where(x => !innerConstants.Contains(x)) + .Where(pair => !string.IsNullOrEmpty(pair)) + .Select(constant => constant.Trim(';')) // trim any leading or trailing semicolons, because we will add our own separating semicolons + .Where(constant => !string.IsNullOrEmpty(constant)) // you aren't allowed to add a semicolon as a constant .Distinct() .ToList(); - if (!constantsToWrite.Any()) + var writeableConstants = foundConstants.Where(constant => !innerConstants.Contains(constant)).ToList(); + if (writeableConstants.Count == 0) { await defaultProperties.DeletePropertyAsync(propertyName, dimensionalConditions); return null; } - - return $"{DefineConstantsRecursivePrefix};" + string.Join(";", constantsToWrite); + + return string.Join(";", writeableConstants); } } diff --git a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/PropertyPages/BuildPropertyPage.xaml b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/PropertyPages/BuildPropertyPage.xaml index d3d39a736f6..b4bee117d44 100644 --- a/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/PropertyPages/BuildPropertyPage.xaml +++ b/src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/PropertyPages/BuildPropertyPage.xaml @@ -61,6 +61,7 @@ + diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/ConfiguredProjectFactory.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/ConfiguredProjectFactory.cs index 96f4304b6e0..89af090ab4f 100644 --- a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/ConfiguredProjectFactory.cs +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.UnitTests/Mocks/ConfiguredProjectFactory.cs @@ -6,10 +6,11 @@ internal static class ConfiguredProjectFactory { public static ConfiguredProject Create(IProjectCapabilitiesScope? capabilities = null, ProjectConfiguration? projectConfiguration = null, ConfiguredProjectServices? services = null, UnconfiguredProject? unconfiguredProject = null) { - var mock = new Mock(); + var mock = new Mock(); mock.Setup(c => c.Capabilities).Returns(capabilities!); mock.Setup(c => c.ProjectConfiguration).Returns(projectConfiguration!); mock.Setup(c => c.Services).Returns(services!); + mock.Setup(c => c.EnsureProjectEvaluatedAsync()).Returns(Task.CompletedTask); mock.SetupGet(c => c.UnconfiguredProject).Returns(unconfiguredProject ?? UnconfiguredProjectFactory.Create()); return mock.Object; } @@ -32,4 +33,6 @@ public static ConfiguredProject ImplementUnconfiguredProject(UnconfiguredProject return mock.Object; } + + internal interface ITestConfiguredProjectImpl : ConfiguredProject, ConfiguredProject2; } diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/CSharp/DefineConstantsCAndFSharpValueProviderTests.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/CSharp/DefineConstantsCAndFSharpValueProviderTests.cs new file mode 100644 index 00000000000..445dd4f2156 --- /dev/null +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/CSharp/DefineConstantsCAndFSharpValueProviderTests.cs @@ -0,0 +1,68 @@ +// Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. + +using Microsoft.Build.Construction; +using Microsoft.VisualStudio.ProjectSystem.Properties; + +namespace Microsoft.VisualStudio.ProjectSystem.VS.Properties; + +public class DefineConstantsCAndFSharpValueProviderTests +{ + private const string PropertyName = "DefineConstants"; + + [Theory] + [InlineData("DEBUG;TRACE", "DEBUG=False,TRACE=False")] + [InlineData("", "")] + public async Task GetExistingUnevaluatedValue(string? defineConstantsValue, string expectedFormattedValue) + { + var provider = CreateInstance(defineConstantsValue, out _, out _); + + var actualPropertyValue = await provider.OnGetUnevaluatedPropertyValueAsync(string.Empty, string.Empty, null!); + Assert.Equal(expectedFormattedValue, actualPropertyValue); + } + + [Theory] + [InlineData("DEBUG,TRACE", null, "DEBUG;TRACE", "DEBUG=False,TRACE=False")] + [InlineData("$(DefineConstants),DEBUG,TRACE", "PROP1;PROP2", "$(DefineConstants);DEBUG;TRACE", "$(DefineConstants)=False,DEBUG=False,TRACE=False")] + public async Task SetUnevaluatedValue(string unevaluatedValueToSet, string? defineConstantsValue, string? expectedSetUnevaluatedValue, string expectedFormattedValue) + { + var provider = CreateInstance(null, out var projectAccessor, out var project); + Mock mockProjectProperties = new Mock(); + mockProjectProperties + .Setup(p => p.GetUnevaluatedPropertyValueAsync(ConfiguredBrowseObject.DefineConstantsProperty)) + .ReturnsAsync(defineConstantsValue); + + var setPropertyValue = await provider.OnSetPropertyValueAsync(PropertyName, unevaluatedValueToSet, mockProjectProperties.Object); + Assert.Equal(expectedSetUnevaluatedValue, setPropertyValue); + + await SetDefineConstantsPropertyAsync(projectAccessor, project, setPropertyValue); + + var actualPropertyFormattedValue = await provider.OnGetUnevaluatedPropertyValueAsync(string.Empty, string.Empty, null!); + Assert.Equal(expectedFormattedValue, actualPropertyFormattedValue); + } + + private static DefineConstantsCAndFSharpValueProvider CreateInstance(string? defineConstantsValue, out IProjectAccessor projectAccessor, out ConfiguredProject project) + { + var projectXml = defineConstantsValue is not null + ? $""" + + + <{ConfiguredBrowseObject.DefineConstantsProperty}>{defineConstantsValue} + + + """ + : ""; + + projectAccessor = IProjectAccessorFactory.Create(ProjectRootElementFactory.Create(projectXml)); + project = ConfiguredProjectFactory.Create(); + + return new DefineConstantsCAndFSharpValueProvider(projectAccessor, project); + } + + private static async Task SetDefineConstantsPropertyAsync(IProjectAccessor projectAccessor, ConfiguredProject project, string? setPropertyValue) + { + await projectAccessor.OpenProjectXmlForWriteAsync(project.UnconfiguredProject, projectXml => + { + projectXml.AddProperty(PropertyName, setPropertyValue); + }); + } +} diff --git a/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/KeyValuePairListEncodingTests.cs b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/KeyValuePairListEncodingTests.cs new file mode 100644 index 00000000000..b9c49436068 --- /dev/null +++ b/tests/Microsoft.VisualStudio.ProjectSystem.Managed.VS.UnitTests/ProjectSystem/VS/Properties/KeyValuePairListEncodingTests.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE.md file in the project root for more information. + +using Microsoft.VisualStudio.ProjectSystem.Debug; + +namespace Microsoft.VisualStudio.ProjectSystem.VS.Properties; + +public class KeyValuePairListEncodingTests +{ + [Theory] + [InlineData("key1=value1;key2=value2", true, new[] { "key1", "value1", "key2", "value2" })] + [InlineData("key1=value1;;key2=value2", true, new[] { "key1", "value1", "key2", "value2" })] + [InlineData("key1=value1;;;key2=value2", true, new[] { "key1", "value1", "key2", "value2" })] + [InlineData("key1=value1;key2=value2;key3=value3", true, new[] { "key1", "value1", "key2", "value2", "key3", "value3" })] + [InlineData("key1;key2=value2", true, new[] { "key1", "", "key2", "value2" })] + [InlineData("key1;key2;key3=value3", true, new[] { "key1", "", "key2", "", "key3", "value3" })] + [InlineData("key1;;;key3;;", true, new[] { "key1", "", "key3", "" })] + [InlineData("", true, new string[0])] + [InlineData(" ", true, new string[0])] + [InlineData("=", true, new string[0])] + [InlineData("", false, new string[0])] + [InlineData(" ", false, new string[0])] + [InlineData("=", false, new[] { "=", "" })] // = can count as part of the key here + [InlineData("key1=value1;=value2=", true, new[] { "key1", "value1", "", "value2=" })] + [InlineData("key1=value1;=value2=", false, new[] { "key1", "value1", "=value2", "" })] + [InlineData("key1=value1;=value2", false, new[] { "key1", "value1", "=value2", "" })] + [InlineData("==", true, new[] { "", "=" })] + [InlineData(";", true, new string[0])] + public void Parse_ValidInput_ReturnsExpectedPairs(string input, bool allowsEmptyKey, string[] expectedPairs) + { + var result = KeyValuePairListEncoding.Parse(input, allowsEmptyKey, ';').SelectMany(pair => new[] { pair.Name, pair.Value }).ToArray(); + Assert.Equal(expectedPairs, result); + } + + [Theory] + [InlineData(new[] { "key1", "value1", "key2", "value2" }, "key1=value1;key2=value2")] + [InlineData(new[] { "key1", "value1", "key2", "value2", "key3", "value3" }, "key1=value1;key2=value2;key3=value3")] + [InlineData(new[] { "key1", "", "key2", "value2" }, "key1;key2=value2")] + [InlineData(new[] { "key1", "", "key2", "", "key3", "value3" }, "key1;key2;key3=value3")] + [InlineData(new string[0], "")] + public void Format_ValidPairs_ReturnsExpectedString(string[] pairs, string expectedString) + { + var nameValuePairs = ToNameValues(pairs); + var result = KeyValuePairListEncoding.Format(nameValuePairs, ';'); + Assert.Equal(expectedString, result); + return; + + static IEnumerable<(string Name, string Value)> ToNameValues(IEnumerable pairs) + { + using var e = pairs.GetEnumerator(); + while (e.MoveNext()) + { + var name = e.Current; + Assert.True(e.MoveNext()); + var value = e.Current; + yield return (name, value); + } + } + } +}