Merge pull request #4132 from brthor/brthor/script-env

Add Scripts environment variable support

src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateScriptsRule.cs

@@ -1,11 +1,13 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Microsoft.Build.Construction;
 using Microsoft.DotNet.Cli.Utils.CommandParsing;
 using Microsoft.DotNet.ProjectJsonMigration.Transforms;
+using Microsoft.DotNet.ProjectModel;
 
 namespace Microsoft.DotNet.ProjectJsonMigration.Rules
 {
@@ -75,8 +77,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
 
         internal string FormatScriptCommand(string scriptCommandline, string scriptExtensionPropertyName)
         {
-            var command = AddScriptExtensionPropertyToCommandLine(scriptCommandline, scriptExtensionPropertyName);
+            var command = ReplaceScriptVariables(scriptCommandline);
-            return ReplaceScriptVariables(command);
+            command = AddScriptExtensionPropertyToCommandLine(command, scriptExtensionPropertyName);
+            return command;
         }
 
         internal string AddScriptExtensionPropertyToCommandLine(string scriptCommandline,
@@ -113,27 +116,32 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             return command;
         }
 
-        internal string ReplaceScriptVariables(string command)
+        internal string ReplaceScriptVariables(string scriptCommandline)
         {
-            foreach (var scriptVariableEntry in ScriptVariableToMSBuildMap)
+            Func<string, string> scriptVariableReplacementDelegate = key =>
             {
-                var scriptVariableName = scriptVariableEntry.Key;
+                if (ScriptVariableToMSBuildMap.ContainsKey(key))
-                var msbuildMapping = scriptVariableEntry.Value;
-
-                if (command.Contains($"%{scriptVariableName}%"))
                 {
-                    if (msbuildMapping == null)
+                    if (ScriptVariableToMSBuildMap[key] == null)
                     {
                         MigrationErrorCodes.MIGRATE1016(
-                                $"{scriptVariableName} is currently an unsupported script variable for project migration")
+                                $"{key} is currently an unsupported script variable for project migration")
                             .Throw();
                     }
 
-                    command = command.Replace($"%{scriptVariableName}%", msbuildMapping);
+                    return ScriptVariableToMSBuildMap[key];
                 }
-            }
+                return $"$({key})";
+            };
 
-            return command;
+            var scriptArguments = CommandGrammar.Process(
+                scriptCommandline,
+                scriptVariableReplacementDelegate,
+                preserveSurroundingQuotes: true);
+
+            scriptArguments = scriptArguments.Where(argument => !string.IsNullOrEmpty(argument)).ToArray();
+
+            return string.Join(" ", scriptArguments);
         }
 
         private bool IsPathRootedForAnyOS(string path)
@@ -175,7 +183,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             { "postpublish", new TargetHookInfo(false, "Publish") }
         };
 
-        private static Dictionary<string, string> ScriptVariableToMSBuildMap => new Dictionary<string, string>()
+        private static Dictionary<string, string> ScriptVariableToMSBuildMap => new Dictionary<string, string>
         {
             { "compile:TargetFramework", null },  // TODO: Need Short framework name in CSProj
             { "compile:ResponseFile", null },     // Not migrated

test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateScripts.cs

@@ -73,7 +73,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         {
             var scriptMigrationRule = new MigrateScriptsRule();
             ProjectRootElement mockProj = ProjectRootElement.Create();
-            var commands = new string[] { "fakecommand" };
+            var commands = new[] { "fakecommand" };
 
             var target = scriptMigrationRule.MigrateScriptSet(mockProj, mockProj.AddPropertyGroup(), commands, scriptName);
 
@@ -90,7 +90,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
             var scriptMigrationRule = new MigrateScriptsRule();
             ProjectRootElement mockProj = ProjectRootElement.Create();
 
-            var commands = new string[] { "fakecommand1", "fakecommand2", "mockcommand3" };
+            var commands = new[] { "fakecommand1", "fakecommand2", "mockcommand3" };
             var commandsInTask = commands.ToDictionary(c => c, c => false);
 
             var target = scriptMigrationRule.MigrateScriptSet(mockProj, mockProj.AddPropertyGroup(), commands, scriptName);
@@ -122,7 +122,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
             var scriptMigrationRule = new MigrateScriptsRule();
             ProjectRootElement mockProj = ProjectRootElement.Create();
 
-            var commands = new string[] { "compile:FullTargetFramework", "compile:Configuration"};
+            var commands = new[] { "compile:FullTargetFramework", "compile:Configuration"};
 
             var target = scriptMigrationRule.MigrateScriptSet(mockProj, mockProj.AddPropertyGroup(), commands, scriptName);
             target.Tasks.Count().Should().Be(commands.Length);
@@ -176,7 +176,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         public void Migrated_ScriptSet_has_ScriptExtension_added_to_script_command(string scriptCommandline, string expectedOutputCommand)
         {
             var scriptMigrationRule = new MigrateScriptsRule();
-            ProjectRootElement mockProj = ProjectRootElement.Create();
 
             var formattedCommand = scriptMigrationRule.AddScriptExtensionPropertyToCommandLine(scriptCommandline,
                 "MigratedScriptExtension_1");
@@ -195,12 +194,18 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
             string expectedOutputCommandPrefix)
         {
             var scriptMigrationRule = new MigrateScriptsRule();
-            ProjectRootElement mockProj = ProjectRootElement.Create();
 
             var formattedCommand = scriptMigrationRule.FormatScriptCommand(scriptCommandline,
                 "MigratedScriptExtension_1");
 
             formattedCommand.Should().StartWith(expectedOutputCommandPrefix);
         }
+
+        [Fact]
+        public void Formatting_script_commands_replaces_unknown_variables_with_MSBuild_Property_for_environment_variable_support()
+        {
+            var scriptMigrationRule = new MigrateScriptsRule();
+            scriptMigrationRule.ReplaceScriptVariables($"%UnknownVariable%").Should().Be("$(UnknownVariable)");
+        }
     }
 }