Skip to content

Commit

Permalink
Minor refactorings/renamings (#15037)
Browse files Browse the repository at this point in the history
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/15037)

Co-authored-by: Stephen Weatherford <Stephen.Weatherford.com>
  • Loading branch information
StephenWeatherford authored Sep 10, 2024
1 parent 0afeb3b commit b7faee2
Show file tree
Hide file tree
Showing 19 changed files with 214 additions and 58 deletions.
4 changes: 2 additions & 2 deletions src/Bicep.Core.IntegrationTests/Emit/TemplateEmitterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ public async Task SourceMap_maps_json_to_bicep_lines(DataSet dataSet)
sourceTextWithSourceMap.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.SourceMap!,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSourceMap),
actualLocation: sourceTextWithSourceMapFileName);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSourceMap),
actualPath: sourceTextWithSourceMapFileName);
}

[TestMethod]
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core.IntegrationTests/LexerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ string getLoggingString(Token token)
sourceTextWithDiags.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Tokens,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainTokens),
actualLocation: resultsFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainTokens),
actualPath: resultsFile);

lexer.GetTokens().Count(token => token.Type == TokenType.EndOfFile).Should().Be(1, "because there should only be 1 EOF token");
lexer.GetTokens().Last().Type.Should().Be(TokenType.EndOfFile, "because the last token should always be EOF.");
Expand Down
5 changes: 2 additions & 3 deletions src/Bicep.Core.IntegrationTests/ParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ public void Parser_should_produce_expected_syntax(DataSet dataSet)
sourceTextWithDiags.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Syntax,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSyntax),
actualLocation: resultsFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSyntax),
actualPath: resultsFile);
}

[DataTestMethod]
Expand Down Expand Up @@ -172,4 +172,3 @@ public override void VisitToken(Token token)
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ public void Print_VariousWidths_OptimizesLayoutAccordingly(int width)
output.Should().EqualWithLineByLineDiffOutput(
TestContext,
expected,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, outputFileName),
actualLocation: outputFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, outputFileName),
actualPath: outputFile);

AssertConsistentOutput(output, options);
}
Expand All @@ -52,8 +52,8 @@ public void Print_DataSet_ProducesExpectedOutput(DataSet dataSet)
output.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Formatted,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, outputFileName),
actualLocation: outputFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, outputFileName),
actualPath: outputFile);

AssertConsistentOutput(output, PrettyPrinterV2Options.Default);
}
Expand Down
12 changes: 6 additions & 6 deletions src/Bicep.Core.IntegrationTests/Semantics/SemanticModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public async Task ProgramsShouldProduceExpectedDiagnostics(DataSet dataSet)
sourceTextWithDiags.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Diagnostics,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainDiagnostics),
actualLocation: resultsFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainDiagnostics),
actualPath: resultsFile);
}

[TestMethod]
Expand Down Expand Up @@ -95,8 +95,8 @@ string getLoggingString(DeclaredSymbol symbol)
sourceTextWithDiags.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Symbols,
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSymbols),
actualLocation: resultsFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainSymbols),
actualPath: resultsFile);
}

[DataTestMethod]
Expand Down Expand Up @@ -346,8 +346,8 @@ public async Task ProgramsShouldProduceExpectedIrTree(DataSet dataSet)
sourceTextWithDiags.Should().EqualWithLineByLineDiffOutput(
TestContext,
dataSet.Ir ?? "",
expectedLocation: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainIr),
actualLocation: resultsFile);
expectedPath: DataSet.GetBaselineUpdatePath(dataSet, DataSet.TestFileMainIr),
actualPath: resultsFile);
}

private static List<SyntaxBase> GetAllBoundSymbolReferences(ProgramSyntax program)
Expand Down
10 changes: 5 additions & 5 deletions src/Bicep.Core.UnitTests/Assertions/BaselineHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ public static class BaselineHelper
public static bool ShouldSetBaseline(TestContext testContext) =>
testContext.Properties.Contains(SetBaseLineSettingName) && string.Equals(testContext.Properties[SetBaseLineSettingName] as string, bool.TrueString, StringComparison.OrdinalIgnoreCase);

public static void SetBaseline(string actualLocation, string expectedLocation)
public static void SetBaseline(string actualPath, string expectedPath)
{
actualLocation = GetAbsolutePathRelativeToRepoRoot(actualLocation);
expectedLocation = GetAbsolutePathRelativeToRepoRoot(expectedLocation);
actualPath = GetAbsolutePathRelativeToRepoRoot(actualPath);
expectedPath = GetAbsolutePathRelativeToRepoRoot(expectedPath);

if (Path.GetDirectoryName(expectedLocation) is { } parentDir &&
if (Path.GetDirectoryName(expectedPath) is { } parentDir &&
!Directory.Exists(parentDir))
{
Directory.CreateDirectory(parentDir);
}

File.Copy(actualLocation, expectedLocation, overwrite: true);
File.Copy(actualPath, expectedPath, overwrite: true);
}

public static string GetAbsolutePathRelativeToRepoRoot(string path)
Expand Down
7 changes: 6 additions & 1 deletion src/Bicep.Core.UnitTests/Assertions/BicepFileAssertions.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using Bicep.Core.Diagnostics;
using Bicep.Core.Parsing;
using Bicep.Core.Workspaces;
using FluentAssertions;
using FluentAssertions.Execution;
using FluentAssertions.Primitives;

namespace Bicep.Core.UnitTests.Assertions
Expand All @@ -26,7 +28,10 @@ public BicepFileAssertions(BicepFile bicepFile)

public AndConstraint<BicepFileAssertions> HaveSourceText(string expected, string because = "", params object[] becauseArgs)
{
Subject.ProgramSyntax.ToString().Should().EqualIgnoringNewlines(expected, because, becauseArgs);
var actual = Subject.ProgramSyntax.ToString().NormalizeNewlines();
expected = expected.NormalizeNewlines();

actual.Should().EqualWithLineByLineDiff(expected, because, becauseArgs);

return new AndConstraint<BicepFileAssertions>(this);
}
Expand Down
88 changes: 84 additions & 4 deletions src/Bicep.Core.UnitTests/Assertions/StringAssertionsExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Runtime.InteropServices;
using System.Text;
using System.Text.RegularExpressions;
using Bicep.Core.Parsing;
using Bicep.Core.PrettyPrintV2;
using Bicep.Core.Semantics;
using Bicep.Core.UnitTests.Utils;
using DiffPlex.DiffBuilder;
using DiffPlex.DiffBuilder.Model;
Expand Down Expand Up @@ -55,7 +59,39 @@ private static string GetDiffMarker(ChangeType type)
return string.Join('\n', lineLogs);
}

public static AndConstraint<StringAssertions> EqualWithLineByLineDiffOutput(this StringAssertions instance, TestContext testContext, string expected, string expectedLocation, string actualLocation, string because = "", params object[] becauseArgs)
public static AndConstraint<StringAssertions> EqualWithLineByLineDiff(this StringAssertions instance, string expected, string because = "", params object[] becauseArgs)
{
var lineDiff = CalculateDiff(expected, instance.Subject);
var hasNewlineDiffsOnly = lineDiff is null && !expected.Equals(instance.Subject, System.StringComparison.Ordinal);
var testPassed = lineDiff is null && !hasNewlineDiffsOnly;

var output = new StringBuilder();
var isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);

Execute.Assertion
.BecauseOf(because, becauseArgs)
.ForCondition(testPassed)
.FailWith(
"""
Expected strings to be equal{reason}, but they are not.
===== DIFF (--actual, ++expected) =====
{4}
===== ACTUAL (length {0}) =====
{1}
===== EXPECTED (length {2}) =====
{3}
===== END =====
""",
instance.Subject.Length,
instance.Subject,
expected.Length,
expected,
lineDiff ?? "differences in newlines only");

return new AndConstraint<StringAssertions>(instance);
}

public static AndConstraint<StringAssertions> EqualWithLineByLineDiffOutput(this StringAssertions instance, TestContext testContext, string expected, string expectedPath, string actualPath, string because = "", params object[] becauseArgs)
{
var lineDiff = CalculateDiff(expected, instance.Subject);
var hasNewlineDiffsOnly = lineDiff is null && !expected.Equals(instance.Subject, System.StringComparison.Ordinal);
Expand All @@ -64,7 +100,7 @@ public static AndConstraint<StringAssertions> EqualWithLineByLineDiffOutput(this
var isBaselineUpdate = !testPassed && BaselineHelper.ShouldSetBaseline(testContext);
if (isBaselineUpdate)
{
BaselineHelper.SetBaseline(actualLocation, expectedLocation);
BaselineHelper.SetBaseline(actualPath, expectedPath);
}

Execute.Assertion
Expand All @@ -73,8 +109,8 @@ public static AndConstraint<StringAssertions> EqualWithLineByLineDiffOutput(this
.FailWith(
BaselineHelper.GetAssertionFormatString(isBaselineUpdate),
lineDiff ?? "differences in newlines only",
BaselineHelper.GetAbsolutePathRelativeToRepoRoot(actualLocation),
BaselineHelper.GetAbsolutePathRelativeToRepoRoot(expectedLocation));
BaselineHelper.GetAbsolutePathRelativeToRepoRoot(actualPath),
BaselineHelper.GetAbsolutePathRelativeToRepoRoot(expectedPath));

return new AndConstraint<StringAssertions>(instance);
}
Expand Down Expand Up @@ -133,6 +169,50 @@ public static AndConstraint<StringAssertions> EqualIgnoringMinimumIndent(this St
return new AndConstraint<StringAssertions>(instance);
}


/// <summary>
/// Compares two strings after normalizing by removing all whitespace
/// </summary>
public static AndConstraint<StringAssertions> EqualIgnoringWhitespace(this StringAssertions instance, string? expected, string because = "", params object[] becauseArgs)
{
var actualStringWithoutWhitespace = instance.Subject is null ? null : new Regex("\\s*").Replace(instance.Subject, "");
var expectedStringWithoutWhitespace = expected is null ? null : new Regex("\\s*").Replace(expected, "");

using (new AssertionScope()) {
Execute.Assertion
.BecauseOf(because, becauseArgs)
.ForCondition(string.Equals(expectedStringWithoutWhitespace, actualStringWithoutWhitespace, StringComparison.Ordinal))
.FailWith("Expected {context:string} to be {0}{reason} when ignoring whitespace, but found {1}. See next message for details.", expected, instance.Subject);

actualStringWithoutWhitespace.Should().Be(expectedStringWithoutWhitespace);
}

return new AndConstraint<StringAssertions>(instance);
}

/// <summary>
/// Compares two strings after normalizing by pretty-printing both strings as Bicep
/// </summary>
public static AndConstraint<StringAssertions> EqualIgnoringBicepFormatting(this StringAssertions instance, string? expected, string because = "", params object[] becauseArgs)
{
var actualStringFormatted = instance.Subject is null ? null : PrettyPrintAsBicep(instance.Subject);
var expectedStringFormatted = expected is null ? null : PrettyPrintAsBicep(expected);

using (new AssertionScope())
{
Execute.Assertion
.BecauseOf(because, becauseArgs)
.ForCondition(string.Equals(expectedStringFormatted, actualStringFormatted, StringComparison.Ordinal))
.FailWith("Expected {context:string} to be {0}{reason} when ignoring Bicep formatting, but found {1}. See next message for details.", expected, instance.Subject);

actualStringFormatted.Should().Be(expectedStringFormatted);
}

return new AndConstraint<StringAssertions>(instance);

static string PrettyPrintAsBicep(string s) => PrettyPrinterV2.PrintValid(CompilationHelper.Compile(s).SourceFile.ProgramSyntax, PrettyPrinterV2Options.Default);
}

/// <summary>
/// Compares two strings after normalizing by removing whitespace from the beginning and ending of all lines
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,56 @@ public void NotContainAny_WithStringComparison(string text, IEnumerable<string>

actualMessage.Should().Be(expectedFailureMessage);
}

[TestMethod]
public void EqualWithLineByLineDiff()
{
string s1 = """
abc
def
ghi
jkl
mno
""";
string s2 = """
abc
def
jkl
mnop
""";
string expectedMessage = """
Expected strings to be equal because I said so, but they are not.
===== DIFF (--actual, ++expected) =====
"[3] ++ ghi
[] -- mnop
[5] ++ mno"
===== ACTUAL (length 19) =====
"abc
def
ghi
jkl
mno"
===== EXPECTED (length 16) =====
"abc
def
jkl
mnop"
===== END =====
""";

string? actualMessage;
try
{
s1.Should().EqualWithLineByLineDiff(s2, "because I said so");
actualMessage = null;
}
catch (Exception ex)
{
actualMessage = ex.Message;
}

actualMessage.Should().Be(expectedMessage);

}
}
}
4 changes: 2 additions & 2 deletions src/Bicep.Core.UnitTests/Baselines/BaselineFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public void ShouldHaveExpectedValue()
this.ReadFromOutputFolder().Should().EqualWithLineByLineDiffOutput(
TestContext,
EmbeddedFile.Contents,
expectedLocation: EmbeddedFile.RelativeSourcePath,
actualLocation: OutputFilePath);
expectedPath: EmbeddedFile.RelativeSourcePath,
actualPath: OutputFilePath);
}

public void ShouldHaveExpectedJsonValue()
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core.UnitTests/Baselines/BaselineFolder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ public BaselineFile GetFileOrEnsureCheckedIn(string relativePath)
"".Should().EqualWithLineByLineDiffOutput(
this.EntryFile.TestContext,
"<missing>",
expectedLocation: embeddedFile.RelativeSourcePath,
actualLocation: outputFile);
expectedPath: embeddedFile.RelativeSourcePath,
actualPath: outputFile);
throw new NotImplementedException("Code cannot reach this point as the previous line will always throw");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,9 @@ private void AddCodeFix(TextSpan span, string replacement, string description)

private static bool TryGetValidIdentifierToken(SyntaxBase syntax, [NotNullWhen(true)] out string? validToken)
{
if (syntax is StringSyntax stringSyntax &&
stringSyntax.TryGetLiteralValue() is { } literalValue)
if (syntax is StringSyntax stringSyntax && stringSyntax.TryGetLiteralValue() is { } literalValue)
{
if (Lexer.IsValidIdentifier(literalValue) &&
// exclude non-contextual keywords like 'nul and 'true' - see https://github.com/Azure/bicep/issues/13347.
!LanguageConstants.NonContextualKeywords.ContainsKey(literalValue))
if (!StringUtils.IsPropertyNameEscapingRequired(literalValue))
{
validToken = literalValue;
return true;
Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Core/Extensions/EnumerableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source)
}
}

public static IEnumerable<T> TakeWhileNotNull<T>(this IEnumerable<T?> source)
where T : class
{
return source.TakeWhile(x => x is not null)
.Cast<T>();
}

public static T[] ToArrayExcludingNull<T>(this IEnumerable<T?> source)
where T : class
=> source.WhereNotNull().ToArray();
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Parsing/BaseParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,7 @@ protected IEnumerable<SyntaxBase> DecorableSyntaxLeadingNodes()
{
yield return this.Decorator();

// All decorators must followed by a newline.
// All decorators must be followed by a newline.
yield return this.WithRecovery(this.NewLine, RecoveryFlags.ConsumeTerminator, TokenType.NewLine);


Expand Down
Loading

0 comments on commit b7faee2

Please sign in to comment.