microsoft / microsoft/CsWin32

Methods that return or accept null parameters should be nullable.

Open
#1,418 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C#
Stars
2.5k
Forks
124
Avg merge
1d 3h
Merged PRs (30d)
9

Description

Actual behavior

Using the type PCWSTR as an example, its ToString() method is:

public override string ToString() => this.Value is null ? null : new string(this.Value);

As such, I receive no compiler hints or warnings that I might be dealing with a null and this has lead to NullReferenceExceptions being thrown on me in production.

As a further example, consider WritePrivateProfileString():

internal static unsafe winmdroot.Foundation.BOOL WritePrivateProfileString(string lpAppName, string lpKeyName, string lpString, string lpFileName)
{
	fixed (char* lpFileNameLocal = lpFileName)
	{
		fixed (char* lpStringLocal = lpString)
		{
			fixed (char* lpKeyNameLocal = lpKeyName)
			{
				fixed (char* lpAppNameLocal = lpAppName)
				{
					winmdroot.Foundation.BOOL __result = PInvoke.WritePrivateProfileString(lpAppNameLocal, lpKeyNameLocal, lpStringLocal, lpFileNameLocal);
					return __result;
				}
			}
		}
	}
}

The parameters lpKeyName and lpString can be null, but they're not marked as nullable and presumably, they don't whinge when null values are provided due to the silencing of warnings. This is obtuse and uninformative to developers via Intellisense.

Expected behavior

Because the output can be null, it should be changed to:

public new string? ToString() => this.Value is null ? null : new string(this.Value);

This would properly override the ToString() method that returns a nullable result so that users can be better informed about what they're working with.

Repro steps

  1. NativeMethods.txt content:
PCWSTR
  1. NativeMethods.json content (if present):
    N/A

  2. Any of your own code that should be shared?
    N/A

Context
  • CsWin32 version: [0.3.183+73e6125f79.RR]
  • Win32Metadata version (if explicitly set by project): N/A
  • Target Framework: [net462]
  • LangVersion (if explicitly set by project): N/A

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the source-generation path that produces the PCWSTR ToString() method and the WritePrivateProfileString() signature shown in the issue. Use the NativeMethods.txt repro to inspect generated output, then verify that nullable return values and nullable parameters are represented in the generated C# without changing non-nullable APIs.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.