Enable nullability, round 3 of x #525
Conversation
|
Ready for review. |
| this.writer = writer; | ||
| this.writer.NewLine = "\n"; |
There was a problem hiding this comment.
| this.writer = writer; | |
| this.writer.NewLine = "\n"; | |
| Writer = writer; |
We don't gain anything by duplicating this
There was a problem hiding this comment.
Sadly, the analyzer doesn't follow the setter through and mark the writer field as non-null if we do this. We could set a MemberNotNull attribute and throw when setting the writer, but we also double up some logic if we do this.
There was a problem hiding this comment.
Unfortunate :/
This would be a fine place for MemberNotNull, but since we'd have to add a dummy class just for this, it's fine as is.
src/Markdig/Syntax/ContainerBlock.cs
Outdated
| } | ||
|
|
||
| public void Sort(Comparison<Block> comparison) | ||
| { | ||
| if (comparison == null) ThrowHelper.ArgumentNullException(nameof(comparison)); | ||
| Array.Sort(children, 0, Count, new BlockComparer(comparison)); | ||
| Array.Sort(children, 0, Count, new BlockComparer(comparison)!); |
There was a problem hiding this comment.
This should be needed right after the new
There was a problem hiding this comment.
I backed up out making the comparison delegate nullable, so these suppressions are no longer needed.
| public int Compare(Block? x, Block? y) | ||
| { | ||
| return comparison(x, y); | ||
| return comparison(x!, y!); | ||
| } |
There was a problem hiding this comment.
This should either accept non-nullable blocks, or the comparer should allow nullable
There was a problem hiding this comment.
This is an annoying case where the nullability contract of IComparer (below) forces us to define these as nullable.
int Compare([AllowNull] T x, [AllowNull] T y);
The System.Comparison delegate, sadly, does NOT define the system contract with nullable attributes. Our options are limited here since we don't control either implementation / contract.
There was a problem hiding this comment.
UPDATE: we can make the comparison explicitly accept nullable. But this forces us to suppress its use in Array.Sort -- and we don't actually have a case to compare against null. IComparer, regardless, requires us to accept null -- so I believe this is a valid case for suppression. We could also make this class oblivious.
|
Thanks! |
Another big chunk of nullability annotations