Skip to content

Commit

Permalink
Raise an error-level diagnostic when two symbols of a given type are …
Browse files Browse the repository at this point in the history
…differentiated only by casing (#11453)

Resolves #502

Because this change is accounting for an intermediate language
limitation, templates that use symbols *of different types* whose names
are differ only by casing are still permitted, since the ARM engine uses
a separate namespace for each category of symbol.
###### Microsoft Reviewers:
codeflow:open?pullrequest=https://github.com/Azure/bicep/pull/11453&drop=dogfoodAlpha
  • Loading branch information
jeskew committed Aug 3, 2023
1 parent 230eecb commit 2b6539d
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 0 deletions.
68 changes: 68 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4906,4 +4906,72 @@ public void Test_Issue10994()

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
}

// https://github.com/Azure/bicep/issues/502
[TestMethod]
public void Test_Issue502()
{
var result = CompilationHelper.Compile("""
var foo = 1
var FoO = 2
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP353", DiagnosticLevel.Error, "The variables \"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 variables \"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("""
param foo int = 1
param FoO int = 2
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP353", DiagnosticLevel.Error, "The parameters \"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 parameters \"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("""
output foo int = 1
output FoO int = 2
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP353", DiagnosticLevel.Error, "The outputs \"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 outputs \"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)), """
type foo = string
type FoO = int
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("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."),
("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(AssertsEnabled: true)), """
assert foo = true
assert FoO = true
""");

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP353", DiagnosticLevel.Error, "The asserts \"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 asserts \"foo\", \"FoO\" differ only in casing. The ARM deployments engine is not case sensitive and will not be able to distinguish between them."),
});

// if the two symbols are of different types, ARM will be able to distinguish between them
result = CompilationHelper.Compile("""
param foo int = 1
var FoO = 2
""");

result.ExcludingLinterDiagnostics().Should().NotHaveAnyDiagnostics();
}
}
5 changes: 5 additions & 0 deletions src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1980,6 +1980,11 @@ public ErrorDiagnostic RuntimeValueNotAllowedInFunctionDeclaration(string? acces
TextSpan,
"BCP352",
$"Failed to evaluate variable \"{name}\": {message}");

public ErrorDiagnostic SymbolsMustBeCaseInsensitivelyUnique(string symbolTypePluralName, IEnumerable<string> symbolNames) => 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.");
}

public static DiagnosticBuilderInternal ForPosition(TextSpan span)
Expand Down
33 changes: 33 additions & 0 deletions src/Bicep.Core/Emit/EmitLimitationCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public static EmitLimitationInfo Calculate(SemanticModel model)
BlockTestFrameworkWithoutExperimentalFeaure(model, diagnostics);
BlockUserDefinedTypesWithUserDefinedFunctions(model, diagnostics);
BlockAssertsWithoutExperimentalFeatures(model, diagnostics);
BlockNamesDistinguishedOnlyByCase(model, diagnostics);
var paramAssignments = CalculateParameterAssignments(model, diagnostics);

return new(diagnostics.GetDiagnostics(), moduleScopeData, resourceScopeData, paramAssignments);
Expand Down Expand Up @@ -590,5 +591,37 @@ private static void BlockAssertsWithoutExperimentalFeatures(SemanticModel model,
}
}
}

private static void BlockNamesDistinguishedOnlyByCase(SemanticModel model, IDiagnosticWriter diagnostics)
{
foreach (var (symbolTypePluralName, symbolsOfType) in new (string, IEnumerable<DeclaredSymbol>)[]
{
("parameters", model.Root.ParameterDeclarations),
("variables", model.Root.VariableDeclarations),
("outputs", model.Root.OutputDeclarations),
("types", model.Root.TypeDeclarations),
("asserts", model.Root.AssertDeclarations),
})
{
BlockCaseInsensitiveNameClashes(symbolTypePluralName, symbolsOfType, diagnostics);
}
}

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

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

diagnostics.WriteMultiple(grouping.Select(
symbol => DiagnosticBuilder.ForPosition(symbol.NameSource).SymbolsMustBeCaseInsensitivelyUnique(symbolTypePluralName, clashingSymbols)));
}
}
}
}

0 comments on commit 2b6539d

Please sign in to comment.