Skip to content

Commit

Permalink
Fix for issue 15396 (#16330)
Browse files Browse the repository at this point in the history
  • Loading branch information
anthony-c-martin authored Feb 5, 2025
1 parent a90dc1f commit 93c5975
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public TestExtensibilityNamespaceProvider(

protected override TypeSymbol GetNamespaceTypeForConfigManagedExtension(BicepSourceFile sourceFile, ResourceScope targetScope, ArtifactResolutionInfo? artifact, ExtensionDeclarationSyntax? syntax, string extensionName)
{
var aliasName = syntax?.Alias?.IdentifierName ?? extensionName;
var aliasName = syntax?.TryGetSymbolName() ?? extensionName;
if (namespaceCreatorFunc(extensionName, aliasName) is { } namespaceType)
{
return namespaceType;
Expand Down
89 changes: 85 additions & 4 deletions src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -750,11 +750,92 @@ public async Task Implicit_extensions_are_included_in_output()
""");

result.Should().GenerateATemplate();
result.Template.Should().HaveJsonAtPath("$.imports", """
result.Template.Should().HaveJsonAtPath("$.imports['foo']", """
{
"foo": {
"provider": "ThirdPartyExtension",
"version": "1.0.0"
"provider": "ThirdPartyExtension",
"version": "1.0.0"
}
""");

result.Template.Should().HaveJsonAtPath("$.resources['fooRes']", """
{
"import": "foo",
"type": "fooType@v1",
"properties": {
"identifier": "foo",
"properties": {
"required": "bar"
}
}
}
""");
}

[TestMethod]
public async Task Implicit_extensions_generate_correct_symbol_names()
{
// https://github.com/Azure/bicep/issues/15396
var fileSystem = new MockFileSystem();
var services = await ExtensionTestHelper.GetServiceBuilderWithPublishedExtension(ThirdPartyTypeHelper.GetTestTypesTgz(), AllFeaturesEnabled, fileSystem);

fileSystem.File.WriteAllText("/bicepconfig.json", """
{
"experimentalFeaturesEnabled": {
"extensibility": true
},
"extensions": {
"bar": "br:example.azurecr.io/extensions/foo:1.2.3"
}
}
""");
var result = await CompilationHelper.RestoreAndCompile(services, """
extension bar
resource fooRes 'fooType@v1' = {
identifier: 'foo'
properties: {
required: 'bar'
}
}
resource bazRes 'bar:fooType@v1' = {
identifier: 'baz'
properties: {
required: 'bar'
}
}
""");

result.Should().GenerateATemplate();
result.Template.Should().HaveJsonAtPath("$.imports['bar']", """
{
"provider": "ThirdPartyExtension",
"version": "1.0.0"
}
""");

result.Template.Should().HaveJsonAtPath("$.resources['fooRes']", """
{
"import": "bar",
"type": "fooType@v1",
"properties": {
"identifier": "foo",
"properties": {
"required": "bar"
}
}
}
""");

result.Template.Should().HaveJsonAtPath("$.resources['bazRes']", """
{
"import": "bar",
"type": "fooType@v1",
"properties": {
"identifier": "baz",
"properties": {
"required": "bar"
}
}
}
""");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ extension msGraphBeta
//ASSERT
result.Should().GenerateATemplate();
result.Template.Should().NotBeNull();
result.Template.Should().HaveValueAtPath("$.imports.MicrosoftGraphBeta.version", versionBeta);
result.Template.Should().HaveValueAtPath("$.imports.msGraphBeta.version", versionBeta);
}
}
}
3 changes: 2 additions & 1 deletion src/Bicep.Core.IntegrationTests/ProviderImportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public async Task Providers_are_disabled_unless_feature_is_enabled()
extension az
""");
result.Should().HaveDiagnostics(new[] {
("BCP203", DiagnosticLevel.Error, "Using extension declaration requires enabling EXPERIMENTAL feature \"Extensibility\"."),
("BCP203", DiagnosticLevel.Error, """Using extension declaration requires enabling EXPERIMENTAL feature "Extensibility"."""),
("BCP084", DiagnosticLevel.Error, """The symbolic name "az" is reserved. Please use a different symbolic name. Reserved namespaces are "az", "sys"."""),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,27 @@
}
},
"imports": {
"MicrosoftGraphBeta": {
"microsoftGraphBeta": {
"provider": "MicrosoftGraph",
"version": "0.1.8-preview"
}
},
"resources": {
"resourceApp": {
"existing": true,
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/applications@beta",
"properties": {
"uniqueName": "resourceApp"
}
},
"group": {
"existing": true,
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/applications@beta",
"properties": {
"uniqueName": "myGroup"
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"_generator": {
"name": "bicep",
"version": "dev",
"templateHash": "15609368468620239727"
"templateHash": "1993574896961692893"
}
},
"parameters": {
Expand All @@ -24,18 +24,18 @@
}
},
"imports": {
"MicrosoftGraph": {
"microsoftGraphV1_0": {
"provider": "MicrosoftGraph",
"version": "0.1.8-preview"
},
"MicrosoftGraphBeta": {
"microsoftGraphBeta": {
"provider": "MicrosoftGraph",
"version": "0.1.8-preview"
}
},
"resources": {
"appV1::myTestFIC": {
"import": "MicrosoftGraph",
"import": "microsoftGraphV1_0",
"type": "Microsoft.Graph/applications/[email protected]",
"properties": {
"name": "[format('{0}/mytestfic', reference('appV1').uniqueName)]",
Expand All @@ -51,7 +51,7 @@
]
},
"resourceApp": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/applications@beta",
"properties": {
"uniqueName": "resourceApp",
Expand Down Expand Up @@ -84,7 +84,7 @@
}
},
"resourceSp": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/servicePrincipals@beta",
"properties": {
"appId": "[reference('resourceApp').appId]"
Expand All @@ -94,15 +94,15 @@
]
},
"clientApp": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/applications@beta",
"properties": {
"uniqueName": "clientApp",
"displayName": "My Client App"
}
},
"clientSp": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/servicePrincipals@beta",
"properties": {
"appId": "[reference('clientApp').appId]"
Expand All @@ -112,7 +112,7 @@
]
},
"permissionGrant": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/oauth2PermissionGrants@beta",
"properties": {
"clientId": "[reference('clientSp').id]",
Expand All @@ -126,7 +126,7 @@
]
},
"appRoleAssignedTo": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/appRoleAssignedTo@beta",
"properties": {
"appRoleId": "[parameters('appRoleId')]",
Expand All @@ -139,7 +139,7 @@
]
},
"group": {
"import": "MicrosoftGraphBeta",
"import": "microsoftGraphBeta",
"type": "Microsoft.Graph/groups@beta",
"properties": {
"uniqueName": "myGroup",
Expand All @@ -159,12 +159,12 @@
]
},
"appV1": {
"import": "MicrosoftGraph",
"import": "microsoftGraphV1_0",
"type": "Microsoft.Graph/[email protected]",
"properties": {
"displayName": "TestAppV1",
"uniqueName": "testAppV1"
}
}
}
}
}
11 changes: 6 additions & 5 deletions src/Bicep.Core/Emit/TemplateWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void Write(SourceAwareJsonTextWriter writer)

this.EmitExtensionsIfPresent(emitter, program.Extensions);

this.EmitResources(jsonWriter, emitter, program.Resources, program.Modules);
this.EmitResources(jsonWriter, emitter, program.Extensions, program.Resources, program.Modules);

this.EmitOutputsIfPresent(emitter, program.Outputs);

Expand Down Expand Up @@ -1172,6 +1172,7 @@ private ExtensionExpression GetExtensionForLocalDeploy()
private void EmitResources(
PositionTrackingJsonTextWriter jsonWriter,
ExpressionEmitter emitter,
ImmutableArray<ExtensionExpression> extensions,
ImmutableArray<DeclaredResourceExpression> resources,
ImmutableArray<DeclaredModuleExpression> modules)
{
Expand All @@ -1186,7 +1187,7 @@ private void EmitResources(
continue;
}

this.EmitResource(emitter, resource);
this.EmitResource(emitter, extensions, resource);
}

foreach (var module in modules)
Expand All @@ -1203,7 +1204,7 @@ private void EmitResources(
{
emitter.EmitProperty(
emitter.GetSymbolicName(resource.ResourceMetadata),
() => EmitResource(emitter, resource),
() => EmitResource(emitter, extensions, resource),
resource.SourceSyntax);
}

Expand All @@ -1218,7 +1219,7 @@ private void EmitResources(
}
}

private void EmitResource(ExpressionEmitter emitter, DeclaredResourceExpression resource)
private void EmitResource(ExpressionEmitter emitter, ImmutableArray<ExtensionExpression> extensions, DeclaredResourceExpression resource)
{
var metadata = resource.ResourceMetadata;

Expand All @@ -1241,7 +1242,7 @@ private void EmitResource(ExpressionEmitter emitter, DeclaredResourceExpression
emitter.EmitProperty("existing", new BooleanLiteralExpression(null, true));
}

var extensionSymbol = Context.SemanticModel.Root.ExtensionDeclarations.FirstOrDefault(i => metadata.Type.DeclaringNamespace.AliasNameEquals(i.Name));
var extensionSymbol = extensions.FirstOrDefault(i => metadata.Type.DeclaringNamespace.AliasNameEquals(i.Name));
if (extensionSymbol is not null)
{
if (this.Context.SemanticModel.Features.LocalDeployEnabled ||
Expand Down
9 changes: 0 additions & 9 deletions src/Bicep.Core/Registry/IModuleRegistry.cs

This file was deleted.

2 changes: 1 addition & 1 deletion src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public ExtensionNameSource(ExtensionDeclarationSyntax extension)
public ExtensionNamespaceSymbol(ISymbolContext context, ExtensionDeclarationSyntax declaringSyntax, TypeSymbol declaredType)
: base(
context,
declaringSyntax.Alias?.IdentifierName ?? declaredType.Name,
declaringSyntax.TryGetSymbolName() ?? declaredType.Name,
declaringSyntax,
new ExtensionNameSource(declaringSyntax))
{
Expand Down
8 changes: 3 additions & 5 deletions src/Bicep.Core/Semantics/Namespaces/NamespaceProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ public IEnumerable<NamespaceResult> GetNamespaces(
assignedProviders.Add(validType.ExtensionName);
}

var name = extension.Alias?.IdentifierName ?? type.Name;
yield return new(name, type, extension);
yield return new(extension.TryGetSymbolName() ?? type.Name, type, extension);
}

// sys isn't included in the implicit extensions config, because we don't want users to customize it.
Expand Down Expand Up @@ -114,8 +113,7 @@ private TypeSymbol GetNamespaceType(

if (artifactFileLookup.ArtifactLookup.TryGetValue(syntax, out var artifact))
{
var aliasName = syntax.Alias?.IdentifierName;
if (GetNamespaceTypeForArtifact(artifact, sourceFile, targetScope, aliasName).IsSuccess(out var namespaceType, out var errorBuilder))
if (GetNamespaceTypeForArtifact(artifact, sourceFile, targetScope, syntax.TryGetSymbolName()).IsSuccess(out var namespaceType, out var errorBuilder))
{
return namespaceType;
}
Expand All @@ -139,7 +137,7 @@ protected virtual TypeSymbol GetNamespaceTypeForConfigManagedExtension(
ExtensionDeclarationSyntax? syntax,
string extensionName)
{
var aliasName = syntax?.Alias?.IdentifierName ?? extensionName;
var aliasName = syntax?.TryGetSymbolName() ?? extensionName;
var diagBuilder = syntax is { } ? DiagnosticBuilder.ForPosition(syntax) : DiagnosticBuilder.ForDocumentStart();

if (artifact is { })
Expand Down
6 changes: 6 additions & 0 deletions src/Bicep.Core/Syntax/ExtensionDeclarationSyntax.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ public ExtensionDeclarationSyntax(IEnumerable<SyntaxBase> leadingNodes, Token ke

public IdentifierSyntax? Alias => (this.AsClause as AliasAsClauseSyntax)?.Alias;

public string? TryGetSymbolName() => (this.Alias, this.SpecificationString) switch {
(not null, _) => this.Alias.IdentifierName,
(null, IdentifierSyntax value) => value.IdentifierName,
_ => null,
};

public override TextSpan Span => TextSpan.Between(this.Keyword, TextSpan.LastNonNull(this.SpecificationString, this.WithClause, this.AsClause));

public SyntaxBase SourceSyntax => SpecificationString;
Expand Down

0 comments on commit 93c5975

Please sign in to comment.