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.

Sign up for GitHub

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

Mark or set encoding param in StreamReader ctor to be nullable #108542

Merged
merged 7 commits into from
Jan 10, 2025
+29 −24
Merged

Mark or set encoding param in StreamReader ctor to be nullable #108542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b0ed7bd
Set encoding param in StreamReader ctor to be nullable
eriawan Oct 4, 2024
b3761ed
Set additional Encoding param to be nullable and refactor ValidateArg…
eriawan Oct 5, 2024
f153b00
Set additional Encoding param to be nullable and refactor ValidateArg…
eriawan Oct 5, 2024
bf87d04
Refactor StreamReader_StringCtorTests accordingly as Encoding param h…
eriawan Oct 5, 2024
ad047bb
Add additional test to check calling ctor with Encoding = null should…
eriawan Dec 19, 2024
c2da02a
Use empty string instead of path
eriawan Dec 19, 2024
c4ad463
Remove all tests related to path usage, focus on usint Stream instead…
eriawan Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/IO/StreamReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ public StreamReader(Stream stream, bool detectEncodingFromByteOrderMarks)
{
}

public StreamReader(Stream stream, Encoding encoding)
public StreamReader(Stream stream, Encoding? encoding)
: this(stream, encoding, true, DefaultBufferSize, false)
{
}

public StreamReader(Stream stream, Encoding encoding, bool detectEncodingFromByteOrderMarks)
public StreamReader(Stream stream, Encoding? encoding, bool detectEncodingFromByteOrderMarks)
: this(stream, encoding, detectEncodingFromByteOrderMarks, DefaultBufferSize, false)
{
}
Expand All @@ -131,7 +131,7 @@ public StreamReader(Stream stream, Encoding encoding, bool detectEncodingFromByt
// unicode, and big endian unicode text, but that's it. If neither
// of those three match, it will use the Encoding you provided.
//
public StreamReader(Stream stream, Encoding encoding, bool detectEncodingFromByteOrderMarks, int bufferSize)
public StreamReader(Stream stream, Encoding? encoding, bool detectEncodingFromByteOrderMarks, int bufferSize)
: this(stream, encoding, detectEncodingFromByteOrderMarks, bufferSize, false)
{
}
Expand Down Expand Up @@ -187,18 +187,18 @@ public StreamReader(string path, bool detectEncodingFromByteOrderMarks)
{
}

public StreamReader(string path, Encoding encoding)
public StreamReader(string path, Encoding? encoding)
Copy link
Member

Choose a reason for hiding this comment

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

I was suprised to see this change, as it was not approved as part of #106236 (comment).

But then I've found @jozkee comment #106658 (review) referring to #2376 (comment) that explains that we need to unify all ctors that accept Encoding. Which we already did for StreamWriter in #106658.

So everything is good 👍

: this(path, encoding, true, DefaultBufferSize)
{
}

public StreamReader(string path, Encoding encoding, bool detectEncodingFromByteOrderMarks)
public StreamReader(string path, Encoding? encoding, bool detectEncodingFromByteOrderMarks)
: this(path, encoding, detectEncodingFromByteOrderMarks, DefaultBufferSize)
{
}

public StreamReader(string path, Encoding encoding, bool detectEncodingFromByteOrderMarks, int bufferSize)
: this(ValidateArgsAndOpenPath(path, encoding, bufferSize), encoding, detectEncodingFromByteOrderMarks, bufferSize, leaveOpen: false)
public StreamReader(string path, Encoding? encoding, bool detectEncodingFromByteOrderMarks, int bufferSize)
: this(ValidateArgsAndOpenPath(path, bufferSize), encoding, detectEncodingFromByteOrderMarks, bufferSize, leaveOpen: false)
{
}

Expand All @@ -207,15 +207,14 @@ public StreamReader(string path, FileStreamOptions options)
{
}

public StreamReader(string path, Encoding encoding, bool detectEncodingFromByteOrderMarks, FileStreamOptions options)
: this(ValidateArgsAndOpenPath(path, encoding, options), encoding, detectEncodingFromByteOrderMarks, DefaultBufferSize)
public StreamReader(string path, Encoding? encoding, bool detectEncodingFromByteOrderMarks, FileStreamOptions options)
: this(ValidateArgsAndOpenPath(path, options), encoding, detectEncodingFromByteOrderMarks, DefaultBufferSize)
{
}

private static FileStream ValidateArgsAndOpenPath(string path, Encoding encoding, FileStreamOptions options)
private static FileStream ValidateArgsAndOpenPath(string path, FileStreamOptions options)
{
ArgumentException.ThrowIfNullOrEmpty(path);
ArgumentNullException.ThrowIfNull(encoding);
ArgumentNullException.ThrowIfNull(options);
if ((options.Access & FileAccess.Read) == 0)
{
Expand All @@ -225,10 +224,9 @@ private static FileStream ValidateArgsAndOpenPath(string path, Encoding encoding
return new FileStream(path, options);
}

private static FileStream ValidateArgsAndOpenPath(string path, Encoding encoding, int bufferSize)
private static FileStream ValidateArgsAndOpenPath(string path, int bufferSize)
{
ArgumentException.ThrowIfNullOrEmpty(path);
ArgumentNullException.ThrowIfNull(encoding);
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(bufferSize);

return new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, DefaultFileStreamBufferSize);
Expand Down
14 changes: 7 additions & 7 deletions src/libraries/System.Runtime/ref/System.Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10762,17 +10762,17 @@ public partial class StreamReader : System.IO.TextReader
public static readonly new System.IO.StreamReader Null;
public StreamReader(System.IO.Stream stream) { }
public StreamReader(System.IO.Stream stream, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding encoding) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding encoding, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding encoding, bool detectEncodingFromByteOrderMarks, int bufferSize) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding? encoding) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding? encoding, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding? encoding, bool detectEncodingFromByteOrderMarks, int bufferSize) { }
public StreamReader(System.IO.Stream stream, System.Text.Encoding? encoding = null, bool detectEncodingFromByteOrderMarks = true, int bufferSize = -1, bool leaveOpen = false) { }
public StreamReader(string path) { }
public StreamReader(string path, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(string path, System.IO.FileStreamOptions options) { }
public StreamReader(string path, System.Text.Encoding encoding) { }
public StreamReader(string path, System.Text.Encoding encoding, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(string path, System.Text.Encoding encoding, bool detectEncodingFromByteOrderMarks, int bufferSize) { }
public StreamReader(string path, System.Text.Encoding encoding, bool detectEncodingFromByteOrderMarks, System.IO.FileStreamOptions options) { }
public StreamReader(string path, System.Text.Encoding? encoding) { }
public StreamReader(string path, System.Text.Encoding? encoding, bool detectEncodingFromByteOrderMarks) { }
public StreamReader(string path, System.Text.Encoding? encoding, bool detectEncodingFromByteOrderMarks, int bufferSize) { }
public StreamReader(string path, System.Text.Encoding? encoding, bool detectEncodingFromByteOrderMarks, System.IO.FileStreamOptions options) { }
public virtual System.IO.Stream BaseStream { get { throw null; } }
public virtual System.Text.Encoding CurrentEncoding { get { throw null; } }
public bool EndOfStream { get { throw null; } }
Expand Down
15 changes: 11 additions & 4 deletions ...braries/System.Runtime/tests/System.IO.Tests/StreamReader/StreamReader.StringCtorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ public static void NullArgs_ThrowsArgumentNullException()
AssertExtensions.Throws<ArgumentNullException>("path", () => new StreamReader((string)null, null, true));
AssertExtensions.Throws<ArgumentNullException>("path", () => new StreamReader((string)null, null, true, null));
AssertExtensions.Throws<ArgumentNullException>("path", () => new StreamReader((string)null, null, true, -1));
AssertExtensions.Throws<ArgumentNullException>("encoding", () => new StreamReader("path", (Encoding)null));
AssertExtensions.Throws<ArgumentNullException>("encoding", () => new StreamReader("path", null, true));
AssertExtensions.Throws<ArgumentNullException>("encoding", () => new StreamReader("path", null, true, null));
AssertExtensions.Throws<ArgumentNullException>("encoding", () => new StreamReader("path", null, true, -1));
eriawan marked this conversation as resolved.
Show resolved Hide resolved
AssertExtensions.Throws<ArgumentNullException>("options", () => new StreamReader("path", (FileStreamOptions)null));
AssertExtensions.Throws<ArgumentNullException>("options", () => new StreamReader("path", Encoding.UTF8, true, null));

Expand All @@ -40,6 +36,17 @@ public static void EmptyPath_ThrowsArgumentException()
AssertExtensions.Throws<ArgumentException>("path", () => new StreamReader("", Encoding.UTF8, true, -1));
}

[Fact]
public static void NullEncodingParamInCtor_ShouldNotThrowException()
{
// Call the constructor with overloads that has Stream and null encoding parameters.
// It should not throw exception, to test passing the nullable encoding parameter..
StreamReader streamReaderTest = new StreamReader(new MemoryStream(), null);
streamReaderTest = new StreamReader(new MemoryStream(), null, false);
streamReaderTest = new StreamReader(new MemoryStream(), null, false, 100);
streamReaderTest = new StreamReader(new MemoryStream(), null, false, 100, false);
}

[Fact]
public static void NegativeBufferSize_ThrowsArgumentOutOfRangeException()
{
Expand Down
Loading