Merge NSB0018 and NSB0004 saga mapping diagnostics with level Error

[Copilot]

The old saga mapping API (ConfigureMapping, ToSaga) was obsoleted in #7510 with TreatAsErrorFromVersion = "10". The analyzer diagnostics NSB0018 and NSB0004 remained at Info and Warning severity respectively, inconsistent with the obsolete API being a compile error.

Changes

NSB0018 (SagaMappingExpressionCanBeRewritten)

Diagnostic is removed, as NSB0004 will cover both cases with DiagnosticSeverity.Error.

NSB0004 (SagaMappingExpressionCanBeSimplified)

• Severity: Changed from DiagnosticSeverity.Warning to DiagnosticSeverity.Error
• Messaging: Updated from "can be simplified" to "must be rewritten" and added "Use the code fix to transition to the new syntax"

Context

Both diagnostics detect the use of the old saga mapping API:

• NSB0004 is raised when there are multiple old-style mappings (e.g., multiple ConfigureMapping().ToSaga() calls), which was previously a Warning so that the user would simplify them with the new syntax.
• NSB0018 was raised when there was a single old-style mapping. The severity was Info because, when introduced, rewriting the syntax was nice-to-have but did not provide more terse, deduplicated code.

Since the old API is now a compile-time error, the remaining analyzer is now an error, but the code fix can still be used.

Before/After

// Old diagnostics
mapper.ConfigureMapping<Msg1>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
// Info: This saga mapping expression can be rewritten...

mapper.ConfigureMapping<Msg1>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
mapper.ConfigureMapping<Msg2>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
// Warning: The saga mapping contains multiple .ToSaga(…) expressions which can be simplified...

// New diagnostics
mapper.ConfigureMapping<Msg1>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
// Error: This saga mapping expression must be rewritten using mapper.MapSaga(…).ToMessage<T>(…) syntax. Use the code fix to transition to the new syntax.

mapper.ConfigureMapping<Msg1>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
mapper.ConfigureMapping<Msg2>(msg => msg.CorrId).ToSaga(saga => saga.CorrId);
// Error: This saga mapping expression must be rewritten using mapper.MapSaga(…).ToMessage<T>(…) syntax. Use the code fix to transition to the new syntax.

Original prompt

Should this be now error? https://github.com/Particular/NServiceBus/blob/master/src/NServiceBus.Core.Analyzer/Sagas/SagaDiagnostics.cs#L138 should probably be an error instead of a warning because as part of #7510 we obsoleted the old saga API. if so, might as well change “can be rewritten” (like it’s optional) to something that says to use the code fix to transition to the new syntax

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[danielmarbach]

@copilot I found out from the documentation that we also have NSB0004.

The diagnostics NSB0004 and NSB0018 are the same but with different severity in different contexts. There is no duplication when only one mapping expression exists, so NSB0018 is presented at level Info. When two or more mapping expressions exist, duplication is present, so NSB0004 is presented as a Warning.

To me it sounds like NSB0004 should be combined into NSB0018 or with NSB0018 now being an error, NSB0004 might need to go.

[Copilot]

@copilot I found out from the documentation that we also have NSB0004.

The diagnostics NSB0004 and NSB0018 are the same but with different severity in different contexts. There is no duplication...

You're correct! Both NSB0004 and NSB0018 detect the old saga mapping API - NSB0004 for multiple old-style mappings and NSB0018 for single old-style mappings. I've updated NSB0004 to also be an Error (from Warning) with the same "must be rewritten" messaging, since both are detecting obsolete API usage. Changes in commit c12e063.

[DavidBoike]

Indeed the only difference between the diagnostics was to have separate Info/Warn for the two cases.

In the past:

• A single mapping expression changed to MapSaga would not really "save you any code", it would just reorder the calls. That's not super revolutionary, so that suggestion was just an INFO.
• A double mapping expression would definitely repeat the ToSaga part, which is why that was a WARN.

Everything else was the same.

While we don't HAVE to merge the diagnostics, since we're here and have the context rebuilt, I think it's a good idea to do so to prevent future instances of "Why are these separate diagnostics, that makes no sense."

• I raised a sub-PR Merge saga diagnostics now that they're both error #7579 to merge them together.

[DavidBoike]

• Approved as-is but if you like Merge saga diagnostics now that they're both error #7579 feel free to merge that and consider this still approved by me.