Skip to content

Analyzer: Prefer Memory overloads for Stream async Read/Write methods#3497

Merged
carlossanlop merged 14 commits intodotnet:masterfrom
carlossanlop:PreferStreamAsyncMemoryOverloads
Apr 29, 2020
Merged

Analyzer: Prefer Memory overloads for Stream async Read/Write methods#3497
carlossanlop merged 14 commits intodotnet:masterfrom
carlossanlop:PreferStreamAsyncMemoryOverloads

Conversation

@carlossanlop
Copy link

@carlossanlop carlossanlop commented Apr 14, 2020

Contributes to dotnet/runtime#33790

Summary

If the user invokes any of these methods from an object that is or inherits from Stream:

ReadAsync(Byte[], Int32, Int32)
ReadAsync(Byte[], Int32, Int32, CancellationToken)
WriteAsync(Byte[], Int32, Int32)
WriteAsync(Byte[], Int32, Int32, CancellationToken)

We should suggest to instead use:
ReadAsync(Memory<Byte>, CancellationToken)
WriteAsync(ReadOnlyMemory<Byte>, CancellationToken)

All unit tests passed locally.

@carlossanlop carlossanlop requested a review from sharwell April 14, 2020 17:54
@codecov
Copy link

codecov bot commented Apr 15, 2020

Codecov Report

Merging #3497 into master will increase coverage by 0.00%.
The diff coverage is 96.20%.

@@            Coverage Diff            @@
##           master    #3497     +/-   ##
=========================================
  Coverage   95.11%   95.12%             
=========================================
  Files        1050     1054      +4     
  Lines      238214   239557   +1343     
  Branches    15503    15523     +20     
=========================================
+ Hits       226579   227872   +1293     
- Misses       9918     9952     +34     
- Partials     1717     1733     +16     

@carlossanlop carlossanlop requested a review from mavasani April 28, 2020 20:37
Copy link

@mavasani mavasani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@carlossanlop
Copy link
Author

@buyaa-n @stephentoub @bartonjs can I get a sign off from you please, if everything looks good?

Copy link

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comment, looks good, thanks!

Co-Authored-By: buyaa-n <buyankhishig.namnan@microsoft.com>
@carlossanlop carlossanlop merged commit b09efe5 into dotnet:master Apr 29, 2020
@carlossanlop carlossanlop deleted the PreferStreamAsyncMemoryOverloads branch April 29, 2020 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants