Merge pull request #26204 from baronfel/token-replacement

Revert to token-per-line response file handling

src/Cli/dotnet/CommonLocalizableStrings.resx

@@ -699,4 +699,7 @@ The default is 'true' if a runtime identifier is specified.</value>
   <data name="SelfContainAndNoSelfContainedConflict" xml:space="preserve">
     <value>The '--self-contained' and '--no-self-contained' options cannot be used together.</value>
   </data>
+  <data name="ResponseFileNotFound" xml:space="preserve">
+    <value>Response file '{0}' does not exist.</value>
+  </data>
 </root>

src/Cli/dotnet/CommonOptions.cs

@@ -17,7 +17,8 @@ namespace Microsoft.DotNet.Cli
     internal static class CommonOptions
     {
         public static Option<string[]> PropertiesOption =
-            new ForwardedOption<string[]>(new string[] { "-property", "/p" })
+            // these are all of the forms that the property switch can be understood by in MSBuild
+            new ForwardedOption<string[]>(new string[] { "--property", "-property", "/property", "/p", "-p", "--p" })
             {
                 IsHidden = true
             }.ForwardAsProperty()

src/Cli/dotnet/OptionForwardingExtensions.cs

@@ -19,7 +19,11 @@ public static class OptionForwardingExtensions
         public static ForwardedOption<T> ForwardAsSingle<T>(this ForwardedOption<T> option, Func<T, string> format) => option.SetForwardingFunction(format);
 
         public static ForwardedOption<string[]> ForwardAsProperty(this ForwardedOption<string[]> option) => option
-            .SetForwardingFunction((optionVals) => optionVals.SelectMany(optionVal => new string[] { $"{option.Aliases.FirstOrDefault()}:{optionVal.Replace("roperty:", string.Empty)}" }));
+            .SetForwardingFunction((optionVals) =>
+                optionVals
+                    .Select(optionVal => optionVal.Replace(";", "%3B")) // must escape semicolon-delimited property values when forwarding them to MSBuild
+                    .Select(optionVal => $"{option.Aliases.FirstOrDefault()}:{optionVal}")
+                );
 
         public static Option<T> ForwardAsMany<T>(this ForwardedOption<T> option, Func<T, IEnumerable<string>> format) => option.SetForwardingFunction(format);
 

src/Cli/dotnet/Parser.cs

@@ -109,6 +109,39 @@ public static Command GetBuiltInCommand(string commandName)
                 .FirstOrDefault(c => c.Name.Equals(commandName, StringComparison.OrdinalIgnoreCase));
         }
 
+        /// <summary>
+        /// Implements token-per-line response file handling for the CLI. We use this instead of the built-in S.CL handling
+        /// to ensure backwards-compatibility with MSBuild.
+        /// </summary>
+        public static bool TokenPerLine(string tokenToReplace, out IReadOnlyList<string> replacementTokens, out string errorMessage) {
+            var filePath = Path.GetFullPath(tokenToReplace);
+            if (File.Exists(filePath)) {
+                var lines = File.ReadAllLines(filePath);
+                var trimmedLines =
+                    lines
+                        // Remove content in the lines that contain #, starting from the point of the #
+                        .Select(line => {
+                            var hashPos = line.IndexOf('#');
+                            if (hashPos == -1) {
+                                return line;
+                            } else if (hashPos == 0) {
+                                return "";
+                            } else {
+                                return line.Substring(0, hashPos).Trim();
+                            }
+                        })
+                        // Remove empty lines
+                        .Where(line => line.Length > 0);
+                replacementTokens = trimmedLines.ToArray();
+                errorMessage = null;
+                return true;
+            } else {
+                replacementTokens = null;
+                errorMessage = string.Format(CommonLocalizableStrings.ResponseFileNotFound, tokenToReplace);
+                return false;
+            }
+        }
+
         public static System.CommandLine.Parsing.Parser Instance { get; } = new CommandLineBuilder(ConfigureCommandLine(RootCommand))
             .UseExceptionHandler(ExceptionHandler)
             .UseHelp()
@@ -118,6 +151,7 @@ public static Command GetBuiltInCommand(string commandName)
             .UseSuggestDirective()
             .DisablePosixBinding()
             .EnableLegacyDoubleDashBehavior()
+            .UseTokenReplacer(TokenPerLine)
             .Build();
 
         private static void ExceptionHandler(Exception exception, InvocationContext context)

src/Cli/dotnet/commands/RestoringCommand.cs

@@ -75,7 +75,7 @@ private static IEnumerable<string> Prepend(string argument, IEnumerable<string>
         private static bool HasArgumentToExcludeFromRestore(IEnumerable<string> arguments)
             => arguments.Any(a => IsExcludedFromRestore(a));
 
-        private static readonly string[] propertyPrefixes = new string[]{ "-", "/" };
+        private static readonly string[] propertyPrefixes = new string[]{ "-", "/", "--" };
 
         private static bool IsExcludedFromRestore(string argument)
             => propertyPrefixes.Any(prefix => argument.StartsWith($"{prefix}property:TargetFramework=", StringComparison.Ordinal)) ||

src/Cli/dotnet/commands/dotnet-complete/ParseCommand.cs

@@ -11,26 +11,29 @@ public static int Run(ParseResult result)
         {
             result.HandleDebugSwitch();
 
-            Console.WriteLine(result.Diagram());
+            var tokens = result.Tokens.Skip(1).Select(t => t.Value).ToArray();
+            var reparsed = Microsoft.DotNet.Cli.Parser.Instance.Parse(tokens);
+            Console.WriteLine(reparsed.Diagram());
 
-            if (result.UnparsedTokens.Any())
+
+            if (reparsed.UnparsedTokens.Any())
             {
                 Console.WriteLine("Unparsed Tokens: ");
-                Console.WriteLine(string.Join(" ", result.UnparsedTokens));
+                Console.WriteLine(string.Join(" ", reparsed.UnparsedTokens));
             }
 
-            var optionValuesToBeForwarded = result.OptionValuesToBeForwarded(ParseCommandParser.GetCommand());
+            var optionValuesToBeForwarded = reparsed.OptionValuesToBeForwarded(ParseCommandParser.GetCommand());
             if (optionValuesToBeForwarded.Any())
             {
                 Console.WriteLine("Option values to be forwarded: ");
                 Console.WriteLine(string.Join(" ", optionValuesToBeForwarded));
             }
-            if (result.Errors.Any())
+            if (reparsed.Errors.Any())
             {
                 Console.WriteLine();
                 Console.WriteLine("ERRORS");
                 Console.WriteLine();
-                foreach (var error in result.Errors)
+                foreach (var error in reparsed.Errors)
                 {
                     Console.WriteLine(error?.Message);
                 }

src/Cli/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs

@@ -57,6 +57,8 @@ public MSBuildForwardingApp(IEnumerable<string> argsToForward, string msbuildPat
             }
         }
 
+        public IEnumerable<string> MSBuildArguments { get { return _forwardingAppWithoutLogging.GetAllArguments(); } }
+
         public void EnvironmentVariable(string name, string value)
         {
             _forwardingAppWithoutLogging.EnvironmentVariable(name, value);

src/Cli/dotnet/xlf/CommonLocalizableStrings.cs.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Balíček</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Možnosti --self-contained a --no-self-contained nelze použít společně.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.de.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Paket</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Die Optionen „--self-contained“ und „--no-self-contained“ können nicht gemeinsam verwendet werden.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.es.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Paquete</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Las opciones '--self-contained' y '--no-self-contained' no se pueden usar juntas.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.fr.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Package</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Les options « --autonome » et « --non-autonome » ne peuvent pas être utilisées ensemble.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.it.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Pacchetto</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Non è possibile usare contemporaneamente le opzioni '--self-contained' e ‘--no-self-contained'.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.ja.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">パッケージ</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">'--self-contained' と '--no-self-contained' オプションは同時に使用できません。</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.ko.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">패키지</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">'--self-contained' 및 '--no-self-contained' 옵션은 함께 사용할 수 없습니다.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.pl.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Pakiet</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Opcji „--self-contained” i „--no-self-contained” nie można używać razem.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.pt-BR.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Pacote</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">As opções '--auto--contidas ' e '--não-independentes ' não podem ser usadas juntas.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.ru.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Пакет</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">Параметры "--self-contained" и "--no-self-contained" нельзя использовать вместе.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.tr.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">Paket</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">'--self-contained' ve '--no-self-contained' seçenekleri birlikte kullanılamaz.</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.zh-Hans.xlf

@@ -165,6 +165,11 @@ EOF
         <target state="translated">打包</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">“--自包含”和“--非自包含”选项不能一起使用。</target>

src/Cli/dotnet/xlf/CommonLocalizableStrings.zh-Hant.xlf

@@ -165,6 +165,11 @@ export PATH="$PATH:{0}"
         <target state="translated">套件</target>
         <note />
       </trans-unit>
+      <trans-unit id="ResponseFileNotFound">
+        <source>Response file '{0}' does not exist.</source>
+        <target state="new">Response file '{0}' does not exist.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="SelfContainAndNoSelfContainedConflict">
         <source>The '--self-contained' and '--no-self-contained' options cannot be used together.</source>
         <target state="translated">不能同時使用 '--self-contained' 和 '--no-self-contained' 選項。</target>

src/Tests/dotnet.Tests/ParserTests/MSBuildArgumentCommandLineParserTests.cs

@@ -2,9 +2,12 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using FluentAssertions;
+using Microsoft.DotNet.Cli;
 using Microsoft.DotNet.Tools;
 using Microsoft.DotNet.Tools.Build;
 using Microsoft.DotNet.Tools.Publish;
+using System.CommandLine;
+using System.Linq;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -22,18 +25,35 @@ public MSBuildArgumentCommandLineParserTests(ITestOutputHelper output)
         [Theory]
         [InlineData(new string[] { "-property:prop1=true", "-p:prop2=false" }, true)]
         [InlineData(new string[] { "-property:prop1=true", "-p:prop2=false" }, false)]
+        [InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, false)]
+        [InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, true)]
         [InlineData(new string[] { "-detailedSummary" }, true)]
         [InlineData(new string[] { "-clp:NoSummary" }, true)]
         [InlineData(new string[] { "-orc" }, true)]
         [InlineData(new string[] { "-orc" }, false)]
         public void MSBuildArgumentsAreForwardedCorrectly(string[] arguments, bool buildCommand)
         {
-            RestoringCommand command = buildCommand ? 
-                BuildCommand.FromArgs(arguments) : 
+            RestoringCommand command = buildCommand ?
+                BuildCommand.FromArgs(arguments) :
                 PublishCommand.FromArgs(arguments);
-            command.GetArgumentsToMSBuild().Split(' ')
-                .Should()
-                .Contain(arguments);
+            var expectedArguments = arguments.Select(a => a.Replace("-property:", "--property:").Replace("-p:", "--property:"));
+            var argString = command.MSBuildArguments;
+
+            foreach (var expectedArg in expectedArguments)
+            {
+                argString.Should().Contain(expectedArg);
+            }
+        }
+
+        [Theory]
+        [InlineData(new string[] { "-p:teamcity_buildConfName=\"Build, Test and Publish\"" }, new string[] { "--property:teamcity_buildConfName=\"Build, Test and Publish\"" })]
+        [InlineData(new string[] { "-p:prop1=true", "-p:prop2=false" }, new string[] { "--property:prop1=true", "--property:prop2=false" })]
+        [InlineData(new string[] { "-p:prop1=\".;/opt/usr\"" }, new string[] { "--property:prop1=\".%3B/opt/usr\"" })]
+        public void Can_pass_msbuild_properties_safely(string[] tokens, string[] forwardedTokens) {
+            var forwardingFunction = (CommonOptions.PropertiesOption as ForwardedOption<string[]>).GetForwardingFunction();
+            var result = CommonOptions.PropertiesOption.Parse(tokens);
+            var parsedTokens = forwardingFunction(result);
+            parsedTokens.Should().BeEquivalentTo(forwardedTokens);
         }
     }
 }