dotnet / dotnet/sdk

Use MSBuild's command-line parser instead of hand-maintained switch allowlists

Open
#56,123 1 comment 0 reactions 0 assignees View on GitHub
untriaged
Dominant language
C#
Stars
3.2k
Forks
1.3k
PR merge metrics
PR metrics pending

Description

## Summary

The SDK decides which command-line tokens belong to MSBuild and which belong to the process a verb launches. It does this in four different places, with three different policies, none of which consult MSBuild's own parser. Every time MSBuild adds a switch, the SDK silently misroutes it until someone notices and patches an allowlist.

MSBuild already exposes the parsing routine needed to stop this. dotnet/msbuild#12836 extracted the command-line parsing out of `XMake` into a reusable `CommandLineParser`, and `src/MSBuild/AssemblyInfo.cs` carries `[assembly: InternalsVisibleTo("dotnet", ...)]` specifically so the SDK can call it:

```csharp
Microsoft.Build.CommandLine.Experimental.CommandLineParser
.Parse(IEnumerable args) -> CommandLineSwitchesAccessor
```

`CommandLineSwitchesAccessor` exposes typed properties for every switch — `MultiThreaded`, `NodeReuse`, `MaxCpuCount`, `GraphBuild`, `IsolateProjects`, `Restore`, `BinaryLogger`, `TerminalLogger`, and so on.

## Current state

| Location | Policy |
| --- | --- |
| `LoggerUtility.SeparateLoggerArguments` | Hardcoded allowlist: `bl`, `noConsoleLogger`, `tl`, `ll`, `tlp`, `clp`. Used by `build`/`publish`/`pack`/`clean`/`restore`, `run`, and `test` under the Microsoft.Testing.Platform runner. |
| `Test/VSTest/TestCommand.cs` | Forwards all unmatched tokens to MSBuild. |
| `dotnet-watch` `CommandLineOptions.GetCommandArguments` | Modeled forwarding options only, plus a hand-extracted `-bl` (`s_binaryLogOptionNames`). |
| `MSBuildArgs.AnalyzeMSBuildArguments` | Re-parses tokens against a small hand-built set of `Option` objects. |

## Consequences

Bugs in this class are found one switch at a time:

- #52229 — terminal logger switches were passed to the test application instead of MSBuild. Fixed by adding entries to the `LoggerUtility` allowlist.
- #49989 — `dotnet watch` had no forwarding option for `-bl`, worked around with a bespoke token scan in `CommandLineOptions`.
- #56120 — `-mt` / `-multiThreaded` was passed to the test application and to the launched app instead of MSBuild. Fixed by adding entries to the same allowlist, plus a second fix in `dotnet watch` mirroring its `-bl` workaround.

Each fix is a separate patch to a separate allowlist. Switches such as `-nr`/`-nodeReuse`, `-graph`, and `-isolate` are still misrouted today by the verbs that route unmatched tokens away from MSBuild, and any switch MSBuild adds in the future starts out broken.

## Proposal

Replace the hand-maintained allowlists with a single classification helper backed by `CommandLineParser`, and have all four call sites use it. The routing policy for each verb — which recognized MSBuild switches a verb should intercept rather than forward to the launched process — stays an SDK decision, but "is this an MSBuild switch at all" stops being one.

## Open questions

Two properties of the current API make it unusable as a drop-in, and would need to be addressed on the MSBuild side first:

1. **`Parse` throws on unrecognized switches.** It ends in `CommandLineSwitches.ThrowErrors()`, and unknown tokens raise `UnknownSwitchError`. The SDK's use case is the opposite: a token MSBuild does not recognize is a valid argument for the test application or the launched app. This needs a non-throwing mode that reports unrecognized tokens rather than failing.

2. **`Parse` reads response files.** It calls `GatherAllSwitches`, which walks parent directories looking for `Directory.Build.rsp` and `MSBuild.rsp`. For classifying tokens that the SDK has already received, that is unwanted I/O, and it would make argument routing depend on response file contents. A parse-only overload would be needed.

Raising as a design discussion rather than a concrete work item, since the shape depends on what MSBuild is willing to expose.

/cc @baronfel

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reviewing LoggerUtility.SeparateLoggerArguments, Test/VSTest/TestCommand.cs, dotnet-watch CommandLineOptions.GetCommandArguments, and MSBuildArgs.AnalyzeMSBuildArguments. Then investigate the proposed CommandLineParser API and its unknown-switch and response-file behavior. Done requires an agreed MSBuild-side parsing interface and a classification approach that all four SDK call sites can use.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
cli, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.