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

Only read files that are UTF8 or UTF8 BOM #787

Merged
merged 10 commits into from
Jan 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
<PackageVersion Include="System.Text.Encoding.CodePages" Version="6.0.0"/>
<PackageVersion Include="System.Text.Json" Version="7.0.0" />
<PackageVersion Include="System.Threading.Tasks.Extensions" Version="4.5.4"/>
<PackageVersion Include="UTF.Unknown" Version="2.3.0"/>
<PackageVersion Include="YamlDotNet" Version="11.1.1"/>
</ItemGroup>
</Project>
4 changes: 3 additions & 1 deletion Src/CSharpier.Cli.Tests/CliTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,9 @@ public async Task Should_Write_Error_With_Multiple_Piped_Files(string input, str

result.ErrorOutput
.Should()
.Be($"Error {output} - Failed to compile so was not formatted.{Environment.NewLine}");
.Be(
$"Error {output} - Failed to compile so was not formatted.{Environment.NewLine} (1,26): error CS1513: }} expected{Environment.NewLine}"
);
result.ExitCode.Should().Be(1);
}

Expand Down
1 change: 0 additions & 1 deletion Src/CSharpier.Cli/CSharpier.Cli.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
<PackageReference Include="System.IO.Abstractions.TestingHelpers"/>
<PackageReference Include="System.IO.Hashing"/>
<PackageReference Include="System.Text.Encoding.CodePages"/>
<PackageReference Include="UTF.Unknown"/>
<PackageReference Include="YamlDotNet"/>
</ItemGroup>
<ItemGroup>
Expand Down
10 changes: 9 additions & 1 deletion Src/CSharpier.Cli/CommandLineFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

namespace CSharpier.Cli;

using System.Text;

internal static class CommandLineFormatter
{
public static async Task<int> Format(
Expand Down Expand Up @@ -366,7 +368,13 @@ CancellationToken cancellationToken

if (codeFormattingResult.Errors.Any())
{
fileIssueLogger.WriteError("Failed to compile so was not formatted.");
var errorMessage = new StringBuilder();
errorMessage.AppendLine("Failed to compile so was not formatted.");
foreach (var message in codeFormattingResult.Errors)
{
errorMessage.AppendLine(message.ToString());
}
fileIssueLogger.WriteError(errorMessage.ToString());
Interlocked.Increment(ref commandLineFormatterResult.FailedCompilation);
return;
}
Expand Down
27 changes: 12 additions & 15 deletions Src/CSharpier.Cli/FileReader.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.IO.Abstractions;
using System.Text;
using UtfUnknown;

namespace CSharpier.Cli;

Expand All @@ -25,24 +24,22 @@ CancellationToken cancellationToken
try
{
await using var fileStream = fileSystem.File.OpenRead(filePath);
var detectionResult = CharsetDetector.DetectFromStream(fileStream);
var encoding = detectionResult?.Detected?.Encoding;
if (encoding == null)
{
unableToDetectEncoding = true;
encoding = Encoding.Default;
}

fileStream.Seek(0, SeekOrigin.Begin);

// this fixes an issue with ANSI encoded files like csharpier-repos\AutoMapper\src\UnitTests\Internationalization.cs
var encodingToRead = encoding.CodePage == 852 ? Encoding.GetEncoding(1252) : encoding;

using var streamReader = new StreamReader(fileStream, encodingToRead);
// this is kind of a "hack" - read the file without the BOM then
// streamReader.CurrentEncoding below will correctly be UTF8 or UTF8-BOM
// https://stackoverflow.com/a/27976558
using var streamReader = new StreamReader(
fileStream,
new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)
);

var fileContents = await streamReader.ReadToEndAsync();

return new FileReaderResult(encoding, fileContents, unableToDetectEncoding);
return new FileReaderResult(
streamReader.CurrentEncoding,
fileContents,
unableToDetectEncoding
);
}
finally
{
Expand Down
1 change: 0 additions & 1 deletion Src/CSharpier.Tests/CSharpier.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
<PackageReference Include="Microsoft.NET.Test.Sdk"/>
<PackageReference Include="NUnit"/>
<PackageReference Include="NUnit3TestAdapter"/>
<PackageReference Include="UTF.Unknown"/>
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\CSharpier.Cli\CSharpier.Cli.csproj"/>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,9 @@ public class ClassName
{
public void MethodName() { }
}

public enum MeetingLocation
{
Café,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Uh... do you want to make the test case Café like the bug? Café kind of makes no sense.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm thinking whatever you are looking at this code with isn't interpreting the file correctly? This is what I see
image

Restaurant
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,9 @@
{
public void MethodName() { }
}

public enum MeetingLocation
{
Café,
Restaurant
}