Fix expanding template expressions from defaultDestination for CLI install

.editorconfig

@@ -82,7 +82,7 @@ dotnet_naming_style.start_underscore_style.required_prefix      = _
 [*.cs]
 # Don't prefer "var" when not apparent
 csharp_style_var_for_built_in_types                   = false : warning
-csharp_style_var_when_type_is_apparent                = true : suggestion
+csharp_style_var_when_type_is_apparent                = false : silent
 csharp_style_var_elsewhere                            = false : warning
 
 # Prefer method-like constructs to have a block body

CHANGELOG.md

@@ -6,6 +6,7 @@ Commit: TBD
 - Adds fileMappings support for filesystem provider
 - Fixes LIB016 error when using fileMappings
 - Fixes issue where restore-on-save in one project in VS removes files restored in a separate project
+- Fixes issue where CLI install command did not expand templated expressions in defaultDestination
 
 ## 3.0.71
 Commit: 33c04f70a4f55f1cddbaddad60fc78a282b298d3

src/LibraryManager/Json/LibraryStateToFileConverter.cs

@@ -6,6 +6,7 @@
 using System.Text;
 using Microsoft.Web.LibraryManager.Contracts;
 using Microsoft.Web.LibraryManager.LibraryNaming;
+using Microsoft.Web.LibraryManager.Utilities;
 
 namespace Microsoft.Web.LibraryManager.Json
 {
@@ -30,7 +31,7 @@ public ILibraryInstallationState ConvertToLibraryInstallationState(LibraryInstal
             string provider = string.IsNullOrEmpty(stateOnDisk.ProviderId) ? _defaultProvider : stateOnDisk.ProviderId;
 
             (string name, string version) = LibraryIdToNameAndVersionConverter.Instance.GetLibraryNameAndVersion(stateOnDisk.LibraryId, provider);
-            string destination = string.IsNullOrEmpty(stateOnDisk.DestinationPath) ? ExpandDestination(_defaultDestination, name, version) : stateOnDisk.DestinationPath;
+            string destination = string.IsNullOrEmpty(stateOnDisk.DestinationPath) ? PathTemplateUtility.ExpandPathTemplate(_defaultDestination, name, version) : stateOnDisk.DestinationPath;
 
             var state = new LibraryInstallationState()
             {
@@ -47,31 +48,6 @@ public ILibraryInstallationState ConvertToLibraryInstallationState(LibraryInstal
             return state;
         }
 
-        /// <summary>
-        /// Expands [Name] and [Version] tokens in the DefaultDestination
-        /// </summary>
-        /// <param name="destination">The default destination string</param>
-        /// <param name="name">Package name</param>
-        /// <param name="version">Package version</param>
-        /// <returns></returns>
-        [SuppressMessage("Globalization", "CA1307:Specify StringComparison for clarity", Justification = "Not available on net481, not needed here (caseless)")]
-        private string ExpandDestination(string destination, string name, string version)
-        {
-            if (destination is null || !destination.Contains("["))
-            {
-                return destination;
-            }
-
-            // if the name contains a slash (either filesystem or scoped packages),
-            // trim that and only take the last segment.
-            int cutIndex = name.LastIndexOfAny(['/', '\\']);
-
-            StringBuilder stringBuilder = new StringBuilder(destination);
-            stringBuilder.Replace("[Name]", cutIndex == -1 ? name : name.Substring(cutIndex + 1));
-            stringBuilder.Replace("[Version]", version);
-            return stringBuilder.ToString();
-        }
-
         public LibraryInstallationStateOnDisk ConvertToLibraryInstallationStateOnDisk(ILibraryInstallationState state)
         {
             if (state == null)

src/LibraryManager/Manifest.cs

@@ -12,6 +12,7 @@
 using Microsoft.Web.LibraryManager.Helpers;
 using Microsoft.Web.LibraryManager.Json;
 using Microsoft.Web.LibraryManager.LibraryNaming;
+using Microsoft.Web.LibraryManager.Utilities;
 using Newtonsoft.Json;
 
 namespace Microsoft.Web.LibraryManager
@@ -150,7 +151,7 @@ private static void UpdateLibraryProviderAndDestination(ILibraryInstallationStat
 
             if (libraryState.DestinationPath == null)
             {
-                libraryState.DestinationPath = defaultDestination;
+                libraryState.DestinationPath = PathTemplateUtility.ExpandPathTemplate(defaultDestination, state.Name, state.Version);
                 libraryState.IsUsingDefaultDestination = true;
             }
         }

src/LibraryManager/Utilities/PathTemplateUtility.cs

@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+
+namespace Microsoft.Web.LibraryManager.Utilities
+{
+    /// <summary>
+    /// Utility for path template operations.
+    /// </summary>
+    public static class PathTemplateUtility
+    {
+        /// <summary>
+        /// Expands a path template using [Name] and [Version] tokens.
+        /// </summary>
+        /// <param name="template">Template string</param>
+        /// <param name="name">Library name</param>
+        /// <param name="version">Library version</param>
+        [SuppressMessage("Globalization", "CA1307:Specify StringComparison for clarity", Justification = "Not available on net481, not needed here (caseless)")]
+        public static string ExpandPathTemplate(string template, string name, string version)
+        {
+            if (template is null || !template.Contains('['))
+            {
+                return template;
+            }
+
+            // if the name contains a slash (either filesystem or scoped packages),
+            // trim that and only take the last segment as the library name.
+            int cutIndex = name.LastIndexOfAny(['/', '\\']);
+            name = cutIndex == -1 ? name : name.Substring(cutIndex + 1);
+
+            return template.Replace("[Name]", name)
+                           .Replace("[Version]", version);
+        }
+    }
+}

src/libman/Commands/InstallCommand.cs

@@ -52,7 +52,6 @@ public InstallCommand(IHostEnvironment hostEnvironment, bool throwOnUnexpectedAr
         private IProvider _provider;
         private ILibraryCatalog _catalog;
 
-        private string InstallDestination { get; set; }
         private string ProviderId { get; set; }
 
         private IProvider ProviderToUse
@@ -116,40 +115,35 @@ protected override async Task<int> ExecuteInternalAsync()
                 _manifest.DefaultProvider = ProviderId;
             }
 
-            InstallDestination = Destination.HasValue() ? Destination.Value() : _manifest.DefaultDestination;
-            if (string.IsNullOrWhiteSpace(InstallDestination))
+            string installDestination = Destination.Value();
+            if (string.IsNullOrWhiteSpace(installDestination) && string.IsNullOrWhiteSpace(_manifest.DefaultDestination))
             {
+                // if there isn't a usable destination, prompt the user for one
                 string destinationHint = string.Join('/', Settings.DefaultDestinationRoot, ProviderToUse.GetSuggestedDestination(library));
-                InstallDestination = GetUserInputWithDefault(
+                installDestination = GetUserInputWithDefault(
                     fieldName: nameof(Destination),
                     defaultFieldValue: destinationHint,
                     optionLongName: Destination.LongName);
             }
 
-            string destinationToUse = Destination.Value();
-            if (string.IsNullOrWhiteSpace(_manifest.DefaultDestination) && string.IsNullOrWhiteSpace(destinationToUse))
-            {
-                destinationToUse = InstallDestination;
-            }
-
-            if (destinationToUse is not null)
+            if (installDestination is not null)
             {
                 // in case the user changed the suggested default, normalize separator to /
-                destinationToUse = destinationToUse.Replace('\\', '/');
+                installDestination = installDestination.Replace('\\', '/');
             }
 
             OperationResult<LibraryInstallationGoalState> result = await _manifest.InstallLibraryAsync(
                 library.Name,
                 library.Version,
                 providerIdToUse,
                 files,
-                destinationToUse,
+                installDestination,
                 CancellationToken.None);
 
             if (result.Success)
             {
                 await _manifest.SaveAsync(Settings.ManifestFileName, CancellationToken.None);
-                Logger.Log(string.Format(Resources.Text.InstalledLibrary, libraryId, InstallDestination), LogLevel.Operation);
+                Logger.Log(string.Format(Resources.Text.InstalledLibrary, libraryId, installDestination), LogLevel.Operation);
             }
             else
             {

test/libman.IntegrationTest/CliBaseTest.cs

@@ -23,14 +23,34 @@ public class CliTestBase
     [TestInitialize]
     public async Task TestInitialize()
     {
+        // Create a test directory for the project where we'll run the tool.  This isolates it from
+        // inheriting any build settings from our solution.
         _testDirectory = Path.Combine(Path.GetTempPath(), "LibmanTest" + Guid.NewGuid().ToString());
         Directory.CreateDirectory(_testDirectory);
 
-        // create an empty nuget.config with only our package source
-        await RunDotnetCommandLineAsync("new nugetconfig");
-        await RunDotnetCommandLineAsync("nuget remove source nuget");
-        await RunDotnetCommandLineAsync("nuget add source ./TestPackages");
-
+        // Create an empty nuget.config with only our package source
+        // We need to set packageSourceMappings to override the defaults.
+        // This is needed because external devs may need to override the root nuget.config
+        // to build (see https://github.com/aspnet/LibraryManager/issues/728), and those
+        // settings are inherited in the test directory.
+        string nugetConfigContent = """
+            <?xml version="1.0" encoding="utf-8"?>
+            <configuration>
+              <packageSources>
+                <clear />
+                <add key="LocalPackages" value="./TestPackages" />
+              </packageSources>
+              <packageSourceMapping>
+                <packageSource key="LocalPackages">
+                  <package pattern="Microsoft.*" />
+                </packageSource>
+              </packageSourceMapping>
+            </configuration>
+            """;
+        File.WriteAllText("nuget.config", nugetConfigContent);
+
+        // This installs the tool in the current (test) working directory, not the project directory
+        // created above.
         await InstallCliToolAsync();
     }
 

test/libman.IntegrationTest/InstallTests.cs

@@ -15,4 +15,21 @@ public async Task Install_FileSpecified()
 
         AssertFileExists("test/jquery/jquery.min.js");
     }
+
+    [TestMethod]
+    public async Task Install_UsingTemplateInDefaultDestination()
+    {
+        string manifest = """
+            {
+                "version": "3.0",
+                "defaultProvider": "cdnjs",
+                "defaultDestination": "wwwroot/lib/[Name]/"
+            }
+            """;
+        await CreateManifestFileAsync(manifest);
+
+        await ExecuteCliToolAsync("install bootstrap@5.3.2 --provider cdnjs --files css/bootstrap.min.css --files js/bootstrap.bundle.min.js");
+        AssertFileExists("wwwroot/lib/bootstrap/css/bootstrap.min.css");
+        AssertFileExists("wwwroot/lib/bootstrap/js/bootstrap.bundle.min.js");
+    }
 }