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.

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

Refactor: Use single Diagnostic class to represent all core diagnostics #14875

Merged
merged 2 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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.Cli/Commands/FormatCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/Bicep.Core.IntegrationTests/ModuleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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!");
}

Expand Down
28 changes: 26 additions & 2 deletions src/Bicep.Core.UnitTests/Assertions/DiagnosticBuilderAssertions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand All @@ -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())
{
Expand All @@ -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());
}
}
63 changes: 0 additions & 63 deletions src/Bicep.Core.UnitTests/Assertions/ErrorBuilderAssertions.cs

This file was deleted.

12 changes: 6 additions & 6 deletions src/Bicep.Core.UnitTests/Registry/ArtifactDispatcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/Bicep.Core.UnitTests/Semantics/AuxiliaryFileCacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Utils/ResultHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core.UnitTests/Utils/TestTypeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Configuration/ExtensionAliasesConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
6 changes: 3 additions & 3 deletions src/Bicep.Core/Configuration/ModuleAliasesConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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))
{
Expand All @@ -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))
{
Expand Down
1 change: 0 additions & 1 deletion src/Bicep.Core/Diagnostics/Diagnostic.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,5 @@ public Diagnostic(
public string Message { get; }

public Uri? Uri { get; }

}
}
Loading
Loading