Refactor: Use single Diagnostic class to represent all core diagnos…

…tics (#14875)

The split class structure between `ErrorDiagnostic` & `Diagnostic` is
something I've been meaning to remove since the very early days of
Bicep. I've decided to do it now, because it'll simplify the refactor
needed for #14842.

The majority of the changes in this PR were made using refactoring
tools, not manually. The main changes I made were:
* Removed `ErrorDiagnostic`, `ErrorBuilderDelegate`,
`FixableErrorDiagnostic`
* Converted `diag is ErrorDiagnostic` type checks to use `diag.Level ==
DiagnosticLevel.Error` instead
* Renamed `ResultWithDiagnostic` to `ResultWithDiagnosticBuilder`
instead to more accurately represent its purpose
* Created `ResultWithDiagnotic<T>`, and replaced usage of `Result<T,
Diagnostic>` with it
* Other changes were mostly downstream changes of the above

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14875)

src/Bicep.Cli/Commands/FormatCommand.cs

@@ -44,7 +44,7 @@ public int Run(FormatArguments args)
         if (!this.fileResolver.TryRead(inputUri).IsSuccess(out var fileContents, out var failureBuilder))
         {
             var diagnostic = failureBuilder(DiagnosticBuilder.ForPosition(new TextSpan(0, 0)));
-            throw new ErrorDiagnosticException(diagnostic);
+            throw new DiagnosticException(diagnostic);
         }
 
         BaseParser parser = PathHelper.HasBicepExtension(inputUri) ? new Parser(fileContents) : new ParamsParser(fileContents);

src/Bicep.Core.IntegrationTests/ModuleTests.cs

@@ -179,9 +179,9 @@ param inputb string
             success.Should().BeFalse();
         }
 
-        private delegate bool TryReadDelegate(Uri fileUri, out string? fileContents, out DiagnosticBuilder.ErrorBuilderDelegate? failureBuilder);
+        private delegate bool TryReadDelegate(Uri fileUri, out string? fileContents, out DiagnosticBuilder.DiagnosticBuilderDelegate? failureBuilder);
 
-        private static void SetupFileReaderMock(Mock<IFileResolver> mockFileResolver, Uri fileUri, string? fileContents, DiagnosticBuilder.ErrorBuilderDelegate? failureBuilder)
+        private static void SetupFileReaderMock(Mock<IFileResolver> mockFileResolver, Uri fileUri, string? fileContents, DiagnosticBuilder.DiagnosticBuilderDelegate? failureBuilder)
         {
             mockFileResolver.Setup(x => x.TryRead(fileUri)).Returns(ResultHelper.Create(fileContents, failureBuilder));
         }
@@ -197,7 +197,7 @@ public void SourceFileGroupingBuilder_build_should_throw_diagnostic_exception_if
             SetupFileReaderMock(mockFileResolver, fileUri, null, x => x.ErrorOccurredReadingFile("Mock read failure!"));
 
             Action buildAction = () => SourceFileGroupingBuilder.Build(mockFileResolver.Object, mockDispatcher, mockConfigurationManager, new Workspace(), fileUri, BicepTestConstants.FeatureProviderFactory);
-            buildAction.Should().Throw<ErrorDiagnosticException>()
+            buildAction.Should().Throw<DiagnosticException>()
                 .And.Diagnostic.Should().HaveCodeAndSeverity("BCP091", DiagnosticLevel.Error).And.HaveMessage("An error occurred reading file. Mock read failure!");
         }
 

src/Bicep.Core.UnitTests/Assertions/DiagnosticBuilderAssertions.cs

@@ -22,9 +22,21 @@ public DiagnosticBuilderAssertions(DiagnosticBuilder.DiagnosticBuilderDelegate d
 
         protected override string Identifier => "DiagnosticBuilderDelegate";
 
+        public AndConstraint<DiagnosticBuilderAssertions> HaveCode(string code, string because = "", params object[] becauseArgs)
+        {
+            var diagnostic = GetDiagnosticFromSubject();
+
+            using (new AssertionScope())
+            {
+                diagnostic.Should().HaveCodeAndSeverity(code, DiagnosticLevel.Error, because, becauseArgs);
+            }
+
+            return new(this);
+        }
+
         public AndConstraint<DiagnosticBuilderAssertions> HaveCodeAndSeverity(string code, DiagnosticLevel level, string because = "", params object[] becauseArgs)
         {
-            Diagnostic diagnostic = GetDiagnosticFromSubject();
+            var diagnostic = GetDiagnosticFromSubject();
 
             using (new AssertionScope())
             {
@@ -36,7 +48,7 @@ public AndConstraint<DiagnosticBuilderAssertions> HaveCodeAndSeverity(string cod
 
         public AndConstraint<DiagnosticBuilderAssertions> HaveMessage(string message, string because = "", params object[] becauseArgs)
         {
-            Diagnostic diagnostic = GetDiagnosticFromSubject();
+            var diagnostic = GetDiagnosticFromSubject();
 
             using (new AssertionScope())
             {
@@ -46,6 +58,18 @@ public AndConstraint<DiagnosticBuilderAssertions> HaveMessage(string message, st
             return new(this);
         }
 
+        public AndConstraint<DiagnosticBuilderAssertions> HaveMessageStartWith(string prefix, string because = "", params object[] becauseArgs)
+        {
+            var diagnostic = GetDiagnosticFromSubject();
+
+            using (new AssertionScope())
+            {
+                diagnostic.Should().HaveMessageStartWith(prefix, because, becauseArgs);
+            }
+
+            return new(this);
+        }
+
         private Diagnostic GetDiagnosticFromSubject() => this.Subject(DiagnosticBuilder.ForDocumentStart());
     }
 }

src/Bicep.Core.UnitTests/Registry/ArtifactDispatcherTests.cs

@@ -83,7 +83,7 @@ public async Task MockRegistries_ModuleLifecycle()
             mock.Setup(x => x.OnRestoreArtifacts(It.IsAny<bool>()))
                 .Returns(Task.CompletedTask);
 
-            ErrorBuilderDelegate? @null = null;
+            DiagnosticBuilderDelegate? @null = null;
             var configuration = BicepTestConstants.BuiltInConfiguration;
 
             var validRefUri = RandomFileUri();
@@ -100,7 +100,7 @@ public async Task MockRegistries_ModuleLifecycle()
 
             var badRefUri = RandomFileUri();
             ArtifactReference? nullRef = null;
-            ErrorBuilderDelegate? badRefError = x => new ErrorDiagnostic(x.TextSpan, "BCPMock", "Bad ref error");
+            DiagnosticBuilderDelegate? badRefError = x => new Diagnostic(x.TextSpan, DiagnosticLevel.Error, "BCPMock", "Bad ref error");
             mock.Setup(m => m.TryParseArtifactReference(ArtifactType.Module, null, "badRef")).Returns(ResultHelper.Create(nullRef, badRefError));
 
             mock.Setup(m => m.IsArtifactRestoreRequired(validRef)).Returns(true);
@@ -113,9 +113,9 @@ public async Task MockRegistries_ModuleLifecycle()
             mock.Setup(m => m.TryGetLocalArtifactEntryPointUri(validRef3)).Returns(ResultHelper.Create(validRef3LocalUri, @null));
 
             mock.Setup(m => m.RestoreArtifacts(It.IsAny<IEnumerable<ArtifactReference>>()))
-                .ReturnsAsync(new Dictionary<ArtifactReference, DiagnosticBuilder.ErrorBuilderDelegate>
+                .ReturnsAsync(new Dictionary<ArtifactReference, DiagnosticBuilder.DiagnosticBuilderDelegate>
                 {
-                    [validRef3] = x => new ErrorDiagnostic(x.TextSpan, "RegFail", "Failed to restore module")
+                    [validRef3] = x => new Diagnostic(x.TextSpan, DiagnosticLevel.Error, "RegFail", "Failed to restore module")
                 });
 
             var dispatcher = CreateDispatcher(BicepTestConstants.ConfigurationManager, fail.Object, mock.Object);
@@ -172,9 +172,9 @@ public async Task GetModuleRestoreStatus_ConfigurationChanges_ReturnsCachedStatu
             var registryMock = StrictMock.Of<IArtifactRegistry>();
             registryMock.SetupGet(x => x.Scheme).Returns("mock");
             registryMock.Setup(x => x.RestoreArtifacts(It.IsAny<IEnumerable<ArtifactReference>>()))
-                .ReturnsAsync(new Dictionary<ArtifactReference, ErrorBuilderDelegate>
+                .ReturnsAsync(new Dictionary<ArtifactReference, DiagnosticBuilderDelegate>
                 {
-                    [badReference] = x => new ErrorDiagnostic(x.TextSpan, "RestoreFailure", "Failed to restore module.")
+                    [badReference] = x => new Diagnostic(x.TextSpan, DiagnosticLevel.Error, "RestoreFailure", "Failed to restore module.")
                 });
             registryMock.Setup(x => x.IsArtifactRestoreRequired(badReference))
                 .Returns(true);

src/Bicep.Core.UnitTests/Semantics/AuxiliaryFileCacheTests.cs

@@ -22,7 +22,7 @@ public void TryReadAsBinaryData_should_return_the_same_cached_result()
 
         var fileResolverMock = StrictMock.Of<IFileResolver>();
         fileResolverMock.Setup(x => x.TryReadAsBinaryData(fileUri, null))
-            .Returns(new ResultWithDiagnostic<BinaryData>(binaryData));
+            .Returns(new ResultWithDiagnosticBuilder<BinaryData>(binaryData));
 
         var sut = new AuxiliaryFileCache(fileResolverMock.Object);
 
@@ -47,7 +47,7 @@ public void TryReadAsBinaryData_caches_negative_results()
 
         var fileResolverMock = StrictMock.Of<IFileResolver>();
         fileResolverMock.Setup(x => x.TryReadAsBinaryData(fileUri, null))
-            .Returns(new ResultWithDiagnostic<BinaryData>(x => x.FilePathInterpolationUnsupported()));
+            .Returns(new ResultWithDiagnosticBuilder<BinaryData>(x => x.FilePathInterpolationUnsupported()));
 
         var sut = new AuxiliaryFileCache(fileResolverMock.Object);
 
@@ -71,7 +71,7 @@ public void ClearEntries_should_clear_cached_results()
 
         var fileResolverMock = StrictMock.Of<IFileResolver>();
         fileResolverMock.Setup(x => x.TryReadAsBinaryData(fileUri, null))
-            .Returns(new ResultWithDiagnostic<BinaryData>(binaryData));
+            .Returns(new ResultWithDiagnosticBuilder<BinaryData>(binaryData));
 
         var sut = new AuxiliaryFileCache(fileResolverMock.Object);
 
@@ -94,7 +94,7 @@ public void PruneInactiveEntries_should_clear_inactive_entries()
 
         var fileResolverMock = StrictMock.Of<IFileResolver>();
         fileResolverMock.Setup(x => x.TryReadAsBinaryData(fileUri, null))
-            .Returns(new ResultWithDiagnostic<BinaryData>(binaryData));
+            .Returns(new ResultWithDiagnosticBuilder<BinaryData>(binaryData));
 
         var sut = new AuxiliaryFileCache(fileResolverMock.Object);
 

src/Bicep.Core.UnitTests/Utils/ResultHelper.cs

@@ -20,7 +20,7 @@ public static Result<TSuccess, TError> Create<TSuccess, TError>(TSuccess? succes
             _ => throw new InvalidOperationException($"{nameof(success)} and {nameof(error)} cannot both be non-null"),
         };
 
-    public static ResultWithDiagnostic<TSuccess> Create<TSuccess>(TSuccess? success, DiagnosticBuilder.ErrorBuilderDelegate? error)
+    public static ResultWithDiagnosticBuilder<TSuccess> Create<TSuccess>(TSuccess? success, DiagnosticBuilder.DiagnosticBuilderDelegate? error)
         where TSuccess : class
         => (success, error) switch
         {

src/Bicep.Core.UnitTests/Utils/TestTypeHelper.cs

@@ -48,7 +48,7 @@ public static IResourceTypeLoader CreateResourceTypeLoaderWithTypes(IEnumerable<
         public static IResourceTypeProviderFactory CreateResourceTypeLoaderFactory(IResourceTypeProvider provider)
         {
             var factory = StrictMock.Of<IResourceTypeProviderFactory>();
-            factory.Setup(m => m.GetResourceTypeProvider(It.IsAny<ArtifactReference?>(), It.IsAny<Uri>(), It.IsAny<bool>())).Returns(new ResultWithDiagnostic<IResourceTypeProvider>(provider));
+            factory.Setup(m => m.GetResourceTypeProvider(It.IsAny<ArtifactReference?>(), It.IsAny<Uri>(), It.IsAny<bool>())).Returns(new ResultWithDiagnosticBuilder<IResourceTypeProvider>(provider));
             factory.Setup(m => m.GetBuiltInAzResourceTypesProvider()).Returns(provider);
             return factory.Object;
         }

src/Bicep.Core/Configuration/ExtensionAliasesConfiguration.cs

@@ -42,7 +42,7 @@ private ExtensionAliasesConfiguration(ExtensionAliases data, Uri? configFileUri)
 
         public ImmutableSortedDictionary<string, OciArtifactExtensionAlias> OciArtifactExtensionAliases => this.Data.OciArtifactExtensionAliases;
 
-        public ResultWithDiagnostic<OciArtifactExtensionAlias> TryGetOciArtifactExtensionAlias(string aliasName)
+        public ResultWithDiagnosticBuilder<OciArtifactExtensionAlias> TryGetOciArtifactExtensionAlias(string aliasName)
         {
             if (!ValidateAliasName(aliasName, out var errorBuilder))
             {
@@ -62,7 +62,7 @@ public ResultWithDiagnostic<OciArtifactExtensionAlias> TryGetOciArtifactExtensio
             return new(alias);
         }
 
-        private static bool ValidateAliasName(string aliasName, [NotNullWhen(false)] out ErrorBuilderDelegate? errorBuilder)
+        private static bool ValidateAliasName(string aliasName, [NotNullWhen(false)] out DiagnosticBuilderDelegate? errorBuilder)
         {
             if (!ExtensionAliasNameRegex().IsMatch(aliasName))
             {

src/Bicep.Core/Configuration/ProvidersConfiguration.cs → src/Bicep.Core/Configuration/ExtensionsConfiguration.cs

@@ -36,7 +36,7 @@ public static ExtensionsConfiguration Bind(JsonElement element)
                 pair => new ExtensionConfigEntry(pair.Value))
         );
 
-    public ResultWithDiagnostic<ExtensionConfigEntry> TryGetExtensionSource(string extensionName)
+    public ResultWithDiagnosticBuilder<ExtensionConfigEntry> TryGetExtensionSource(string extensionName)
     {
         if (!this.Data.TryGetValue(extensionName, out var extensionConfigEntry))
         {

src/Bicep.Core/Configuration/ModuleAliasesConfiguration.cs

@@ -63,7 +63,7 @@ public ImmutableSortedDictionary<string, TemplateSpecModuleAlias> GetTemplateSpe
             return this.Data.TemplateSpecModuleAliases;
         }
 
-        public ResultWithDiagnostic<TemplateSpecModuleAlias> TryGetTemplateSpecModuleAlias(string aliasName)
+        public ResultWithDiagnosticBuilder<TemplateSpecModuleAlias> TryGetTemplateSpecModuleAlias(string aliasName)
         {
             if (!ValidateAliasName(aliasName, out var errorBuilder))
             {
@@ -88,7 +88,7 @@ public ResultWithDiagnostic<TemplateSpecModuleAlias> TryGetTemplateSpecModuleAli
             return new(alias);
         }
 
-        public ResultWithDiagnostic<OciArtifactModuleAlias> TryGetOciArtifactModuleAlias(string aliasName)
+        public ResultWithDiagnosticBuilder<OciArtifactModuleAlias> TryGetOciArtifactModuleAlias(string aliasName)
         {
             if (!ValidateAliasName(aliasName, out var errorBuilder))
             {
@@ -108,7 +108,7 @@ public ResultWithDiagnostic<OciArtifactModuleAlias> TryGetOciArtifactModuleAlias
             return new(alias);
         }
 
-        private static bool ValidateAliasName(string aliasName, [NotNullWhen(false)] out ErrorBuilderDelegate? errorBuilder)
+        private static bool ValidateAliasName(string aliasName, [NotNullWhen(false)] out DiagnosticBuilderDelegate? errorBuilder)
         {
             if (!ModuleAliasNameRegex().IsMatch(aliasName))
             {

src/Bicep.Core/Diagnostics/Diagnostic.cs

@@ -40,6 +40,5 @@ public Diagnostic(
         public string Message { get; }
 
         public Uri? Uri { get; }
-
     }
 }