Skip to content

Nullability mismatch: NodeLaunchData.EnvironmentOverrides should be IDictionary<string, string?>? #13761

Description

@rainersigwald

Body of this issue authored by GitHub Copilot at @rainersigwald's request.

Problem

DotnetHostEnvironmentHelper.CreateDotnetRootEnvironmentOverrides() returns IDictionary<string, string?>?. The null values in the dictionary are intentional sentinels meaning "remove this variable from the child process environment" — they're consumed by DotnetHostEnvironmentHelper.ApplyEnvironmentOverrides, which calls environment.Remove(key) when the value is null. This is how the helper clears the architecture-specific overrides (e.g. DOTNET_ROOT_X64) that would otherwise take precedence over DOTNET_ROOT.

But NodeLaunchData.EnvironmentOverrides (INodeLauncher.cs:28) is declared as IDictionary<string, string>? — i.e., the dictionary itself can be null but its values cannot. The two existing call sites both have to paper over the mismatch:

Both call sites are correct in practice (because the consumer chain ultimately routes through ApplyEnvironmentOverrides, which honours the null sentinels), but the type signature lies about what the API contract actually is.

Proposed fix

Change NodeLaunchData.EnvironmentOverrides — and any consumer types it flows through (INodeLauncher.Start, NodeLauncher.Start, Process start-info wiring on the receiving side, etc.) — to IDictionary<string, string?>? so the null-as-sentinel semantics are encoded in the type system. Then drop the ! in MSBuildClient.cs and the implicit reliance on #nullable disable in NodeProviderOutOfProc.cs.

This will surface any consumer that's currently mishandling null values (e.g., enumerating with KeyValuePair<string, string> instead of KeyValuePair<string, string?> and skipping the Remove path).

Acceptance

  • NodeLaunchData.EnvironmentOverrides is typed IDictionary<string, string?>?.
  • Neither call site uses ! or #nullable disable to assign to it.
  • All consumers either set or remove the variable based on whether the value is null, matching ApplyEnvironmentOverrides.
  • No behavior change at runtime; this is a type-system tightening only.

Context

Surfaced while reviewing #13716 (the DOTNET_CLI_USE_MSBUILD_SERVER=true server-launch fix). That PR added a temporary block comment in MSBuildClient.cs describing the suppression; this issue tracks doing it properly so the comment can be removed.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions