ReadAsync length

[alexis-]

Hello,

First of all, this is a really nice project. I'm enjoying reading your code a lot, and this will definitely save me time so thanks. :)

While familiarizing myself with H.Pipes, I came across this bit in PipeStreamReader.cs

    private async Task<byte[]> ReadAsync(int length, CancellationToken cancellationToken = default)
    {
        var buffer = new byte[length];
#if NETSTANDARD2_1 || NETCOREAPP3_1_OR_GREATER
        await BaseStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);
#elif NET461_OR_GREATER || NETSTANDARD2_0
        await BaseStream.ReadAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
#else
#error Target Framework is not supported
#endif

        return buffer;
    }

Shouldn't the return value (= number of bytes read) of ReadAsync be assigned and compared with the intended message length?

var readLength = await BaseStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);

if (readLength != length)
  throw new IOException($"Expected {length} bytes but read {readLength}");

[HavenDV]

Thanks for the code review. Yes, it really is worth it. In fact, I'm surprised I don't have warnings about not using explicitly ignoring return values via the discard pattern _ = Method(); I'm trying to figure out how to enable this permanently for all my projects. It seems this line in .editorconfig does not work in VS 2022 latest preview:

csharp_style_unused_value_assignment_preference = discard_variable:warning

[HavenDV]

Had to tinker to find why this diagnostic didn't work: https://github.com/dotnet/roslyn/issues/60875

[HavenDV]

Fixed https://github.com/HavenDV/H.Pipes/commit/030e9ba6d3b1c70fb624dd90ca7b8ee2f1a410f0

[alexis-]

Already fixed! Nice 👍

It's surprising that the unused_value_assignment bug made is this far into the release cycle of VS2022.