Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullability of StreamReader and StreamWriter encoding parameter is not consistent #2376

Open
jnm2 opened this issue Jan 30, 2020 · 5 comments
Labels
area-System.IO enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@jnm2
Copy link
Contributor

jnm2 commented Jan 30, 2020

StreamReader(Stream, Encoding) does not throw when null is passed

StreamReader(Stream, Encoding, bool) does not throw when null is passed

StreamReader(Stream, Encoding, bool, int) does not throw when null is passed

StreamReader(Stream, Encoding?, bool, int, bool) does not throw when null is passed


StreamWriter(Stream, Encoding) does not throw when null is passed

StreamWriter(Stream, Encoding, int) does not throw when null is passed

StreamWriter(Stream, Encoding?, int, bool) does not throw when null is passed

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 30, 2020
@jnm2 jnm2 changed the title Nullability of StreamReader encoding parameter is not consistent Nullability of StreamReader and StreamWriter encoding parameter is not consistent Jan 30, 2020
@stephentoub
Copy link
Member

The overloads where you have an X next to them historically have thrown when Encoding is null, and still do on .NET Framework. It was only recently changed for .NET Core, in dotnet/coreclr#24056, specifically because of a desire to have optional arguments.

We should probably change the other Encoding arguments to be nullable as well.

@stephentoub
Copy link
Member

That said, it's strange that we allow null for Encoding with the Stream-based constructors but not with the string-based constructors.

@JeremyKuhne, what's the rhyme/reason for that?

@jnm2
Copy link
Contributor Author

jnm2 commented Jan 30, 2020

The overloads where you have an X next to them historically have thrown when Encoding is null, and still do on .NET Framework.

The overload without an X throws on .NET Framework also.

@stephentoub
Copy link
Member

stephentoub commented Jan 30, 2020

The overload without an X throws on .NET Framework also.

I understand that; I was commenting on the ones that are currently specified to take Encoding rather than Encoding?.

@JeremyKuhne
Copy link
Member

it's strange that we allow null for Encoding with the Stream-based constructors but not with the string-based constructors.

what's the rhyme/reason for that?

Oversight, really. The original issue was targeting leaveOpen which has no meaning for the path constructors. I agree that we should be consistent with the path constructors and Encoding. @carlossanlop, any thoughts?

@JeremyKuhne JeremyKuhne added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Mar 5, 2020
@JeremyKuhne JeremyKuhne added this to the 5.0 milestone Mar 5, 2020
@carlossanlop carlossanlop modified the milestones: 5.0.0, Future Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.IO enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants