Simplify ProfilePathInfo and ensure profile variable is set during startup (#2287)

Refactor the ProfilePathInfo structure to use a record type for simplicity. Ensure the profile variable is set during the startup process, regardless of whether profiles are loaded. This is to make behavior consistent with ConsoleHost and pwsh -noprofile ($profile is available regardless of if profiles were actually loaded or not)

Author: JustinGrote

src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs

@@ -267,18 +267,12 @@ private HostStartupInfo CreateHostStartupInfo()
         {
             _logger.Log(PsesLogLevel.Debug, "Creating startup info object");
 
-            ProfilePathInfo profilePaths = null;
-            if (_config.ProfilePaths.AllUsersAllHosts != null
-                || _config.ProfilePaths.AllUsersCurrentHost != null
-                || _config.ProfilePaths.CurrentUserAllHosts != null
-                || _config.ProfilePaths.CurrentUserCurrentHost != null)
-            {
-                profilePaths = new ProfilePathInfo(
-                    _config.ProfilePaths.CurrentUserAllHosts,
-                    _config.ProfilePaths.CurrentUserCurrentHost,
-                    _config.ProfilePaths.AllUsersAllHosts,
-                    _config.ProfilePaths.AllUsersCurrentHost);
-            }
+            ProfilePathInfo profilePaths = new(
+                _config.ProfilePaths.CurrentUserAllHosts,
+                _config.ProfilePaths.CurrentUserCurrentHost,
+                _config.ProfilePaths.AllUsersAllHosts,
+                _config.ProfilePaths.AllUsersCurrentHost
+            );
 
             return new HostStartupInfo(
                 _config.HostInfo.Name,

src/PowerShellEditorServices/Hosting/HostStartupInfo.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
+#nullable enable
 
 using System;
 using System.Collections.Generic;
@@ -190,32 +191,13 @@ public HostStartupInfo(
     }
 
     /// <summary>
-    /// This is a strange class that is generally <c>null</c> or otherwise just has a single path
-    /// set. It is eventually parsed one-by-one when setting up the PowerShell runspace.
+    /// Stores profile information passed from Start-EditorServices to be used for loading profiles if configured
+    /// and for the $PROFILE variable in the initial session state.
     /// </summary>
-    /// <remarks>
-    /// TODO: Simplify this as a <see langword="record"/>.
-    /// </remarks>
-    public sealed class ProfilePathInfo
-    {
-        public ProfilePathInfo(
-            string currentUserAllHosts,
-            string currentUserCurrentHost,
-            string allUsersAllHosts,
-            string allUsersCurrentHost)
-        {
-            CurrentUserAllHosts = currentUserAllHosts;
-            CurrentUserCurrentHost = currentUserCurrentHost;
-            AllUsersAllHosts = allUsersAllHosts;
-            AllUsersCurrentHost = allUsersCurrentHost;
-        }
-
-        public string CurrentUserAllHosts { get; }
-
-        public string CurrentUserCurrentHost { get; }
-
-        public string AllUsersAllHosts { get; }
-
-        public string AllUsersCurrentHost { get; }
-    }
+    public readonly record struct ProfilePathInfo(
+        string CurrentUserAllHosts,
+        string CurrentUserCurrentHost,
+        string AllUsersAllHosts,
+        string AllUsersCurrentHost
+    );
 }

src/PowerShellEditorServices/Services/PowerShell/Host/PsesInternalHost.cs

@@ -306,12 +306,20 @@ public async Task<bool> TryStartAsync(HostStartOptions startOptions, Cancellatio
                 _logger.LogDebug("InitialWorkingDirectory set!");
             }
 
+            _logger.LogDebug("Setting profile variable...");
+            await SetProfileVariableAsync(cancellationToken).ConfigureAwait(false);
+            _logger.LogDebug("Profile variable set!");
+
             if (startOptions.LoadProfiles)
             {
                 _logger.LogDebug("Loading profiles...");
                 await LoadHostProfilesAsync(cancellationToken).ConfigureAwait(false);
                 _logger.LogDebug("Profiles loaded!");
             }
+            else
+            {
+                _logger.LogDebug("Profile loading skipped per configuration!");
+            }
 
             if (!string.IsNullOrEmpty(startOptions.ShellIntegrationScript))
             {
@@ -583,13 +591,35 @@ internal void DisableTranscribeOnly()
             }
         }
 
+        internal Task SetProfileVariableAsync(CancellationToken cancellationToken)
+        {
+            // If the CurrentUserCurrentHost profile is null then we cannot create the profile variable
+            if (_hostInfo.ProfilePaths.CurrentUserCurrentHost is null)
+            {
+                return Task.CompletedTask;
+            }
+
+            // NOTE: This is a special task run on startup!
+            return ExecuteDelegateAsync(
+            "SetProfileVariable",
+            executionOptions: null,
+            (pwsh, _) => pwsh.SetProfileVariable(_hostInfo.ProfilePaths),
+            cancellationToken);
+        }
+
         internal Task LoadHostProfilesAsync(CancellationToken cancellationToken)
         {
+            // If the CurrentUserCurrentHost profile is null then we cannot instantiate
+            if (_hostInfo.ProfilePaths.CurrentUserCurrentHost is null)
+            {
+                return Task.CompletedTask;
+            }
+
             // NOTE: This is a special task run on startup!
             return ExecuteDelegateAsync(
                 "LoadProfiles",
                 executionOptions: null,
-                (pwsh, _) => pwsh.LoadProfiles(_hostInfo.ProfilePaths),
+            (pwsh, _) => pwsh.LoadProfileScripts(_hostInfo.ProfilePaths),
                 cancellationToken);
         }
 
@@ -812,8 +842,9 @@ private void RunTopLevelExecutionLoop()
             {
                 // Make sure we execute any startup tasks first. These should be, in order:
                 // 1. Delegate to register psEditor variable
-                // 2. LoadProfiles delegate
-                // 3. Delegate to import PSEditModule
+                // 2. SetProfileVariable delegate
+                // 3. Optional LoadProfiles delegate
+                // 4. Delegate to import PSEditModule
                 while (_taskQueue.TryTake(out ISynchronousTask task))
                 {
                     task.ExecuteSynchronously(CancellationToken.None);

src/PowerShellEditorServices/Services/PowerShell/Utility/PowerShellExtensions.cs

@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
-
+#nullable enable
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
@@ -69,7 +69,7 @@ and not PSInvocationState.Failed
             pwsh.InvocationStateChanged += handler;
         }
 
-        public static Collection<TResult> InvokeAndClear<TResult>(this PowerShell pwsh, PSInvocationSettings invocationSettings = null)
+        public static Collection<TResult> InvokeAndClear<TResult>(this PowerShell pwsh, PSInvocationSettings? invocationSettings = null)
         {
             try
             {
@@ -81,7 +81,7 @@ public static Collection<TResult> InvokeAndClear<TResult>(this PowerShell pwsh,
             }
         }
 
-        public static void InvokeAndClear(this PowerShell pwsh, PSInvocationSettings invocationSettings = null)
+        public static void InvokeAndClear(this PowerShell pwsh, PSInvocationSettings? invocationSettings = null)
         {
             try
             {
@@ -93,13 +93,13 @@ public static void InvokeAndClear(this PowerShell pwsh, PSInvocationSettings inv
             }
         }
 
-        public static Collection<TResult> InvokeCommand<TResult>(this PowerShell pwsh, PSCommand psCommand, PSInvocationSettings invocationSettings = null)
+        public static Collection<TResult> InvokeCommand<TResult>(this PowerShell pwsh, PSCommand psCommand, PSInvocationSettings? invocationSettings = null)
         {
             pwsh.Commands = psCommand;
             return pwsh.InvokeAndClear<TResult>(invocationSettings);
         }
 
-        public static void InvokeCommand(this PowerShell pwsh, PSCommand psCommand, PSInvocationSettings invocationSettings = null)
+        public static void InvokeCommand(this PowerShell pwsh, PSCommand psCommand, PSInvocationSettings? invocationSettings = null)
         {
             pwsh.Commands = psCommand;
             pwsh.InvokeAndClear(invocationSettings);
@@ -193,7 +193,7 @@ public static void SetCorrectExecutionPolicy(this PowerShell pwsh, ILogger logge
             }
         }
 
-        public static void LoadProfiles(this PowerShell pwsh, ProfilePathInfo profilePaths)
+        public static void SetProfileVariable(this PowerShell pwsh, ProfilePathInfo profilePaths)
         {
             // Per the documentation, "the `$PROFILE` variable stores the path to the 'Current User,
             // Current Host' profile. The other profiles are saved in note properties of the
@@ -202,15 +202,24 @@ public static void LoadProfiles(this PowerShell pwsh, ProfilePathInfo profilePat
             // https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_profiles?view=powershell-7.1#the-profile-variable
             PSObject profileVariable = PSObject.AsPSObject(profilePaths.CurrentUserCurrentHost);
 
+            profileVariable.Members.Add(new PSNoteProperty(nameof(profilePaths.AllUsersAllHosts), profilePaths.AllUsersAllHosts));
+            profileVariable.Members.Add(new PSNoteProperty(nameof(profilePaths.AllUsersCurrentHost), profilePaths.AllUsersCurrentHost));
+            profileVariable.Members.Add(new PSNoteProperty(nameof(profilePaths.CurrentUserAllHosts), profilePaths.CurrentUserAllHosts));
+            profileVariable.Members.Add(new PSNoteProperty(nameof(profilePaths.CurrentUserCurrentHost), profilePaths.CurrentUserCurrentHost));
+
+            pwsh.Runspace.SessionStateProxy.SetVariable("PROFILE", profileVariable);
+        }
+
+        public static void LoadProfileScripts(this PowerShell pwsh, ProfilePathInfo profilePaths)
+        {
+            PSObject profileVariable = PSObject.AsPSObject(profilePaths.CurrentUserCurrentHost);
+
             PSCommand psCommand = new PSCommand()
                 .AddProfileLoadIfExists(profileVariable, nameof(profilePaths.AllUsersAllHosts), profilePaths.AllUsersAllHosts)
                 .AddProfileLoadIfExists(profileVariable, nameof(profilePaths.AllUsersCurrentHost), profilePaths.AllUsersCurrentHost)
                 .AddProfileLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserAllHosts), profilePaths.CurrentUserAllHosts)
                 .AddProfileLoadIfExists(profileVariable, nameof(profilePaths.CurrentUserCurrentHost), profilePaths.CurrentUserCurrentHost);
 
-            // NOTE: This must be set before the profiles are loaded.
-            pwsh.Runspace.SessionStateProxy.SetVariable("PROFILE", profileVariable);
-
             // NOTE: Because it's possible there are no profiles defined, we might have an empty
             // command. Since this is being executed directly, we can't rely on `ThrowOnError =
             // false` to avoid an exception here. Instead, we must just not execute it.
@@ -253,7 +262,7 @@ private static StringBuilder AddErrorString(this StringBuilder sb, ErrorRecord e
                 .AppendLine("Exception:")
                 .Append("    ").Append(error.Exception.ToString() ?? "<null>");
 
-            Exception innerException = error.Exception?.InnerException;
+            Exception? innerException = error.Exception?.InnerException;
             while (innerException != null)
             {
                 sb.AppendLine("InnerException:")

test/PowerShellEditorServices.Test/Services/Symbols/PSScriptAnalyzerTests.cs

@@ -29,7 +29,7 @@ public class PSScriptAnalyzerTests
                 profileId: "",
                 version: null,
                 psHost: null,
-                profilePaths: null,
+                profilePaths: default,
                 featureFlags: null,
                 additionalModules: null,
                 initialSessionState: null,

test/PowerShellEditorServices.Test/Session/PsesInternalHostTests.cs

@@ -105,23 +105,45 @@ public async Task CanCancelExecutionWithMethod()
         }
 
         [Fact]
-        public async Task CanHandleNoProfiles()
+        public async Task CanHandleMissingProfilePaths()
         {
-            // Call LoadProfiles with profile paths that won't exist, and assert that it does not
-            // throw PSInvalidOperationException (which it previously did when it tried to invoke an
-            // empty command).
+            // Call LoadProfileScripts with profile paths that won't exist, and assert that it does
+            // not throw PSInvalidOperationException (which it previously did when it tried to
+            // invoke an empty command).
             ProfilePathInfo emptyProfilePaths = new("", "", "", "");
             await psesHost.ExecuteDelegateAsync(
-                "LoadProfiles",
+                "SetProfileVariableAndLoadProfileScripts",
                 executionOptions: null,
                 (pwsh, _) =>
                 {
-                    pwsh.LoadProfiles(emptyProfilePaths);
+                    pwsh.SetProfileVariable(emptyProfilePaths);
+                    pwsh.LoadProfileScripts(emptyProfilePaths);
+
+                    Assert.Equal(emptyProfilePaths.CurrentUserCurrentHost, pwsh.Runspace.SessionStateProxy.GetVariable("PROFILE")?.ToString());
                     Assert.Empty(pwsh.Commands.Commands);
                 },
                 CancellationToken.None);
         }
 
+        [Fact]
+        public async Task SetsProfileVariableWhenProfilesAreNotLoaded()
+        {
+            // This host fixture starts with LoadProfiles = false. Ensure $PROFILE is still set.
+            IReadOnlyList<string> profileVariable = await psesHost.ExecutePSCommandAsync<string>(
+                new PSCommand().AddScript("$PROFILE"),
+                CancellationToken.None);
+
+            Assert.Collection(profileVariable,
+                (p) => Assert.Equal(PsesHostFactory.TestProfilePaths.CurrentUserCurrentHost, p));
+
+            // Ensure profile scripts were not loaded as part of startup.
+            IReadOnlyList<PSObject> profileLoadedCommand = await psesHost.ExecutePSCommandAsync<PSObject>(
+                new PSCommand().AddScript("Get-Command Assert-ProfileLoaded -ErrorAction Ignore"),
+                CancellationToken.None);
+
+            Assert.Empty(profileLoadedCommand);
+        }
+
         // NOTE: Tests where we call functions that use PowerShell runspaces are slightly more
         // complicated than one would expect because we explicitly need the methods to run on the
         // pipeline thread, otherwise Windows complains about the the thread's apartment state not