Skip to content

Commit

Permalink
Ensure \n is used when emitting template (#14581)
Browse files Browse the repository at this point in the history
Currently, Bicep generates inconsistent newlines depending on whether
the template is built using the Build command (and with or without the
`--stdout` flag) or through the VS Code extension.

This PR updates the emission code to consistently use `\n` for newlines.
This change should prevent issues like
#14383 in the future.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14581)
  • Loading branch information
shenglol authored Jul 17, 2024
1 parent 00a5bc4 commit ac8392e
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 7 deletions.
7 changes: 6 additions & 1 deletion src/Bicep.Cli.IntegrationTests/BuildCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System.IO.Abstractions;
using System.Text.RegularExpressions;
using Bicep.Cli.UnitTests;
using Bicep.Core;
using Bicep.Core.Configuration;
Expand Down Expand Up @@ -88,7 +89,10 @@ public async Task Build_Valid_SingleFile_WithTemplateSpecReference_ShouldSucceed
var compiledFilePath = Path.Combine(outputDirectory, DataSet.TestFileMainCompiled);
File.Exists(compiledFilePath).Should().BeTrue();

var actual = JToken.Parse(File.ReadAllText(compiledFilePath));
var compiledFileContent = File.ReadAllText(compiledFilePath);
compiledFileContent.Should().OnlyContainLFNewline();

var actual = JToken.Parse(compiledFileContent);

actual.Should().EqualWithJsonDiffOutput(
TestContext,
Expand Down Expand Up @@ -219,6 +223,7 @@ public async Task Build_Valid_SingleFile_WithTemplateSpecReference_ToStdOut_Shou
{
result.Should().Be(0);
output.Should().NotBeEmpty();
output.Should().OnlyContainLFNewline();
AssertNoErrors(error);
}

Expand Down
3 changes: 3 additions & 0 deletions src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ public async Task Build_Valid_Params_File_To_Outdir_Should_Succeed(BaselineData_
AssertNoErrors(error);
}

data.Compiled!.ReadFromOutputFolder().Should().OnlyContainLFNewline();
data.Compiled!.ShouldHaveExpectedJsonValue();
}

Expand All @@ -328,6 +329,8 @@ public async Task Build_Valid_Params_File_ToStdOut_Should_Succeed(BaselineData_B
}

var parametersStdout = output.FromJson<BuildParamsStdout>();
parametersStdout.parametersJson.Should().OnlyContainLFNewline();

data.Compiled!.WriteToOutputFolder(parametersStdout.parametersJson);
data.Compiled.ShouldHaveExpectedJsonValue();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public async Task SourceMap_maps_json_to_bicep_lines(DataSet dataSet)
var sourceMap = emitResult.SourceMap!;

using var streamReader = new StreamReader(new MemoryStream(memoryStream.ToArray()));
var jsonLines = (await streamReader.ReadToEndAsync()).Split(System.Environment.NewLine);
var jsonLines = (await streamReader.ReadToEndAsync()).Split("\n");

var sourceTextWithSourceMap = OutputHelper.AddSourceMapToSourceText(
dataSet.Bicep, DataSet.TestFileMain, dataSet.HasCrLfNewlines() ? "\r\n" : "\n", sourceMap, jsonLines);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Bicep.Core.UnitTests.Assertions
{
public static class StringAssertionsExtensions
public static partial class StringAssertionsExtensions
{
private static string EscapeWhitespace(string input)
=> input
Expand Down Expand Up @@ -200,9 +200,27 @@ public static AndConstraint<StringAssertions> BeInKebabCasing(this StringAsserti
return new AndConstraint<StringAssertions>(instance);
}

public static AndConstraint<StringAssertions> OnlyContainLFNewline(this StringAssertions instance, string because = "", params object[] becauseArgs) =>
instance.HaveConsistentNewlines("\n", because, becauseArgs);

public static AndConstraint<StringAssertions> HaveConsistentNewlines(this StringAssertions instance, string newline, string because = "", params object[] becauseArgs)
{
var newlines = NewlineSequence().Matches(instance.Subject).Select(x => x.Value).Distinct().ToArray();

Execute.Assertion
.BecauseOf(because, becauseArgs)
.ForCondition(newlines.Length == 1 && newlines[0] == newline)
.FailWith("Expected all newlines in {0} to be {1}, but found inconsistent newlines.", instance.Subject, newline);

return new(instance);
}

private static bool Contains(string actual, string expected, StringComparison comparison)
{
return (actual ?? string.Empty).Contains(expected ?? string.Empty, comparison);
}

[GeneratedRegex("\r\n|\r|\n")]
private static partial Regex NewlineSequence();
}
}
1 change: 1 addition & 0 deletions src/Bicep.Core/Emit/PositionTrackingJsonTextWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ private class PositionTrackingTextWriter : TextWriter
public PositionTrackingTextWriter(TextWriter textWriter)
{
this.internalWriter = textWriter;
this.NewLine = textWriter.NewLine;
}

public override Encoding Encoding => this.internalWriter.Encoding;
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/SourceAwareJsonTextWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class SourceAwareJsonTextWriter : JsonTextWriter
public SourceAwareJsonTextWriter(TextWriter textWriter, BicepSourceFile? sourceFileToTrack = default) : base(textWriter)
{
this.sourceFile = sourceFileToTrack;
this.TrackingJsonWriter = new PositionTrackingJsonTextWriter(new StringWriter(), this.sourceFile);
this.TrackingJsonWriter = new PositionTrackingJsonTextWriter(new StringWriter() { NewLine = "\n" }, this.sourceFile);
}

public void ProcessSourceMap(JToken templateWithHash)
Expand Down
5 changes: 4 additions & 1 deletion src/Bicep.Core/Emit/TemplateEmitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public EmitResult EmitTemplateGeneratedParameterFile(TextWriter textWriter, stri
/// <param name="stream">The stream to write the template</param>
public EmitResult Emit(Stream stream)
{
using var sw = new StreamWriter(stream, UTF8EncodingWithoutBom, 4096, leaveOpen: true);
using var sw = new StreamWriter(stream, UTF8EncodingWithoutBom, 4096, leaveOpen: true)
{
NewLine = "\n",
};

return Emit(sw);
}
Expand Down
2 changes: 2 additions & 0 deletions src/Bicep.LangServer.IntegrationTests/BuildCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ await client.Workspace.ExecuteCommand(new Command
});

var buildCommandOutput = File.ReadAllText(Path.ChangeExtension(bicepFilePath, ".json"));
buildCommandOutput.Should().OnlyContainLFNewline();
buildCommandOutput.Should().BeEquivalentToIgnoringNewlines(expectedJson);
}

Expand Down Expand Up @@ -87,6 +88,7 @@ await client.Workspace.ExecuteCommand(new Command
});

var buildCommandOutput = File.ReadAllText(Path.ChangeExtension(bicepFilePath, ".json"));
buildCommandOutput.Should().OnlyContainLFNewline();
buildCommandOutput.Should().BeEquivalentToIgnoringNewlines(expectedJson);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.LangServer/Handlers/BicepBuildCommandHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ public override async Task<string> Handle(string bicepFilePath, CancellationToke
}

var documentUri = DocumentUri.FromFileSystemPath(bicepFilePath);
return await GenerateCompiledFileAndReturnBuildOutputMessage(bicepFilePath, documentUri);
return await GenerateCompiledFileAndReturnBuildOutputMessageAsync(bicepFilePath, documentUri);
}

private async Task<string> GenerateCompiledFileAndReturnBuildOutputMessage(string bicepFilePath, DocumentUri documentUri)
private async Task<string> GenerateCompiledFileAndReturnBuildOutputMessageAsync(string bicepFilePath, DocumentUri documentUri)
{
string compiledFilePath = PathHelper.GetDefaultBuildOutputPath(bicepFilePath);

Expand Down

0 comments on commit ac8392e

Please sign in to comment.