Unify the managed and NativeAOT CLI parsers into one shared implementation

src/Cli/Microsoft.DotNet.Cli.Utils/MSBuildForwardingAppWithoutLogging.cs

@@ -9,41 +9,78 @@ namespace Microsoft.DotNet.Cli.Utils;
 
 internal sealed class MSBuildForwardingAppWithoutLogging
 {
+    /// <summary>
+    /// An override flag that determines whether to always execute MSBuild out-of-process. By default the managed dotnet CLI
+    /// prefers to execute MSBuild in-process to prevent needing to spawn another process' central/worker node,
+    /// but this flag can be used to force out-of-process execution.
+    /// </summary>
     private static readonly bool AlwaysExecuteMSBuildOutOfProc = Env.GetEnvironmentVariableAsBool("DOTNET_CLI_RUN_MSBUILD_OUTOFPROC");
+
+    /// <summary>
+    /// An override flag that determines whether to use the MSBuild server - a persistent central node that can serve
+    /// as a place to cache data and prevent re-doing CoreCLR startup/JITting for small builds.
+    /// By default, the MSBuild server is disabled due to stability/correctness concerns with some 1P tasks that keep static state around,
+    /// but it can be used by users that are confident they will not encounter those issues.
+    /// </summary>
     private static readonly bool UseMSBuildServer = Env.GetEnvironmentVariableAsBool("DOTNET_CLI_USE_MSBUILD_SERVER", false);
+
+    /// <summary>
+    /// What the SDK's opinion is on the default terminal logger. The SDK defaults to '<c>auto</c>' which will use the terminal logger if the output is going to a terminal, otherwise it will use the console logger.
+    /// Some users prefer to always use the legacy console logger, so this gives them a way to consistently do so.
+    /// </summary>
     private static readonly string? TerminalLoggerDefault = Env.GetEnvironmentVariable("DOTNET_CLI_CONFIGURE_MSBUILD_TERMINAL_LOGGER");
 
     public static string MSBuildVersion
     {
         get => Build.Evaluation.ProjectCollection.DisplayVersion;
     }
+
     private const string MSBuildExeName = "MSBuild.dll";
 
     private const string SdksDirectoryName = "Sdks";
 
+    /// <summary>
+    /// The SDK's default MSBuild verbosity level - we choose <see cref="VerbosityOptions.Minimal"/> as a good balance between information and terminal noise.
+    /// </summary>
     internal const VerbosityOptions DefaultVerbosity = VerbosityOptions.m;
 
-    // Null if we're running MSBuild in-proc.
+    /// <summary>
+    /// The forwarding app implementation for executing MSBuild out-of-process.
+    /// </summary>
+    /// <remarks>
+    /// This is null if we're running MSBuild in-process.
+    /// </remarks>
     private ForwardingAppImplementation? _forwardingApp;
 
+    /// <summary>
+    /// A test-only hook for the MSBuildExtensionsPath, which is a key location that MSBuild logic is read from by the MSBuild Common Targets.
+    /// </summary>
     internal static string? MSBuildExtensionsPathTestHook = null;
 
     /// <summary>
     /// Structure describing the parsed and forwarded MSBuild arguments for this command.
     /// </summary>
     private MSBuildArgs _msbuildArgs;
 
-    // Path to the MSBuild binary to use.
+    /// <summary>
+    /// Path to the MSBuild binary to use - this is set by constructor parameter or looked up via <see cref="GetMSBuildExePath"/>.
+    /// </summary>
     public string MSBuildPath { get; }
 
-    // True if, given current state of the class, MSBuild would be executed in its own process.
+    /// <summary>
+    /// True if, given current state of the class, MSBuild would be executed in its own process.
+    /// </summary>
     public bool ExecuteMSBuildOutOfProc => _forwardingApp != null;
 
+    /// <summary>
+    /// The set of environment variables that must be set on the MSBuild process (or the current
+    /// process when executing in-proc) for the build to behave correctly.
+    /// </summary>
     private readonly Dictionary<string, string?> _msbuildRequiredEnvironmentVariables = GetMSBuildRequiredEnvironmentVariables();
 
     private readonly List<string> _msbuildRequiredParameters = ["-maxcpucount", $"--verbosity:{DefaultVerbosity}"];
 
-    public MSBuildForwardingAppWithoutLogging(MSBuildArgs msbuildArgs, string? msbuildPath = null)
+    public MSBuildForwardingAppWithoutLogging(MSBuildArgs msbuildArgs, string? msbuildPath = null, bool forceOutOfProc = false)
     {
         string defaultMSBuildPath = GetMSBuildExePath();
         _msbuildArgs = msbuildArgs;
@@ -63,8 +100,9 @@ public MSBuildForwardingAppWithoutLogging(MSBuildArgs msbuildArgs, string? msbui
 
         EnvironmentVariable("MSBUILDUSESERVER", UseMSBuildServer ? "1" : "0");
 
-        // If DOTNET_CLI_RUN_MSBUILD_OUTOFPROC is set or we're asked to execute a non-default binary, call MSBuild out-of-proc.
-        if (AlwaysExecuteMSBuildOutOfProc || !string.Equals(MSBuildPath, defaultMSBuildPath, StringComparison.OrdinalIgnoreCase))
+        // If DOTNET_CLI_RUN_MSBUILD_OUTOFPROC is set, the caller requires it (e.g. the AOT CLI, which
+        // cannot host MSBuild in-process), or we're asked to execute a non-default binary, call MSBuild out-of-proc.
+        if (AlwaysExecuteMSBuildOutOfProc || forceOutOfProc || !string.Equals(MSBuildPath, defaultMSBuildPath, StringComparison.OrdinalIgnoreCase))
         {
             InitializeForOutOfProcForwarding();
         }
@@ -106,6 +144,9 @@ private static string EmitProperty(KeyValuePair<string, string> property, string
             : $"--{label}:{property.Key}={property.Value}";
     }
 
+    /// <summary>
+    /// Add an environment variable to the state that will be passed to MSBuild when it is run.
+    /// </summary>
     public void EnvironmentVariable(string name, string? value)
     {
         if (_forwardingApp != null)
@@ -129,6 +170,9 @@ public void EnvironmentVariable(string name, string? value)
         }
     }
 
+    /// <summary>
+    /// Run the MSBuild arguments that have been previously specified.
+    /// </summary>
     public int Execute()
     {
         if (_forwardingApp != null)
@@ -141,6 +185,11 @@ public int Execute()
         }
     }
 
+    /// <summary>
+    /// Directly executes MSBuild's <see cref="Build.CommandLine.MSBuildApp.Main"/> method in the current process.
+    /// Sets up the local environment with required MSBuild environment variables before handing off execution entirely to MSBuild.
+    /// After execution, the original environment variables are restored for any remaining cleanup work the dotnet CLI needs to perform.
+    /// </summary>
     public int ExecuteInProc(string[] arguments)
     {
         // Save current environment variables before overwriting them.
@@ -186,13 +235,22 @@ public int ExecuteInProc(string[] arguments)
     private static string Escape(string propertyValue) =>
         propertyValue.Replace(";", "%3B").Replace("://", ":%2F%2F");
 
+    /// <summary>
+    /// Gets the path to the MSBuild executable. By default, this will be the 'MSBuild.dll' file in the same location as the `dotnet.dll` binary.
+    /// </summary>
+    /// <returns></returns>
     private static string GetMSBuildExePath()
     {
         return Path.Combine(
             AppContext.BaseDirectory,
             MSBuildExeName);
     }
 
+    /// <summary>
+    /// Gets the path to the MSBuild SDKs directory - where the SDKs will be loaded from by the default, local-path-based SDK resolver.
+    /// By default, this will be the 'SDKs' directory in the same location as the `dotnet.dll` binary, but it can be overridden by the `MSBuildSDKsPath` environment variable.
+    /// </summary>
+    /// <returns></returns>
     public static string GetMSBuildSDKsPath()
     {
         var envMSBuildSDKsPath = Environment.GetEnvironmentVariable("MSBuildSDKsPath");
@@ -207,11 +265,17 @@ public static string GetMSBuildSDKsPath()
             SdksDirectoryName);
     }
 
-    private static string GetDotnetPath()
-    {
-        return new Muxer().MuxerPath;
-    }
+    private static string GetDotnetPath() => new Muxer().MuxerPath;
 
+    /// <summary>
+    /// Gets the required environment variables for MSBuild.
+    /// The Common Targets require specific environment variables to be set in order to function correctly:
+    /// <list type="bullet">
+    /// <item><term>MSBuildExtensionsPath</term><description>The path to the 'MSBuild extensions' - where the Common Targets themselves will be loaded from. Also where SDK Resolvers will be loaded from.</description></item>
+    /// <item><term>MSBuildSDKsPath</term><description>The path to the 'MSBuild SDKs' - where the SDKs will be loaded from by the default resolver. </description></item>
+    /// <item><term>DOTNET_HOST_PATH</term><description>The path to the .NET SDK host - used to execute .NET applications by targets in the Common Targets that need to run managed .NET binaries that are not shipped with apphosts.</description></item>
+    /// </list>
+    /// </summary>
     internal static Dictionary<string, string?> GetMSBuildRequiredEnvironmentVariables()
     {
         return new()

src/Cli/dn/dn-native-debug.vcxproj

@@ -145,7 +145,6 @@ DOTNET_CLI_ENABLEAOT=true</LocalDebuggerEnvironment>
     <None Include="..\dotnet-aot\ManagedHost.cs" Link="AOT Sources\ManagedHost.cs" />
     <None Include="Program.cs" Link="AOT Sources\dn.Program.cs" />
     <None Include="..\dotnet\Program.cs" Link="Managed Sources\dotnet.Program.cs" />
-    <None Include="..\dotnet\CommandLineInfo.cs" Link="AOT Sources\CommandLineInfo.cs" />
     <None Include="debug-dn.cmd" />
   </ItemGroup>
 

src/Cli/dotnet-aot/AotSourceFiles.props

@@ -25,8 +25,10 @@
 
   <!-- Common AOT scaffolding: shared by every AOT command. Reuse, do not duplicate. -->
   <ItemGroup>
-    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CommandLineInfo.cs" Link="CommandLineInfo.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CliUsage.cs" Link="CliUsage.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CliSchema.cs" Link="CliSchema.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Parser.cs" Link="Parser.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\ParserOptionActions.cs" Link="ParserOptionActions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CommandBase.cs" Link="CommandBase.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CommandParsingException.cs" Link="CommandParsingException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\CommandNotAvailableInAotException.cs" Link="CommandNotAvailableInAotException.cs" />
@@ -73,4 +75,24 @@
     <PackageReference Include="Microsoft.VisualStudio.SolutionPersistence" />
   </ItemGroup>
 
+  <!--
+    Forwarding-app help support: the shared DotnetHelpBuilder renders help for the external-tool
+    commands (msbuild/nuget/vstest/format/fsi) by shelling out to the underlying tool. These sources
+    use AOT-friendly (out-of-process) codepaths under #if CLI_AOT so the help writer needs no
+    conditional compilation of its own.
+  -->
+  <ItemGroup>
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\ForwardingApp.cs" Link="ForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\ICommandRunner.cs" Link="ICommandRunner.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\NuGetForwardingApp.cs" Link="NuGetForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\NuGetSignatureVerificationEnabler.cs" Link="NuGetSignatureVerificationEnabler.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\NuGet\NuGetCommand.cs" Link="Commands\NuGet\NuGetCommand.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\MSBuild\MSBuildForwardingApp.cs" Link="Commands\MSBuild\MSBuildForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\Run\CommonRunHelpers.cs" Link="Commands\Run\CommonRunHelpers.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\Fsi\FsiForwardingApp.cs" Link="Commands\Fsi\FsiForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\Format\FormatForwardingApp.cs" Link="Commands\Format\FormatForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\Test\VSTest\VSTestForwardingApp.cs" Link="Commands\Test\VSTest\VSTestForwardingApp.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)..\dotnet\Commands\Test\VSTest\VSTestTrace.cs" Link="Commands\Test\VSTest\VSTestTrace.cs" />
+  </ItemGroup>
+
 </Project>

src/Cli/dotnet-aot/DESIGN.md

@@ -94,10 +94,14 @@ A NativeAOT shared library (`NativeLib=Shared`) that exports a single
 `[UnmanagedCallersOnly]` entry point: `dotnet_execute`. This layer contains
 the dual-path dispatch logic.
 
-**Fast path** — When `DOTNET_CLI_ENABLEAOT=true`, the AOT bridge compiles a
-minimal `Parser` (guarded by `#if CLI_AOT`) that handles simple commands
-(`--version`, `--info`) entirely in native code. If the parser recognizes the
-command, it executes immediately and returns.
+**Fast path** — When `DOTNET_CLI_ENABLEAOT=true`, the AOT bridge builds the
+**full** command tree (the same `DotNetCommandDefinition` used by the managed
+CLI) so that parsing and `--help` match the managed CLI exactly. Commands that
+can run entirely in AOT (`--version`, `--info`, and the AOT-capable `sln`
+subcommands) execute immediately and return. Every other command is wired with
+a fallback action that throws `CommandNotAvailableInAotException`; the bridge
+catches it (and any unexpected parse-time failure) and transparently falls
+through to the managed CLI.
 
 **Slow path** — When `DOTNET_CLI_ENABLEAOT` is not set or the AOT parser does
 not handle the command, the bridge calls `ManagedHost.RunApp()`, which uses the
@@ -151,20 +155,28 @@ the appropriate implementation:
 <DefineConstants>$(DefineConstants);CLI_AOT</DefineConstants>
 
 <Compile Include="..\dotnet\Program.cs" Link="Program.cs" />
-<Compile Include="..\dotnet\CommandLineInfo.cs" Link="CommandLineInfo.cs" />
 <Compile Include="..\dotnet\Parser.cs" Link="Parser.cs" />
+<Compile Include="..\dotnet\ParserOptionActions.cs" Link="ParserOptionActions.cs" />
 ```
 
 In the shared files:
 
-- **`Parser.cs`** — Under `#if CLI_AOT`, defines a minimal parser with only
-  `--version` and `--info`. Under `#else`, defines the full command tree.
+- **`Parser.cs`** — A single shared `Parser` class builds the same full
+  `DotNetCommandDefinition` tree in both modes. Only the action wiring differs,
+  isolated to small inline `#if CLI_AOT` regions: the managed build wires the
+  real command handlers, while the AOT build attaches a managed-fallback handler
+  to every command (overriding it with real implementations where AOT can run
+  the command, e.g. `sln`). The help writer (`DotnetHelpBuilder`) has no
+  conditional compilation: help for the external-tool commands
+  (msbuild/nuget/vstest/format/fsi) renders from AOT because those forwarding
+  apps use AOT-friendly out-of-process codepaths under `#if CLI_AOT`.
 - **`Program.cs`** — Under `#if CLI_AOT`, provides a simple `Main` that
   delegates to the AOT parser. Under `#else`, provides the full CLI entry point
   with telemetry, signal handlers, and workload checks.
-- **`CommandLineInfo.cs`** — Uses `#if CLI_AOT` to substitute lightweight
-  implementations for workload info, localized strings, and OS detection that
-  would otherwise pull in dependencies incompatible with AOT.
+- **`ParserOptionActions.cs`** — The shared `--help`/`--version`/`--info` option
+  actions. `PrintInfoAction` uses `#if !CLI_AOT` to omit the workload and MSBuild
+  details that aren't AOT-compatible yet; the diagnostics and `--cli-schema`
+  actions are `#if !CLI_AOT` (the AOT build defers those to the managed CLI).
 
 ```mermaid
 graph LR