Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue 15396 #16330

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Fix for issue 15396 #16330

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b3f13b
Fox for issue 15396
anthony-c-martin Feb 5, 2025
cd4f100
Fix tests
anthony-c-martin Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Bicep.Core.IntegrationTests/Extensibility/TestExtensibilityNamespaceProvider.cs
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,11 +751,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
8 changes: 4 additions & 4 deletions src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.existing.json
View file
Open in desktop
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"
}
}
}
}
}
26 changes: 13 additions & 13 deletions src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading