Skip to content

Commit

Permalink
Fix linter discrepancies when optional module name is enabled (#16106)
Browse files Browse the repository at this point in the history
This PR addresses minor issues with the optional module name feature,
where certain linter rules were incorrectly triggered or omitted when
the feature was enabled.

Closes #12895.
###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/16106)
  • Loading branch information
shenglol authored Jan 15, 2025
1 parent 2ece9de commit e0d1c9c
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 8 deletions.
21 changes: 21 additions & 0 deletions src/Bicep.Core.IntegrationTests/LoopInvariantTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using Bicep.Core.Diagnostics;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Utils;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -258,6 +259,26 @@ public void OptionalInvariantModulePropertiesWhenRequiredPropertyIsMissingShould
});
}

[TestMethod]
public void OptionalModuleNameShouldNotProduceWarning()
{
const string text = @"
module mod 'mod.bicep' = [for a in []: {
scope: resourceGroup()
params: {
foo: 's'
}
}]
";
var serviceBuilder = new ServiceBuilder().WithFeatureOverrides(new(OptionalModuleNamesEnabled: true));
var result = CompilationHelper.Compile(
serviceBuilder,
("main.bicep", text),
("mod.bicep", "param foo string"));

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

[TestMethod]
public void MultipleInvariantModulePropertiesShouldProduceWarning()
{
Expand Down
31 changes: 31 additions & 0 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4231,6 +4231,37 @@ public void Test_Issue6065()
name: 'mysql'
}
output sql resource = sql
"));

result.ExcludingLinterDiagnostics().Should().HaveDiagnostics(new[]
{
("BCP320", DiagnosticLevel.Error, "The properties of module output resources cannot be accessed directly. To use the properties of this resource, pass it as a resource-typed parameter to another module and access the parameter's properties therein."),
});
}

[TestMethod]
public void Test_Issue12895()
{

var result = CompilationHelper.Compile(Services.WithFeatureOverrides(new(ResourceTypedParamsAndOutputsEnabled: true, OptionalModuleNamesEnabled: true)),
("main.bicep", @"
module mymodule 'test.bicep' = {
}
resource myresource 'Microsoft.Sql/servers@2021-08-01-preview' = {
name: 'myothersql'
location: resourceGroup().location
properties: {
administratorLogin: mymodule.outputs.sql.properties.administratorLogin
}
}
"),
("test.bicep", @"
resource sql 'Microsoft.Sql/servers@2021-08-01-preview' existing = {
name: 'mysql'
}
output sql resource = sql
"));

Expand Down
19 changes: 17 additions & 2 deletions src/Bicep.Core/Emit/EmitLimitationCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,14 @@ private static void DetectUnexpectedModuleLoopInvariantProperties(SemanticModel
continue;
}

if (semanticModel.Features.OptionalModuleNamesEnabled &&
propertyMap.Keys.FirstOrDefault(x => x.Name.Equals(LanguageConstants.ModuleNamePropertyName)) is null)
{

// The module name is generated and implictly uses the loop variable (copyIndex()).
continue;
}

var indexVariable = @for.IndexVariable;
if (propertyMap.All(pair => IsInvariant(semanticModel, itemVariable, indexVariable, pair.Value)))
{
Expand Down Expand Up @@ -469,9 +477,16 @@ private static void BlockModuleOutputResourcePropertyAccess(SemanticModel model,
.Select(syntaxToBlock => DiagnosticBuilder.ForPosition(syntaxToBlock).ModuleOutputResourcePropertyAccessDetected()));

private static bool IsModuleOutputResourceRuntimePropertyAccess(SemanticModel model, SyntaxBase syntax)
=> syntax is PropertyAccessSyntax propertyAccess &&
{
if (syntax is PropertyAccessSyntax propertyAccess &&
model.ResourceMetadata.TryLookup(propertyAccess.BaseExpression) is ModuleOutputResourceMetadata &&
!AzResourceTypeProvider.ReadWriteDeployTimeConstantPropertyNames.Contains(propertyAccess.PropertyName.IdentifierName);
!AzResourceTypeProvider.ReadWriteDeployTimeConstantPropertyNames.Contains(propertyAccess.PropertyName.IdentifierName))
{
return true;
}

return false;
}

private static bool IsModuleOutputResourceListFunction(SemanticModel model, SyntaxBase syntax)
=> syntax is InstanceFunctionCallSyntax instanceFunctionCall &&
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Intermediate/ExpressionBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ moduleCollectionOutputs.BaseExpression is ArrayAccessSyntax moduleArrayAccess &&
Context.SemanticModel.ResourceMetadata.TryLookup(propertyAccess.BaseExpression) is ModuleOutputResourceMetadata moduleCollectionOutputMetadata &&
moduleCollectionOutputMetadata.Module.IsCollection)
{
var indexContext = TryGetReplacementContext(moduleCollectionOutputMetadata.NameSyntax, moduleArrayAccess.IndexExpression, propertyAccess);
var indexContext = TryGetReplacementContext(moduleCollectionOutputMetadata.Module, moduleArrayAccess.IndexExpression, propertyAccess);
return ConvertResourcePropertyAccess(propertyAccess, moduleCollectionOutputMetadata, indexContext, propertyAccess.PropertyName.IdentifierName, flags);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ namespace Bicep.Core.Semantics.Metadata
public record ModuleOutputResourceMetadata(
ResourceType Type,
ModuleSymbol Module,
SyntaxBase NameSyntax,
string OutputName)
: ResourceMetadata(Type, IsExistingResource: true)
{
Expand Down
7 changes: 3 additions & 4 deletions src/Bicep.Core/Semantics/Metadata/ResourceMetadataCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ public ResourceMetadataCache(SemanticModel semanticModel)
// the discovery is driven by semantics not syntax.
public ResourceMetadata? TryAdd(ModuleSymbol module, string output)
{
if (module.TryGetBodyPropertyValue(AzResourceTypeProvider.ResourceNamePropertyName) is { } nameSyntax &&
module.TryGetBodyObjectType() is ObjectType objectType &&
if (module.TryGetBodyObjectType() is ObjectType objectType &&
objectType.Properties.TryGetValue(LanguageConstants.ModuleOutputsPropertyName, out var outputsProperty) &&
outputsProperty.TypeReference.Type is ObjectType outputsType &&
outputsType.Properties.TryGetValue(output, out var property))
{
if (property.TypeReference.Type is ResourceType resourceType)
{
var metadata = new ModuleOutputResourceMetadata(resourceType, module, nameSyntax, output);
var metadata = new ModuleOutputResourceMetadata(resourceType, module, output);
moduleOutputLookup.TryAdd((module, output), metadata);
return metadata;
}
Expand Down Expand Up @@ -155,7 +154,7 @@ private bool IsModuleScalarOutputAccess(PropertyAccessSyntax propertyAccessSynta
childPropertyAccess.PropertyName.IdentifierName == LanguageConstants.ModuleOutputsPropertyName &&
childPropertyAccess.BaseExpression is VariableAccessSyntax grandChildAccess &&
this.semanticModel.GetSymbolInfo(grandChildAccess) is ModuleSymbol module &&
module.TryGetBodyPropertyValue(AzResourceTypeProvider.ResourceNamePropertyName) is { } name)
(module.TryGetBodyPropertyValue(AzResourceTypeProvider.ResourceNamePropertyName) is not null || this.semanticModel.Features.OptionalModuleNamesEnabled))
{
symbol = module;
return true;
Expand Down

0 comments on commit e0d1c9c

Please sign in to comment.