Enable nullable references in Moq.csproj#1535
Enable nullable references in Moq.csproj#1535andrewimcclement wants to merge 93 commits intodevlooped:mainfrom
Conversation
|
|
|
To fix the dotnet-format build error, plz run: |
|
Run the dotnet format command. |
…r ease of use of nullability checks.
…e non-nullability.
…hing ever calls `new Condition(null)` so we can simply rely on nullable annotations, so long as we guard the public interface.
… ProxyFactory.cs.
…still need dealing with.
…ke nullability is a little dodgy here.
There was a problem hiding this comment.
Pull Request Overview
Enables nullable reference types in Moq.csproj and applies nullable annotations across the codebase
- Turn on
<Nullable>enable</Nullable>and defineNULLABLE_REFERENCE_TYPESin the project file - Annotate thousands of reference-type members with
?, add null-forgiving operators (!), and nullability attributes - Suppress nullable warnings (
default!) and leave TODOs where nullable behavior needs further attention
Reviewed Changes
Copilot reviewed 78 out of 78 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Moq/Mock`1.cs | Contains a leftover TODO in the parameterless constructor |
| src/Moq/StringBuilderExtensions.cs | Manually disposes enumerator instead of using a using or foreach |
| // The skipInitialize parameter is not used at all, and it's | ||
| // just to differentiate this ctor that should do nothing | ||
| // from the regular ones which initializes the proxy, etc. | ||
| // TODO: How should nullable references be handled here? |
There was a problem hiding this comment.
Resolve or remove this TODO. Clarify how nullable references should be handled in this constructor (e.g., initialize instance or document intent) to avoid leaving incomplete code.
src/Moq/StringBuilderExtensions.cs
Outdated
| stringBuilder.AppendValueOf(enumerator.Current); | ||
| } | ||
|
|
||
| (enumerator as IDisposable)?.Dispose(); |
There was a problem hiding this comment.
[nitpick] Consider using a foreach loop or wrapping the enumerator in a using statement to ensure deterministic disposal even if an exception occurs during iteration.
There was a problem hiding this comment.
Tidied, testing for IReadOnlyList<object> now instead.
…ng for `IReadOnlyList<object>`. This does handle more types than before (previously only T[] and List<T> were handled) but I do not think the extension is problematic. This also avoids the theoretical issue of the enumerator not being disposed if an exception is thrown during iteration, since we are using the indexer directly.
3 participants
Aims to resolve #1418 , #1416 .
Does not extend to Moq.Tests.csproj, etc.
Nullable reference issues are enabled as warnings in Moq.csproj. Several issues have been left unresolved, as I lack the knowledge to resolve them correctly at this time (perhaps maintainers can help here, or else it may be acceptable to put this PR down as is and slowly pick off the remaining nullable reference warnings one at a time).
In general I have tried hard to avoid actually changing the code functionality, except where it seemed clear. So several simplifications could be made in follow up PRs.
See the docs for details of the nullability attributes I have used.