Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@bamarsha
Copy link
Contributor

@bamarsha bamarsha commented Oct 6, 2020

These are a few minor nullability issues that don't seem to be caught by the C# compiler or Visual Studio, but Rider marks them as errors. They seem to be legitimate issues, so here are some fixes:

  • ExternalRewriteSteps.cs: T could be a value type, in which case unboxing the potentially null result from ?.Invoke could throw an exception. Using default handles both value and reference types without errors.
  • ProjectLoader.cs: p.Name is known to be not null here, but p.Name?.ToLowerInvariant() implies that it could be null, and should convert the type from string to string?. But the value is used as a dictionary key which must be non-null.
  • LanguageServer/Utils.cs: TResult may be null, but it is not known to be either a value type or a reference type, so it's not possible to use TResult?. A [MaybeNullWhen(false)] out parameter can't be easily used either, because local functions don't support attributes until C# 9. The simplest thing here seems to be to use the null-forgiving operator, because the return value is used safely in the code below.

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@bamarsha bamarsha closed this Oct 7, 2020
@bamarsha bamarsha reopened this Oct 7, 2020
@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@swernli swernli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I hadn't actually known about the default literal before, very cool to see it used here!

@bamarsha bamarsha merged commit b19fe17 into main Oct 7, 2020
@bamarsha bamarsha deleted the samarsha/nullable-fixes branch October 7, 2020 20:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants