Improve nullable annotations for ILogger.BeginScope

[maxkoshevoi]

Based on this discussion: #66892 (comment)

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

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

Issue Details

---

Based on this discussion: #66892 (comment)

Author:	maxkoshevoi
Assignees:	-
Labels:	new-api-needs-documentation, area-Extensions-Logging, community-contribution
Milestone:	-

[eerhardt]

cc @davidfowl @lodejard @rynowak @BrennanConroy @halter73 - FYI, in case you have any feedback on this annotation. Specifically, the IDisposable? return type is what I am concerned about. But we already have 2 places that can return null from this interface method, so it does match reality.

runtime/src/libraries/Microsoft.Extensions.Logging.EventLog/src/EventLogLogger.cs

Lines 51 to 54 in eb51b02

	public IDisposable BeginScope<TState>(TState state)
	{
	return _externalScopeProvider?.Push(state);
	}

https://github.com/dotnet/aspnetcore/blob/17695facaeaf9d6d83aea9bfe87747180922117a/src/Logging.AzureAppServices/src/BatchingLogger.cs#L21-L24

[eerhardt]

LGTM - assuming no one else has any feedback.

[BrennanConroy]

Scopes aren't supported by all loggers so the IDisposable? makes sense to me.

[BrennanConroy]

Just curious, how does this not complain about hiding the nullable annotation on ILogger.BeginScope?

[eerhardt]

Your interface implementation can be more specific than the interface. Here the interface says "You can return an IDisposable, or null". And this implementation says "I only ever return an IDisposable", which is allowed.

The inverse isn't allowed though.

[eerhardt]

I'm going to merge this. If anyone else has feedback, we can open a new PR to address it.