Enable nullable for NuGet.Protocol plugin messages, logging, and contracts#7434
Draft
nkolev92 wants to merge 3 commits into
Draft
Enable nullable for NuGet.Protocol plugin messages, logging, and contracts#7434nkolev92 wants to merge 3 commits into
nkolev92 wants to merge 3 commits into
Conversation
nkolev92
changed the title
nkolev92
force-pushed
the
dev-nkolev92-nullableProtocolPluginMessages
branch
from
38170f0 to
b7f05bb
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes `#nullable disable` from the plugin constants, exceptions,
event args, value objects, and interfaces in `NuGet.Protocol`.
Key behavioral annotations:
- `LineReadEventArgs.Line` becomes nullable; `StandardOutputReceiver`
already null-checks it.
- `ProtocolErrorEventArgs.Message` becomes nullable because the 1-arg
ctor never sets it.
- `PluginDiscoveryResult.Message` becomes nullable; returns null when
the plugin file is `Valid`.
- `PluginCreationResult.{Plugin, PluginMulticlientUtilities, Claims,
Message, Exception}` become nullable because the three ctors set
mutually exclusive subsets. An `internal bool IsSuccess` helper is
added with `[MemberNotNullWhen]` attributes so consumers can have
the compiler enforce the invariant.
- `IConnection.ProtocolVersion` becomes nullable per the doc comment.
- `IMessageDispatcher.SetConnection(IConnection?)` accepts null per
the doc comment.
- `IRequestHandlers.TryGet` out parameter uses
`[NotNullWhen(true)] out IRequestHandler?`.
- `PluginException`/`ProtocolException` ctors accept nullable
`message` and `innerException` to match `System.Exception` base.
Progress: NuGet/Home#14851
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
nkolev92
force-pushed
the
dev-nkolev92-nullableProtocolPluginMessages
branch
from
d9001ac to
f3fa223
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Progress: NuGet/Home#14851
Description
Completes the nullable enablement of the
NuGet.Protocolplugin layer: every type underNuGet.Protocol.Plugins/Messages/andNuGet.Protocol.Plugins/Logging/, plusMessageUtilitiesand the surrounding plugin contracts (interfaces, exceptions, event args, constants, and small value objects), is now annotated. The plugin runtime (Connection,Sender,Receiver,MessageDispatcher, request handlers, lifecycle, discovery, package operations) is intentionally left for a follow-up PR.Non-mechanical changes worth calling out:
PluginLogger.CreateStreamWriternow throws an explicitInvalidOperationExceptionif_logDirectoryPathis missing or ifProcess.MainModule?.FileNameis null, instead of relying on an implicitNullReferenceException.Message(string, MessageType, MessageMethod, JObject)constructor previously had an empty body, leavingRequestId,Type,Method, andPayloadObjectuninitialized. It now delegates to the internal constructor so the documented validation actually runs.PluginCreationResulthad five reference-type properties that are mutually exclusive depending on whether the creation succeeded. Added aninternal bool IsSuccessdiscriminator with[MemberNotNullWhen]attributes so the compiler enforces the documented contract: whenMessageis non-nullPlugin/PluginMulticlientUtilities/Claimsare null, and vice versa.LineReadEventArgs.LineandProtocolErrorEventArgs.Messageare now nullable to match the call sites that already produced null values (StandardOutputReceiver null-guards lines; the single-argumentProtocolErrorEventArgsconstructor never setsMessage).IConnection.ProtocolVersionandIMessageDispatcher.SetConnection's parameter are now nullable, matching what the XML docs already said.IRequestHandlers.TryGetis annotated with[NotNullWhen(true)]on theoutparameter so callers don't need a redundant null check after a successful lookup.message/innerExceptionparameters to match theSystem.Exceptionbase contract.PR Checklist