Allow null arguments in ProcessStartInfo constructor and Process.Start

[Copilot]

Nullability-only annotation update: arguments in ProcessStartInfo(string, string) and Process.Start(string, string) was typed as non-nullable but silently accepted null at runtime (stored as null, getter returns string.Empty via ??). This aligns the public API contract with actual behavior.

Changes

• ProcessStartInfo.cs — string arguments → string? arguments in the two-arg constructor
• Process.cs — string arguments → string? arguments in Process.Start(string, string)
• ref/System.Diagnostics.Process.cs — updated reference assembly to match
• ProcessStartInfoTests.cs — added TestArgumentsNullProperty covering null passed to constructor and assigned via property

// Previously caused nullable warnings; now correct
var psi = new ProcessStartInfo("dotnet", null);
var p = Process.Start("dotnet", null);

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Allow ProcessStartInfo and Process.Start arguments to be null</issue_title>
<issue_description>The arguments in ProcessStartInfo / Process.Start are currently not allowed to be null but it is ok if they are null.
E.g.

runtime/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs

Line 52 in 35b1dd2

public ProcessStartInfo(string fileName, string arguments)

runtime/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs

Line 1316 in 35b1dd2

public static Process Start(string fileName, string arguments)

Proposed API changes:

- public ProcessStartInfo(string fileName, string arguments) 
+ public ProcessStartInfo(string fileName, string? arguments) 
- public static Process Start(string fileName, string arguments) 
+ public static Process Start(string fileName, string? arguments) 
```</issue_description>

<agent_instructions>Make following nullable annotations update for `ProcessStartInfo` class from the System.Diagnostics.Process project:

```diff
namespace System.Diagnostics;

public partial class Process
{
- public ProcessStartInfo(string fileName, string arguments) 
+ public ProcessStartInfo(string fileName, string? arguments) 
- public static Process Start(string fileName, string arguments) 
+ public static Process Start(string fileName, string? arguments) 
}

• update the reference file that can be found here
• update the implementation in the System.Diagnostics.Process project here
• ensure that providing a null as arguments is covered with unit tests in to src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@stephentoub > but it is ok if they are null

What do you mean?</comment_new>
<comment_new>@adamsitnik
I am fine with the proposed API changes:

- public ProcessStartInfo(string fileName, string arguments) 
+ public ProcessStartInfo(string fileName, string? arguments) 
- public static Process Start(string fileName, string arguments) 
+ public static Process Start(string fileName, string? arguments) 

I'll mark it as ready for review so the API review board can discuss it along with other proposed Process related APIs later this week</comment_new>
<comment_new>@bartonjs
Video

Nullablity-only: Looks good as proposed

namespace System.Diagnostics;

public partial class Process
{
- public ProcessStartInfo(string fileName, string arguments) 
+ public ProcessStartInfo(string fileName, string? arguments) 
- public static Process Start(string fileName, string arguments) 
+ public static Process Start(string fileName, string? arguments) 
}
```</body></comment_new>
</comments>

• Fixes Allow ProcessStartInfo and Process.Start arguments to be null #124093

---

⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.

[Copilot]

Pull request overview

Updates System.Diagnostics.Process APIs to allow arguments to be nullable (string?) for the ProcessStartInfo(string, ...) ctor and Process.Start(string, ...) overload, aligning annotations with intended behavior.

Changes:

• Annotate ProcessStartInfo(string fileName, string? arguments) to allow null arguments.
• Annotate Process.Start(string fileName, string? arguments) to allow null arguments.
• Update the reference assembly and add a unit test for null Arguments.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs	Makes the (string fileName, string? arguments) ctor accept nullable arguments.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs	Makes Process.Start(string, string? arguments) accept nullable arguments and forwards to ProcessStartInfo.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates public API surface in ref to match new nullability annotations.
src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs	Adds coverage asserting null arguments round-trip as string.Empty.

[adamsitnik]

LGTM assuming the tests are going to pass

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[github-actions]

Note

This review was generated by Copilot.

🤖 Copilot Code Review — PR #126076

Holistic Assessment

Motivation: Justified. The Arguments property already accepts null via [AllowNull] and the backing field is string?, but the constructor and Process.Start overload rejected null at the type level — an inconsistency that produces spurious NRT warnings for callers. Issue #124093 documents the problem clearly.

Approach: Correct and minimal. The change updates only the two signatures in the approved API proposal, and the existing null-coalescing in the Arguments getter (_arguments ?? string.Empty) means null is already handled safely downstream.

Summary: ⚠️ Needs Human Review. The code change itself is correct and safe. Two items warrant human attention: (1) confirming the api-approved label on issue #124093 (I verified the issue content matches exactly, but could not programmatically confirm the label), and (2) a minor test coverage gap where the Process.Start(string, string?) entry point isn't directly exercised.

---

Detailed Findings

✅ Correctness — Null is safely handled at all consumption points

The _arguments backing field was already string?. The Arguments property getter returns string.Empty for null. All downstream consumers use string.IsNullOrEmpty() guards before processing arguments:

• Process.cs:1263 — validation check
• Process.Unix.cs:577, 596 — ParseArgv null-guards
• ProcessStartInfo.cs:207 — AppendArgumentsTo null-guard

No null-safety risk from this change. Flagged by: Claude, GPT-5.4.

✅ API Approval — Proposed shape matches issue #124093

Issue #124093 proposes exactly these two signature changes:

- public ProcessStartInfo(string fileName, string arguments)
+ public ProcessStartInfo(string fileName, string? arguments)
- public static Process Start(string fileName, string arguments)
+ public static Process Start(string fileName, string? arguments)

The ref/ assembly changes match the proposal exactly. Human reviewer: please confirm #124093 carries the api-approved label — I was unable to verify the label programmatically.

⚠️ Testing — Process.Start(string, string?) not directly tested (advisory, non-blocking)

The new TestArgumentsNullProperty test covers the ProcessStartInfo constructor with null and the Arguments property setter with null, which is good. However, the other changed public API — Process.Start(string fileName, string? arguments) — is not directly exercised with a null argument.

The risk is low since Process.Start(fileName, null) is a one-liner that delegates to new ProcessStartInfo(fileName, null) which IS tested. That said, testing the public entry point directly would be more thorough and could catch future regressions if the delegation changes. Consider adding a test that calls Process.Start("someExe", (string?)null) — even if it throws Win32Exception for a nonexistent executable, it would confirm the null argument doesn't cause an unexpected NullReferenceException or ArgumentNullException. Flagged by: Claude, GPT-5.4.

💡 Consistency — Windows-specific overload still has non-nullable arguments (follow-up)

Process.Start(string fileName, string arguments, string userName, SecureString password, string domain) in Process.Windows.cs:60 and its ref entry at line 173 still use non-nullable string arguments. This overload also delegates to new ProcessStartInfo(fileName, arguments), so it would work fine with null at runtime, but the type-level contract is now inconsistent with the two-parameter overload.

This is not a defect in this PR — the overload is not part of the approved API in #124093. If desired, it could be proposed as a follow-up. Flagged by: Claude, GPT-5.4.

💡 Test style — Consider merging with existing test

Per repo conventions, [Theory] with [InlineData] is preferred over separate [Fact] methods for similar tests. The new TestArgumentsNullProperty could potentially be combined with the existing TestArgumentsProperty test. However, the existing test file uses [Fact] for similar property tests (e.g., TestCreateNoWindowProperty, TestUserNameProperty), so the current style is consistent with the file's pattern. Non-blocking.

---

Models contributing to this review: Claude Opus 4.6 (primary), GPT-5.4. Gemini was attempted but returned an error.

Generated by Code Review for issue #126076 · ◷

[krwq]

optional feedback

[adamsitnik]

/ba-g the timeouts are very unlikely to be related to this change..