adress pr comments

src/Bicep.Core.UnitTests/BicepTestConstants.cs

@@ -53,7 +53,8 @@ public static class BicepTestConstants
  public static readonly ITemplateSpecRepositoryFactory TemplateSpecRepositoryFactory = StrictMock.Of<ITemplateSpecRepositoryFactory>().Object;
 
  // Linter rules added to this list will be automtically disabled for most tests.
- public static readonly string[] AnalyzerRulesToDisableInTests = Array.Empty<string>();
+ // use-recent-api-versions is problematic for tests but it's off by default so doesn't need to appear here
+ public static readonly string[] AnalyzerRulesToDisableInTests = Array.Empty<string>(); 
 
  public static readonly RootConfiguration BuiltInConfigurationWithAllAnalyzersEnabled = IConfigurationManager.GetBuiltInConfiguration();
  public static readonly RootConfiguration BuiltInConfigurationWithAllAnalyzersDisabled = IConfigurationManager.GetBuiltInConfiguration().WithAllAnalyzersDisabled();
@@ -135,27 +136,27 @@ public static Mock<ITelemetryProvider> CreateMockTelemetryProvider()
  }
 
  public static BinaryData BicepProviderManifestWithEmptyTypesLayer = BinaryData.FromString($$"""
-{
- "schemaVersion": 2,
- "mediaType": "application/vnd.oci.image.manifest.v1+json",
- "artifactType": "{{BicepMediaTypes.BicepProviderArtifactType}}",
- "config": {
- "mediaType": "{{BicepMediaTypes.BicepProviderConfigV1}}",
- "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
- "size": 2
- },
- "layers": [
- {
- "mediaType": "{{BicepMediaTypes.BicepProviderArtifactLayerV1TarGzip}}",
- "digest": "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
- "size": 0
- }
- ],
- "annotations": {
- "bicep.serialization.format": "v1",
- "org.opencontainers.image.created": "2023-05-04T16:40:05Z"
- }
-}
-""");
+ {
+  "schemaVersion": 2,
+  "mediaType": "application/vnd.oci.image.manifest.v1+json",
+  "artifactType": "{{BicepMediaTypes.BicepProviderArtifactType}}",
+  "config": {
+  "mediaType": "{{BicepMediaTypes.BicepProviderConfigV1}}",
+  "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
+  "size": 2
+  },
+  "layers": [
+  {
+  "mediaType": "{{BicepMediaTypes.BicepProviderArtifactLayerV1TarGzip}}",
+  "digest": "sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
+  "size": 0
+  }
+  ],
+  "annotations": {
+  "bicep.serialization.format": "v1",
+  "org.opencontainers.image.created": "2023-05-04T16:40:05Z"
+  }
+ }
+ """);
  }
 }

src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs

@@ -2110,7 +2110,7 @@ public ErrorDiagnostic RuntimeValueNotAllowedInFunctionDeclaration(string? acces
  "BCP380",
  $"Artifacts of type: \"{artifactType}\" are not supported."
  );
- 
+
  public FixableDiagnostic ProviderDeclarationViaImportKeywordIsDeprecated(ProviderDeclarationSyntax syntax)
  {
  var codeFix = new CodeFix(
@@ -2132,7 +2132,7 @@ public FixableDiagnostic ProviderDeclarationViaImportKeywordIsDeprecated(Provide
  public ErrorDiagnostic ArtifactFilePathCouldNotBeResolved(string ociManifestPath) => new(
  TextSpan,
  "BCP382",
- $"The artifact file path could not be resolved from the OCI manifest file: \"{ociManifestPath}\"."
+ $"The provider package is malformed and could not be loaded from \"{ociManifestPath}\"."
  );
  }
 

src/Bicep.Core/Registry/OciArtifactRegistry.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -88,8 +89,12 @@ public override bool IsArtifactRestoreRequired(OciArtifactReference reference)
  {
  ArtifactType.Module => !this.FileResolver.FileExists(this.GetArtifactFileUri(reference, ArtifactFileType.ModuleMain)),
  ArtifactType.Provider => !this.FileResolver.FileExists(this.GetArtifactFileUri(reference, ArtifactFileType.Provider)),
- _ => default // should never happen
+ _ => throw new UnreachableException()
  };
+
+ // TODO: Provider artifacts don't write a ModuleMain file, so this code is incorrect.
+ // That can be solved by only writing layer data files only (see code under features.PublishSourceEnabled)
+ // and not main.json directly (https://github.com/Azure/bicep/issues/11900)
  return artifactFilesNotFound ||
  !this.FileResolver.FileExists(this.GetArtifactFileUri(reference, ArtifactFileType.Manifest)) ||
  !this.FileResolver.FileExists(this.GetArtifactFileUri(reference, ArtifactFileType.Metadata));
@@ -138,7 +143,7 @@ public override ResultWithDiagnostic<Uri> TryGetLocalArtifactEntryPointUri(OciAr
  {
  ArtifactType.Module => ArtifactFileType.ModuleMain,
  ArtifactType.Provider => ArtifactFileType.Provider,
- _ => default // should never happen
+ _ => throw new UnreachableException()
  };
 
  var localUri = this.GetArtifactFileUri(reference, artifactFileType);

src/Bicep.Core/Semantics/DeclarationVisitor.cs

@@ -162,55 +162,55 @@ public override void VisitAssertDeclarationSyntax(AssertDeclarationSyntax syntax
  public override void VisitProviderDeclarationSyntax(ProviderDeclarationSyntax syntax)
  {
  base.VisitProviderDeclarationSyntax(syntax);
+ DeclareSymbol(new ProviderNamespaceSymbol(this.context, syntax, ResolveProviderSymbol(syntax)));
+ }
 
- TypeSymbol resolveProviderSymbol()
+ private TypeSymbol ResolveProviderSymbol(ProviderDeclarationSyntax syntax)
+ {
+ if (!features.ExtensibilityEnabled)
  {
- if (!features.ExtensibilityEnabled)
- {
- return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).ProvidersAreDisabled());
- }
-
- // Check for interpolated specification strings
- if (syntax.SpecificationString is StringSyntax specificationString && specificationString.IsInterpolated())
- {
- return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.SpecificationString).ProviderSpecificationInterpolationUnsupported());
- }
+ return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).ProvidersAreDisabled());
+ }
 
- if (!syntax.Specification.IsValid)
- {
- return (syntax.SpecificationString is StringSyntax)
- ? ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.Specification).InvalidProviderSpecification())
- : ErrorType.Empty();
- }
+ // Check for interpolated specification strings
+ if (syntax.SpecificationString is StringSyntax specificationString && specificationString.IsInterpolated())
+ {
+ return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.SpecificationString).ProviderSpecificationInterpolationUnsupported());
+ }
 
- // Check if the MSGraph provider is recognized and enabled
- if (syntax.Specification.Name == MicrosoftGraphNamespaceType.BuiltInName && !features.MicrosoftGraphPreviewEnabled)
- {
- return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).UnrecognizedProvider(syntax.Specification.Name));
- }
+ if (!syntax.Specification.IsValid)
+ {
+ return (syntax.SpecificationString is StringSyntax)
+ ? ErrorType.Create(DiagnosticBuilder.ForPosition(syntax.Specification).InvalidProviderSpecification())
+ : ErrorType.Empty();
+ }
 
- Uri? providerUri = null;
- if (syntax.Specification.Name == AzNamespaceType.BuiltInName &&
- features.DynamicTypeLoadingEnabled &&
- !this.artifactFileLookup.TryGetProviderFileUri(syntax).IsSuccess(out providerUri, out var providerUriLookupErrorBuilder))
- {
- return ErrorType.Create(providerUriLookupErrorBuilder(DiagnosticBuilder.ForPosition(syntax)));
- }
+ // Check if the MSGraph provider is recognized and enabled
+ if (syntax.Specification.Name == MicrosoftGraphNamespaceType.BuiltInName && !features.MicrosoftGraphPreviewEnabled)
+ {
+ return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).UnrecognizedProvider(syntax.Specification.Name));
+ }
 
- ResourceTypesProviderDescriptor providerDescriptor = new(
- syntax.Specification.Name,
- syntax.Alias?.IdentifierName,
- providerUri,
- syntax.Specification.Version);
+ Uri? providerUri = null;
+ if (syntax.Specification.Name == AzNamespaceType.BuiltInName &&
+ features.DynamicTypeLoadingEnabled &&
+ !this.artifactFileLookup.TryGetProviderFileUri(syntax).IsSuccess(out providerUri, out var providerUriLookupErrorBuilder))
+ {
+ return ErrorType.Create(providerUriLookupErrorBuilder(DiagnosticBuilder.ForPosition(syntax)));
+ }
 
- if (namespaceProvider.TryGetNamespace(providerDescriptor, targetScope, features, sourceFileKind) is not { } namespaceType)
- {
- return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).UnrecognizedProvider(syntax.Specification!.Name));
- }
+ ResourceTypesProviderDescriptor providerDescriptor = new(
+ syntax.Specification.Name,
+ syntax.Alias?.IdentifierName,
+ providerUri,
+ syntax.Specification.Version);
 
- return namespaceType;
+ if (namespaceProvider.TryGetNamespace(providerDescriptor, targetScope, features, sourceFileKind) is not { } namespaceType)
+ {
+ return ErrorType.Create(DiagnosticBuilder.ForPosition(syntax).UnrecognizedProvider(syntax.Specification!.Name));
  }
- DeclareSymbol(new ProviderNamespaceSymbol(this.context, syntax, resolveProviderSymbol()));
+
+ return namespaceType;
  }
 
  public override void VisitParameterAssignmentSyntax(ParameterAssignmentSyntax syntax)

src/Bicep.Core/Semantics/Namespaces/DefaultNamespaceProvider.cs

@@ -29,14 +29,14 @@ public DefaultNamespaceProvider(IResourceTypeProviderFactory resourceTypeLoaderF
  this.resourceTypeLoaderFactory = resourceTypeLoaderFactory;
  this.providerLookup = new Dictionary<string, GetNamespaceDelegate>
  {
- [AzNamespaceType.BuiltInName] = CreateAzNamespace,
+ [AzNamespaceType.BuiltInName] = TryCreateAzNamespace,
  [SystemNamespaceType.BuiltInName] = (providerDescriptor, resourceScope, features, sourceFileKind) => SystemNamespaceType.Create(providerDescriptor.Alias, features, sourceFileKind),
  [K8sNamespaceType.BuiltInName] = (providerDescriptor, resourceScope, features, sourceFileKind) => K8sNamespaceType.Create(providerDescriptor.Alias),
  [MicrosoftGraphNamespaceType.BuiltInName] = (providerDescriptor, resourceScope, features, sourceFileKind) => MicrosoftGraphNamespaceType.Create(providerDescriptor.Alias),
  }.ToImmutableDictionary();
  }
 
- private NamespaceType? CreateAzNamespace(ResourceTypesProviderDescriptor providerDescriptor, ResourceScope scope, IFeatureProvider features, BicepSourceFileKind sourceFileKind)
+ private NamespaceType? TryCreateAzNamespace(ResourceTypesProviderDescriptor providerDescriptor, ResourceScope scope, IFeatureProvider features, BicepSourceFileKind sourceFileKind)
  {
  if (!features.DynamicTypeLoadingEnabled)
  {

src/Bicep.Core/TypeSystem/Az/OciTypeLoader.cs

@@ -13,7 +13,7 @@ public class OciTypeLoader : TypeLoader
  {
  private readonly ImmutableDictionary<string, byte[]> typesCache;
  private const string typesArtifactFilename = "types.tgz";
- public OciTypeLoader(ImmutableDictionary<string, byte[]> typesCache)
+ private OciTypeLoader(ImmutableDictionary<string, byte[]> typesCache)
  {
  this.typesCache = typesCache;
  }
@@ -33,7 +33,7 @@ public static OciTypeLoader FromTgz(string typesFileDir)
  if (entry.DataStream is null)
  {
  var errorMessage = $"Failed to restore {entry.Name} from OCI provider data";
- throw new ArgumentException(errorMessage, entry.Name);
+ throw new InvalidOperationException(errorMessage);
  }
 
  using var memoryStream = new MemoryStream();
@@ -53,7 +53,7 @@ public static OciTypeLoader FromTgz(string typesFileDir)
 
  protected override Stream GetContentStreamAtPath(string path)
  {
- if (typesCache.TryGetValue($"{path}", out var bytes))
+ if (typesCache.TryGetValue(path, out var bytes))
  {
  return new MemoryStream(bytes);
  }

src/Bicep.Core/TypeSystem/Az/ResourceTypeProviderFactory.cs

@@ -16,29 +16,33 @@
 
 namespace Bicep.Core.TypeSystem
 {
+
+
  public class ResourceTypeProviderFactory : IResourceTypeProviderFactory
  {
+ private record Key(string Name, string Version);
 
- private readonly (string, string) BuiltInAzLoaderKey = ("az", IResourceTypeProvider.BuiltInVersion);
- private readonly Dictionary<(string, string), Lazy<IResourceTypeProvider>> resourceTypeLoaders;
+ private readonly Key BuiltInAzLoaderKey = new("az", IResourceTypeProvider.BuiltInVersion);
+ private readonly Dictionary<Key, IResourceTypeProvider> resourceTypeLoaders;
 
  public ResourceTypeProviderFactory()
  {
  resourceTypeLoaders = new() {
- {BuiltInAzLoaderKey, new(new AzResourceTypeProvider(new AzResourceTypeLoader(new AzTypeLoader())))},
+ {BuiltInAzLoaderKey, new AzResourceTypeProvider(new AzResourceTypeLoader(new AzTypeLoader()))},
  };
  }
 
  public ResultWithDiagnostic<IResourceTypeProvider> GetResourceTypeProvider(ResourceTypesProviderDescriptor providerDescriptor, IFeatureProvider features)
  {
  if (!features.DynamicTypeLoadingEnabled)
  {
- return new(resourceTypeLoaders[BuiltInAzLoaderKey].Value);
+ return new(resourceTypeLoaders[BuiltInAzLoaderKey]);
  }
- var key = (providerDescriptor.Alias, providerDescriptor.Version);
+ var key = new Key(providerDescriptor.Alias, providerDescriptor.Version);
+
  if (resourceTypeLoaders.ContainsKey(key))
  {
- return new(resourceTypeLoaders[key].Value);
+ return new(resourceTypeLoaders[key]);
  }
 
  // compose the path to the OCI manifest based on the cache root directory and provider version
@@ -58,20 +62,19 @@ public ResultWithDiagnostic<IResourceTypeProvider> GetResourceTypeProvider(Resou
  }
 
  // Register a new types loader
- Lazy<IResourceTypeProvider> newResourceTypeLoader = providerDescriptor.Alias switch
+ IResourceTypeProvider newResourceTypeLoader = providerDescriptor.Alias switch
  {
- AzNamespaceType.BuiltInName => new(new AzResourceTypeProvider(new AzResourceTypeLoader(OciTypeLoader.FromTgz(providerDescriptor.Path)), providerDescriptor.Version)),
+ AzNamespaceType.BuiltInName => new AzResourceTypeProvider(new AzResourceTypeLoader(OciTypeLoader.FromTgz(providerDescriptor.Path)), providerDescriptor.Version),
  _ => throw new NotImplementedException($"The provider {providerDescriptor.Alias} is not supported."),
  };
 
- resourceTypeLoaders[key] = newResourceTypeLoader;
- return new(newResourceTypeLoader.Value);
+ return new(resourceTypeLoaders[key] = newResourceTypeLoader);
 
  }
 
  public IResourceTypeProvider GetBuiltInAzResourceTypesProvider()
  {
- return resourceTypeLoaders[BuiltInAzLoaderKey].Value;
+ return resourceTypeLoaders[BuiltInAzLoaderKey];
  }
  }
 }

src/Bicep.LangServer/IServiceCollectionExtensions.cs

@@ -23,7 +23,6 @@ public static class IServiceCollectionExtensions
 {
  public static IServiceCollection AddBicepCore(this IServiceCollection services) => services
  .AddSingleton<INamespaceProvider, DefaultNamespaceProvider>()
- .AddSingleton<IResourceTypeLoader, AzResourceTypeLoader>()
  .AddSingleton<IResourceTypeProviderFactory, ResourceTypeProviderFactory>()
  .AddSingleton<IContainerRegistryClientFactory, ContainerRegistryClientFactory>()
  .AddSingleton<ITemplateSpecRepositoryFactory, TemplateSpecRepositoryFactory>()