Skip to content
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
35 changes: 22 additions & 13 deletions DiagnosableExceptions.GenDoc/SolutionErrorDocumentationGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,19 @@ private static IReadOnlyList<ProjectInfo> FilterProjects(IReadOnlyList<ProjectIn
List<ProjectInfo> included = new();

foreach (ProjectInfo project in projects) {
bool optedIn = TryReadOptInFromProjectFile(project.ProjectPath, options.OptInPropertyName);
if (optedIn) {
bool? optedIn = TryReadOptInFromProjectFile(project.ProjectPath, options.OptInPropertyName);

if (optedIn == true) {
included.Add(project);

continue;
}

// An explicit opt-out (property present and set to a falsy value) is always honored, even when
// IncludeProjectsWithoutOptIn is true.
if (optedIn == false) { continue; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Evaluate opt-out properties before skipping projects

When IncludeProjectsWithoutOptIn is true, this branch now turns any falsy occurrence of the opt-in property into a hard skip, but TryReadOptInFromProjectFile only scans the raw XML and does not evaluate MSBuild conditions or the selected options.Configuration. A project that has GenerateErrorDocumentation=false in a Release-only property group, for example, will be omitted during Debug solution-wide generation instead of falling back to the global include, so its errors silently disappear from the generated catalog. Please evaluate the property through MSBuild for the active configuration before treating false as an explicit opt-out.

Useful? React with 👍 / 👎.


// The opt-in property is absent: fall back to the global policy.
if (options.IncludeProjectsWithoutOptIn) {
included.Add(project);
}
Expand All @@ -114,10 +120,10 @@ private static IReadOnlyList<ProjectInfo> FilterProjects(IReadOnlyList<ProjectIn
return included;
}

private static bool TryReadOptInFromProjectFile(string projectPath, string optInPropertyName) {
private static bool? TryReadOptInFromProjectFile(string projectPath, string optInPropertyName) {
try {
ProjectRootElement? root = ProjectRootElement.Open(projectPath);
if (root is null) { return false; }
if (root is null) { return null; }

foreach (ProjectPropertyGroupElement group in root.PropertyGroups) {
foreach (ProjectPropertyElement prop in group.Properties) {
Expand All @@ -127,19 +133,17 @@ private static bool TryReadOptInFromProjectFile(string projectPath, string optIn
}
}

return false;
return null;
} catch {
return false;
return null;
}
}

private static string? ResolveTargetPath(string projectPath, SolutionGenerationOptions options) {
// We purposely avoid MSBuild object model evaluation here to keep dependencies minimal and robust.
// We read from the project file:
// - TargetFramework or TargetFrameworks
// Then compute the expected TargetPath through standard output conventions is risky.
// So we rely on MSBuild property "TargetPath" via "dotnet msbuild -getProperty:TargetPath".
// This stays SDK-driven and avoids referencing Microsoft.Build assemblies in this lib.
// We resolve TargetPath by invoking the SDK ("dotnet msbuild -getProperty:TargetPath") rather than evaluating
// the project through the MSBuild object model. Computing the output path ourselves from output conventions
// would be fragile, and a full evaluation would require resolving the project's full import graph; delegating
// to the installed SDK keeps the result authoritative across project styles.
string tfm = ResolveTargetFrameworkMoniker(projectPath, options);

return DotNetGetProperty(projectPath, options.Configuration, tfm, "TargetPath", options.Logger);
Expand Down Expand Up @@ -329,7 +333,12 @@ private static IEnumerable<ErrorDocumentation> ExtractFromAssemblies(IReadOnlyLi
results.AddRange(docs);
}

return results.OrderBy(x => x.Code, StringComparer.OrdinalIgnoreCase);
// Deduplicate across assemblies as well: the same error Code declared in two assemblies must collapse to a
// single catalog entry, mirroring the per-assembly deduplication performed by the reader.
return results
.GroupBy(x => x.Code, StringComparer.OrdinalIgnoreCase)
.Select(g => g.First())
.OrderBy(x => x.Code, StringComparer.OrdinalIgnoreCase);
}

#endregion
Expand Down
4 changes: 2 additions & 2 deletions DiagnosableExceptions.GenDoc/SolutionGenerationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public sealed class SolutionGenerationOptions {
public string OptInPropertyName { get; init; } = "GenerateErrorDocumentation";

/// <summary>
/// Additional arguments passed to "dotnet build".
/// Example: "--no-restore"
/// Additional arguments appended verbatim to the "dotnet build" command line (e.g. "--no-restore").
/// Defaults to "--nologo".
/// </summary>
public string? DotNetBuildAdditionalArguments { get; init; } = "--nologo";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ namespace DiagnosableExceptions.UnitTests {

public class AssemblyErrorDocumentationReaderTests {

[Fact]
public void Test1() {
[Fact(DisplayName = "The reader extracts the documented errors of an assembly, keyed by their error code.")]
public void TheReaderExtractsTheDocumentedErrorsOfAnAssembly() {
// Setup
Assembly assembly = Assembly.GetAssembly(typeof(Temperature))!;

Expand All @@ -25,10 +25,13 @@ public void Test1() {
ErrorDocumentation[] errorDocumentations = documentation.ToArray();

Check.That(errorDocumentations).CountIs(4);
ErrorDocumentation amountCurrencyMismatch = errorDocumentations[0];
ErrorDocumentation bankTransactionFileDateOutOfStatementPeriod = errorDocumentations[1];
ErrorDocumentation bankTransactionFileStatementTotalAmountMismatch = errorDocumentations[2];
ErrorDocumentation temperatureBelowAbsoluteZero = errorDocumentations[3];

Dictionary<string, ErrorDocumentation> byCode = errorDocumentations.ToDictionary(doc => doc.Code!, StringComparer.Ordinal);

ErrorDocumentation amountCurrencyMismatch = byCode["AMOUNT_CURRENCY_MISMATCH"];
ErrorDocumentation bankTransactionFileDateOutOfStatementPeriod = byCode["BANK_TRANSACTION_FILE_DATE_OUT_OF_STATEMENT_PERIOD"];
ErrorDocumentation bankTransactionFileStatementTotalAmountMismatch = byCode["BANK_TRANSACTION_FILE_STATEMENT_TOTAL_AMOUNT_MISMATCH"];
ErrorDocumentation temperatureBelowAbsoluteZero = byCode["TEMPERATURE_BELOW_ABSOLUTE_ZERO"];

// ------------------------------------------------------------------
// AMOUNT_CURRENCY_MISMATCH
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public void AnErrorDocumentationBuilderRejectsANullExample() {
// Exercise & verify
Check.ThatCode(() => builder.WithExamples(factory))
.Throws<ErrorDocumentationException>()
.WithMessage("Example factory at index 0 returned null. Factories must return a valid exception instance.");
.WithMessage("Example factory at index 0 returned null. Factories must return a valid error instance.");
}

[Fact(DisplayName = "An error documentation builder rejects inconsistent error codes across examples.")]
Expand All @@ -223,7 +223,7 @@ public void AnErrorDocumentationBuilderRejectsInconsistentErrorCodesAcrossExampl
// Exercise & verify
Check.ThatCode(() => builder.WithExamples(first, second))
.Throws<ErrorDocumentationException>()
.WithMessage("All example factories must produce exceptions with the same ErrorCode. Example at index 1 produced a different ErrorCode. Expected 'CODE_A', but received 'CODE_B'.");
.WithMessage("All example factories must produce errors with the same ErrorCode. Example at index 1 produced a different ErrorCode. Expected 'CODE_A', but received 'CODE_B'.");
}

[Fact(DisplayName = "An error documentation builder uses the examples error code as documentation code.")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ namespace DiagnosableExceptions.UnitTests;

[Collection("SmartEnumSideEffects")]
[TestSubject(typeof(Error))]
public sealed class DiagnosableExceptionTests : IDisposable {
public sealed class ErrorTests : IDisposable {

#region Constructors declarations

public DiagnosableExceptionTests() {
public ErrorTests() {
ErrorContextKey.ResetForTests();
ErrorCode.ResetForTests();
}
Expand All @@ -29,7 +29,7 @@ public void Dispose() {
ErrorCode.ResetForTests();
}

[Fact(DisplayName = "A diagnosable exception has a unique instance identifier.")]
[Fact(DisplayName = "An error has a unique instance identifier.")]
public void ADiagnosableExceptionHasAUniqueInstanceIdentifier() {
// Setup
ErrorCode anyErrorCode = ErrorCodeFactory.CreateAny();
Expand All @@ -45,7 +45,7 @@ public void ADiagnosableExceptionHasAUniqueInstanceIdentifier() {
Check.That(firstError.InstanceId).IsNotEqualTo(secondError.InstanceId);
}

[Fact(DisplayName = "A diagnosable exception captures its occurrence time in UTC.")]
[Fact(DisplayName = "An error captures its occurrence time in UTC.")]
public void ADiagnosableExceptionCapturesItsOccurrenceTimeInUtc() {
// Setup
ErrorCode anyErrorCode = ErrorCodeFactory.CreateAny();
Expand All @@ -67,7 +67,7 @@ public void ADiagnosableExceptionCapturesItsOccurrenceTimeInUtc() {
Check.That(error.OccurredAt <= after).IsTrue();
}

[Fact(DisplayName = "A diagnosable exception preserves the provided error code.")]
[Fact(DisplayName = "An error preserves the provided error code.")]
public void ADiagnosableExceptionPreservesTheProvidedErrorCode() {
// Setup
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -80,7 +80,7 @@ public void ADiagnosableExceptionPreservesTheProvidedErrorCode() {
Check.That(error.Code).IsEqualTo(temperatureBelowAbsoluteZero);
}

[Fact(DisplayName = "A diagnosable exception preserves the provided short message.")]
[Fact(DisplayName = "An error preserves the provided short message.")]
public void ADiagnosableExceptionPreservesTheProvidedShortMessage() {
// Exercise
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -91,7 +91,7 @@ public void ADiagnosableExceptionPreservesTheProvidedShortMessage() {
Check.That(error.ShortMessage).IsEqualTo("short");
}

[Fact(DisplayName = "A diagnosable exception has an empty context when no context is provided.")]
[Fact(DisplayName = "An error has an empty context when no context is provided.")]
public void ADiagnosableExceptionHasAnEmptyContextWhenNoContextIsProvided() {
// Exercise
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -104,7 +104,7 @@ public void ADiagnosableExceptionHasAnEmptyContextWhenNoContextIsProvided() {
Check.That(error.Context.Values).CountIs(0);
}

[Fact(DisplayName = "A diagnosable exception includes the provided context entries.")]
[Fact(DisplayName = "An error includes the provided context entries.")]
public void ADiagnosableExceptionIncludesTheProvidedContextEntries() {
// Setup
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -123,7 +123,7 @@ public void ADiagnosableExceptionIncludesTheProvidedContextEntries() {
Check.That(value).IsEqualTo("u-123");
}

[Fact(DisplayName = "A diagnosable exception has no inner exceptions by default.")]
[Fact(DisplayName = "An error has no inner errors by default.")]
public void ADiagnosableExceptionHasNoInnerExceptionsByDefault() {
// Exercise
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -135,7 +135,7 @@ public void ADiagnosableExceptionHasNoInnerExceptionsByDefault() {
Check.That(error.InnerErrors).CountIs(0);
}

[Fact(DisplayName = "A diagnosable exception preserves a single inner exception.")]
[Fact(DisplayName = "An error preserves a single inner error.")]
public void ADiagnosableExceptionPreservesASingleInnerException() {
// Setup
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -150,7 +150,7 @@ public void ADiagnosableExceptionPreservesASingleInnerException() {
Check.That(rootError.InnerErrors[0]).IsSameReferenceAs(innerError);
}

[Fact(DisplayName = "A diagnosable exception preserves multiple inner exceptions.")]
[Fact(DisplayName = "An error preserves multiple inner errors.")]
public void ADiagnosableExceptionPreservesMultipleInnerExceptions() {
// Setup
string anyErrorMessage = ErrorMessageFactory.CreateAnyMessage();
Expand All @@ -170,7 +170,7 @@ public void ADiagnosableExceptionPreservesMultipleInnerExceptions() {
Check.That(rootError.InnerErrors[1]).IsSameReferenceAs(secondInnerError);
}

[Fact(DisplayName = "A diagnosable exception can be created without inner exceptions even when a null collection is provided.")]
[Fact(DisplayName = "An error can be created without inner errors even when a null collection is provided.")]
public void ADiagnosableExceptionCanBeCreatedWithoutInnerExceptionsEvenWhenANullCollectionIsProvided() {
// Exercise
ErrorCode anyErrorCode = ErrorCodeFactory.CreateAny();
Expand All @@ -181,7 +181,7 @@ public void ADiagnosableExceptionCanBeCreatedWithoutInnerExceptionsEvenWhenANull
Check.That(error.InnerErrors).CountIs(0);
}

[Fact(DisplayName = "A diagnosable exception created with a null inner exception has no inner exceptions.")]
[Fact(DisplayName = "An error created with a null inner error has no inner errors.")]
public void ADiagnosableExceptionCreatedWithANullInnerExceptionHasNoInnerExceptions() {
// Exercise
ErrorCode anyErrorCode = ErrorCodeFactory.CreateAny();
Expand Down
103 changes: 0 additions & 103 deletions DiagnosableExceptions.UnitTests/InfrastructureExceptionTests.cs

This file was deleted.

Loading