Skip to content

Commit

Permalink
Catch case-insensitive clashes of type property names (#11457)
Browse files Browse the repository at this point in the history
As a follow-up to #11453, this PR updates the case-insensitive name
clash detector to look at type properties, too, since these are also
case-insensitive in ARM.
###### Microsoft Reviewers:
codeflow:open?pullrequest=https://github.com/Azure/bicep/pull/11457&drop=dogfoodAlpha
  • Loading branch information
jeskew committed Aug 3, 2023
1 parent 2b6539d commit 09d0d4b
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 10 deletions.
13 changes: 13 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4955,6 +4955,19 @@ public void Test_Issue502()
("BCP353", DiagnosticLevel.Error, "The types \"foo\", \"FoO\" differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them."),
});

result = CompilationHelper.Compile(Services.WithFeatureOverrides(new(UserDefinedTypesEnabled: true)), """
param x {
foo: string
FoO: int
}
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP353", DiagnosticLevel.Error, "The type properties \"foo\", \"FoO\" differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them."),
("BCP353", DiagnosticLevel.Error, "The type properties \"foo\", \"FoO\" differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them."),
});

result = CompilationHelper.Compile(Services.WithFeatureOverrides(new(AssertsEnabled: true)), """
assert foo = true
assert FoO = true
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1981,10 +1981,10 @@ public ErrorDiagnostic RuntimeValueNotAllowedInFunctionDeclaration(string? acces
"BCP352",
$"Failed to evaluate variable \"{name}\": {message}");

public ErrorDiagnostic SymbolsMustBeCaseInsensitivelyUnique(string symbolTypePluralName, IEnumerable<string> symbolNames) => new(
public ErrorDiagnostic ItemsMustBeCaseInsensitivelyUnique(string itemTypePluralName, IEnumerable<string> itemNames) => new(
TextSpan,
"BCP353",
$"The {symbolTypePluralName} {ToQuotedString(symbolNames)} differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them.");
$"The {itemTypePluralName} {ToQuotedString(itemNames)} differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them.");
}

public static DiagnosticBuilderInternal ForPosition(TextSpan span)
Expand Down
26 changes: 18 additions & 8 deletions src/Bicep.Core/Emit/EmitLimitationCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@
using System.Linq;
using Bicep.Core.DataFlow;
using Bicep.Core.Diagnostics;
using Bicep.Core.Extensions;
using Bicep.Core.Parsing;
using Bicep.Core.Semantics;
using Bicep.Core.Semantics.Metadata;
using Bicep.Core.Syntax;
using Bicep.Core.Syntax.Visitors;
using Bicep.Core.TypeSystem;
using Bicep.Core.TypeSystem.Az;
using Bicep.Core.Utils;
using Bicep.Core.Extensions;
using Bicep.Core.Syntax.Visitors;
using Microsoft.WindowsAzure.ResourceStack.Common.Extensions;
using Newtonsoft.Json.Linq;

Expand Down Expand Up @@ -603,24 +604,33 @@ private static void BlockNamesDistinguishedOnlyByCase(SemanticModel model, IDiag
("asserts", model.Root.AssertDeclarations),
})
{
BlockCaseInsensitiveNameClashes(symbolTypePluralName, symbolsOfType, diagnostics);
BlockCaseInsensitiveNameClashes(symbolTypePluralName, symbolsOfType, s => s.Name, s => s.NameSource, diagnostics);
}

foreach (var objectTypeDeclaration in SyntaxAggregator.AggregateByType<ObjectTypeSyntax>(model.SourceFile.ProgramSyntax))
{
BlockCaseInsensitiveNameClashes("type properties",
objectTypeDeclaration.Properties.SelectMany(p => p.TryGetKeyText() is string key ? (key, p.Key).AsEnumerable() : Enumerable.Empty<(string, SyntaxBase)>()),
t => t.Item1,
t => t.Item2,
diagnostics);
}
}

private static void BlockCaseInsensitiveNameClashes(string symbolTypePluralName, IEnumerable<DeclaredSymbol> symbolsOfType, IDiagnosticWriter diagnostics)
private static void BlockCaseInsensitiveNameClashes<T>(string itemTypePluralName, IEnumerable<T> itemsOfType, Func<T, string> nameExtractor, Func<T, IPositionable> nameSyntaxExtractor, IDiagnosticWriter diagnostics)
{
foreach (var grouping in symbolsOfType.ToLookup(s => s.Name, StringComparer.OrdinalIgnoreCase).Where(g => g.Count() > 1))
foreach (var grouping in itemsOfType.ToLookup(nameExtractor, StringComparer.OrdinalIgnoreCase).Where(g => g.Count() > 1))
{
var clashingSymbols = grouping.Select(s => s.Name).ToArray();
var clashingNames = grouping.Select(nameExtractor).ToArray();

// if any symbols are exact matches, a different diagnostic about multiple declarations will have already been raised
if (clashingSymbols.Distinct().Count() != clashingSymbols.Length)
if (clashingNames.Distinct().Count() != clashingNames.Length)
{
continue;
}

diagnostics.WriteMultiple(grouping.Select(
symbol => DiagnosticBuilder.ForPosition(symbol.NameSource).SymbolsMustBeCaseInsensitivelyUnique(symbolTypePluralName, clashingSymbols)));
item => DiagnosticBuilder.ForPosition(nameSyntaxExtractor(item)).ItemsMustBeCaseInsensitivelyUnique(itemTypePluralName, clashingNames)));
}
}
}
Expand Down

0 comments on commit 09d0d4b

Please sign in to comment.