From 1b3f13ba1d67b60d31ecec88f48bd8c1942da283 Mon Sep 17 00:00:00 2001 From: Anthony Martin <38542602+anthony-c-martin@users.noreply.github.com> Date: Tue, 4 Feb 2025 22:02:14 -0500 Subject: [PATCH 1/2] Fox for issue 15396 --- .../TestExtensibilityNamespaceProvider.cs | 2 +- .../ExtensionRegistryTests.cs | 89 ++++++++++++++++++- .../microsoftGraph/main.existing.json | 8 +- .../extensibility/microsoftGraph/main.json | 26 +++--- src/Bicep.Core/Emit/TemplateWriter.cs | 11 +-- src/Bicep.Core/Registry/IModuleRegistry.cs | 9 -- .../Semantics/ExtensionNamespaceSymbol.cs | 2 +- .../Semantics/Namespaces/NamespaceProvider.cs | 8 +- .../Syntax/ExtensionDeclarationSyntax.cs | 6 ++ 9 files changed, 119 insertions(+), 42 deletions(-) delete mode 100644 src/Bicep.Core/Registry/IModuleRegistry.cs diff --git a/src/Bicep.Core.IntegrationTests/Extensibility/TestExtensibilityNamespaceProvider.cs b/src/Bicep.Core.IntegrationTests/Extensibility/TestExtensibilityNamespaceProvider.cs index 21e1c2ca3a4..63dbc520e83 100644 --- a/src/Bicep.Core.IntegrationTests/Extensibility/TestExtensibilityNamespaceProvider.cs +++ b/src/Bicep.Core.IntegrationTests/Extensibility/TestExtensibilityNamespaceProvider.cs @@ -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; diff --git a/src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs b/src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs index 092e31649ad..16ee96ea2b4 100644 --- a/src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs +++ b/src/Bicep.Core.IntegrationTests/ExtensionRegistryTests.cs @@ -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" + } } } """); diff --git a/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.existing.json b/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.existing.json index 9e7221081aa..78488d37b3c 100644 --- a/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.existing.json +++ b/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.existing.json @@ -14,7 +14,7 @@ } }, "imports": { - "MicrosoftGraphBeta": { + "microsoftGraphBeta": { "provider": "MicrosoftGraph", "version": "0.1.8-preview" } @@ -22,7 +22,7 @@ "resources": { "resourceApp": { "existing": true, - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/applications@beta", "properties": { "uniqueName": "resourceApp" @@ -30,11 +30,11 @@ }, "group": { "existing": true, - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/applications@beta", "properties": { "uniqueName": "myGroup" } } } -} +} \ No newline at end of file diff --git a/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.json b/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.json index 62a6b9c34b0..088a7a4f8e7 100644 --- a/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.json +++ b/src/Bicep.Core.Samples/Files/user_submitted/extensibility/microsoftGraph/main.json @@ -10,7 +10,7 @@ "_generator": { "name": "bicep", "version": "dev", - "templateHash": "15609368468620239727" + "templateHash": "1993574896961692893" } }, "parameters": { @@ -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/federatedIdentityCredentials@v1.0", "properties": { "name": "[format('{0}/mytestfic', reference('appV1').uniqueName)]", @@ -51,7 +51,7 @@ ] }, "resourceApp": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/applications@beta", "properties": { "uniqueName": "resourceApp", @@ -84,7 +84,7 @@ } }, "resourceSp": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/servicePrincipals@beta", "properties": { "appId": "[reference('resourceApp').appId]" @@ -94,7 +94,7 @@ ] }, "clientApp": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/applications@beta", "properties": { "uniqueName": "clientApp", @@ -102,7 +102,7 @@ } }, "clientSp": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/servicePrincipals@beta", "properties": { "appId": "[reference('clientApp').appId]" @@ -112,7 +112,7 @@ ] }, "permissionGrant": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/oauth2PermissionGrants@beta", "properties": { "clientId": "[reference('clientSp').id]", @@ -126,7 +126,7 @@ ] }, "appRoleAssignedTo": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/appRoleAssignedTo@beta", "properties": { "appRoleId": "[parameters('appRoleId')]", @@ -139,7 +139,7 @@ ] }, "group": { - "import": "MicrosoftGraphBeta", + "import": "microsoftGraphBeta", "type": "Microsoft.Graph/groups@beta", "properties": { "uniqueName": "myGroup", @@ -159,7 +159,7 @@ ] }, "appV1": { - "import": "MicrosoftGraph", + "import": "microsoftGraphV1_0", "type": "Microsoft.Graph/applications@v1.0", "properties": { "displayName": "TestAppV1", @@ -167,4 +167,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/Bicep.Core/Emit/TemplateWriter.cs b/src/Bicep.Core/Emit/TemplateWriter.cs index 3c169f11fe8..224e5937550 100644 --- a/src/Bicep.Core/Emit/TemplateWriter.cs +++ b/src/Bicep.Core/Emit/TemplateWriter.cs @@ -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); @@ -1172,6 +1172,7 @@ private ExtensionExpression GetExtensionForLocalDeploy() private void EmitResources( PositionTrackingJsonTextWriter jsonWriter, ExpressionEmitter emitter, + ImmutableArray extensions, ImmutableArray resources, ImmutableArray modules) { @@ -1186,7 +1187,7 @@ private void EmitResources( continue; } - this.EmitResource(emitter, resource); + this.EmitResource(emitter, extensions, resource); } foreach (var module in modules) @@ -1203,7 +1204,7 @@ private void EmitResources( { emitter.EmitProperty( emitter.GetSymbolicName(resource.ResourceMetadata), - () => EmitResource(emitter, resource), + () => EmitResource(emitter, extensions, resource), resource.SourceSyntax); } @@ -1218,7 +1219,7 @@ private void EmitResources( } } - private void EmitResource(ExpressionEmitter emitter, DeclaredResourceExpression resource) + private void EmitResource(ExpressionEmitter emitter, ImmutableArray extensions, DeclaredResourceExpression resource) { var metadata = resource.ResourceMetadata; @@ -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 || diff --git a/src/Bicep.Core/Registry/IModuleRegistry.cs b/src/Bicep.Core/Registry/IModuleRegistry.cs deleted file mode 100644 index 5430845657f..00000000000 --- a/src/Bicep.Core/Registry/IModuleRegistry.cs +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -namespace Bicep.Core.Registry -{ - public interface IModuleRegistry : IArtifactRegistry - { - } -} diff --git a/src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs b/src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs index 77b121ecc74..b05a80a1231 100644 --- a/src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs +++ b/src/Bicep.Core/Semantics/ExtensionNamespaceSymbol.cs @@ -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)) { diff --git a/src/Bicep.Core/Semantics/Namespaces/NamespaceProvider.cs b/src/Bicep.Core/Semantics/Namespaces/NamespaceProvider.cs index 2a4aece9adb..8884eecf359 100644 --- a/src/Bicep.Core/Semantics/Namespaces/NamespaceProvider.cs +++ b/src/Bicep.Core/Semantics/Namespaces/NamespaceProvider.cs @@ -60,8 +60,7 @@ public IEnumerable 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. @@ -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; } @@ -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 { }) diff --git a/src/Bicep.Core/Syntax/ExtensionDeclarationSyntax.cs b/src/Bicep.Core/Syntax/ExtensionDeclarationSyntax.cs index f0d624cfd4e..8b69992bb33 100644 --- a/src/Bicep.Core/Syntax/ExtensionDeclarationSyntax.cs +++ b/src/Bicep.Core/Syntax/ExtensionDeclarationSyntax.cs @@ -34,6 +34,12 @@ public ExtensionDeclarationSyntax(IEnumerable 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; From cd4f1001ebd817b23c1926bb716e101446295a3f Mon Sep 17 00:00:00 2001 From: Anthony Martin <38542602+anthony-c-martin@users.noreply.github.com> Date: Wed, 5 Feb 2025 05:51:41 -0500 Subject: [PATCH 2/2] Fix tests --- .../MsGraphTypesViaRegistryTests.cs | 2 +- src/Bicep.Core.IntegrationTests/ProviderImportTests.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/MsGraphTypesViaRegistryTests.cs b/src/Bicep.Core.IntegrationTests/MsGraphTypesViaRegistryTests.cs index 619ed830b14..894337cabf2 100644 --- a/src/Bicep.Core.IntegrationTests/MsGraphTypesViaRegistryTests.cs +++ b/src/Bicep.Core.IntegrationTests/MsGraphTypesViaRegistryTests.cs @@ -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); } } } diff --git a/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs b/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs index 54118be47b6..8a905e5bcf1 100644 --- a/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs +++ b/src/Bicep.Core.IntegrationTests/ProviderImportTests.cs @@ -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"."""), }); }