From 873818f51fb9854602dca3f0123a1135f31218e6 Mon Sep 17 00:00:00 2001
From: Ignacio Errico <ignacio.errico@gmail.com>
Date: Thu, 20 Jan 2022 15:07:37 -0300
Subject: [PATCH] #1091 Quote path to DotNetWatch.targets (#1210)

* #1091 Quote path to DotNetWatch.targets

- Since the path is passed as a command-line parameter to dotnet, it needs to be enclosed in quotation marks in case it contains a space.

* #1091 Minor refactoring

- Apply SRP to the methods: now each method has its own responsibility, it's clear what the arguments passed to `dotnet` are, and the quotation marks are where they should be, making their intent clear.

Co-authored-by: Ignacio Errico <pedro.errico@ey.com>
---
 .../Watch/Internal/MsBuildFileSetFactory.cs   | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/src/Microsoft.Tye.Hosting/Watch/Internal/MsBuildFileSetFactory.cs b/src/Microsoft.Tye.Hosting/Watch/Internal/MsBuildFileSetFactory.cs
index 07321ee8c..0b13da002 100644
--- a/src/Microsoft.Tye.Hosting/Watch/Internal/MsBuildFileSetFactory.cs
+++ b/src/Microsoft.Tye.Hosting/Watch/Internal/MsBuildFileSetFactory.cs
@@ -19,10 +19,10 @@ public class MsBuildFileSetFactory : IFileSetFactory
     {
         private const string TargetName = "GenerateWatchList";
         private const string WatchTargetsFileName = "DotNetWatch.targets";
+        private readonly bool _waitOnError;
         private readonly ILogger _logger;
         private readonly string _projectFile;
-        private readonly bool _waitOnError;
-        private readonly IReadOnlyList<string> _buildFlags;
+        private readonly bool _trace;
 
         public MsBuildFileSetFactory(ILogger reporter,
             string projectFile,
@@ -39,7 +39,7 @@ internal MsBuildFileSetFactory(ILogger logger,
         {
             _logger = logger;
             _projectFile = projectFile;
-            _buildFlags = InitializeArgs(FindTargetsFile(), trace);
+            _trace = trace;
         }
 
         public async Task<IFileSet> CreateAsync(CancellationToken cancellationToken)
@@ -53,19 +53,11 @@ public async Task<IFileSet> CreateAsync(CancellationToken cancellationToken)
                 {
                     cancellationToken.ThrowIfCancellationRequested();
 
-                    var args = new StringBuilder();
-                    args.Append($"msbuild \"{_projectFile}\" /p:_DotNetWatchListFile=\"{watchList}\"");
-                    foreach (var flag in _buildFlags)
-                    {
-                        args.Append(" ");
-                        args.Append(flag);
-                    }
-
                     var processSpec = new ProcessSpec
                     {
                         Executable = "dotnet",
-                        WorkingDirectory = projectDir!,
-                        Arguments = args.ToString()
+                        Arguments = GetArgs(watchList!),
+                        WorkingDirectory = projectDir!
                     };
 
                     _logger.LogDebug($"Running MSBuild target '{TargetName}' on '{_projectFile}'");
@@ -128,26 +120,30 @@ public async Task<IFileSet> CreateAsync(CancellationToken cancellationToken)
             }
         }
 
-        private IReadOnlyList<string> InitializeArgs(string watchTargetsFile, bool trace)
+        private string GetArgs(string watchList)
         {
+            var watchTargetsFile = FindTargetsFile();
+
             var args = new List<string>
             {
+                $"msbuild \"{_projectFile}\"",
+                $"/p:_DotNetWatchListFile=\"{watchList}\"",
                 "/nologo",
                 "/v:n",
-                "/t:" + TargetName,
+                $"/t:{TargetName}",
                 "/p:DotNetWatchBuild=true", // extensibility point for users
                 "/p:DesignTimeBuild=true", // don't do expensive things
-                "/p:CustomAfterMicrosoftCommonTargets=" + watchTargetsFile,
-                "/p:CustomAfterMicrosoftCommonCrossTargetingTargets=" + watchTargetsFile,
+                $"/p:CustomAfterMicrosoftCommonTargets=\"{watchTargetsFile}\"",
+                $"/p:CustomAfterMicrosoftCommonCrossTargetingTargets=\"{watchTargetsFile}\"",
             };
 
-            if (trace)
+            if (_trace)
             {
                 // enables capturing markers to know which projects have been visited
                 args.Add("/p:_DotNetWatchTraceOutput=true");
             }
 
-            return args;
+            return string.Join(" ", args);
         }
 
         private string FindTargetsFile()
@@ -164,9 +160,10 @@ private string FindTargetsFile()
             var targetPath = searchPaths.Select(p => Path.Combine(p, WatchTargetsFileName)).FirstOrDefault(File.Exists);
             if (targetPath == null)
             {
-                _logger.LogError("Fatal error: could not find DotNetWatch.targets");
+                _logger.LogError($"Fatal error: could not find {WatchTargetsFileName}");
                 return null!;
             }
+
             return targetPath;
         }
     }