We are building endpoints using NServiceBus 9.0.3 and .NET8 with the new C# nullable behavior enabled and run into a Code Analysis conflict.
We use SonarCloud to analyse our code during builds via Azure DevOps pipelines and have TreatWarningsAsError set true by policy. That setup yields S3900 errors whenever we do not check method parameters for nullable values before using them.
Initial code:
protected override void ConfigureHowToFindSaga(SagaPropertyMapper<OrderSagaData> mapper)
{
// Fails on SonarCloud S3900 for not checking mapper parameter for null values before this call
mapper.MapSaga(saga => saga.OrderId)
.ToMessage<StartOrder>(message => message.OrderId)
.ToMessage<CompleteOrder>(message => message.OrderId);
}
The Saga’s override ConfigureHowToFindSaga(SagaPropertyMapper<…> mapper) method does not allow to make the mapper parameter nullable by adding a ? to its type to avoid the required null check. This also makes no sense as a fix because it would allow calls with a null value for mapper, resulting in no message - saga state mapping.
So to keep SonarCloud happy we added an ArgumentNullException.ThrowIfNull(mapper) line before the mapping logic, like this:
protected override void ConfigureHowToFindSaga(SagaPropertyMapper<OrderSagaData> mapper)
{
// Fails on NSB0003 for having logic other than mapping
ArgumentNullException.ThrowIfNull(mapper);
mapper.MapSaga(saga => saga.OrderId)
.ToMessage<StartOrder>(message => message.OrderId)
.ToMessage<CompleteOrder>(message => message.OrderId);
}
But then we get a NSB0003 error from the NServiceBus code analysis package
NSB0003 means you cannot add any logic other than mapping code to map message properties to saga state properties.
The ArgurmentNullException.ThrowIfNull(mapper) line gets in the way of that check.
We tried to suppress both kind of warnings without any success, either using
- inline #pragma warning disable statements in the saga class
- a .globalconfig file containing the breaking warning identifiers with severity level set to none
- a Directory.Build.props file containing the breaking warning identifiers with severity level set to none
That’s because SonarCloud sees the #pragma warning disable also as a problem.
Ignoring S3900 warnings via rule files is a problem as it ignores them also in other situations.
We could of course mark each SonarCloud S3900 warning regarding such Saga methods as false positive, but that’s a lot of hassle each time.
How can we solve this without altering our build policies to lower standards?
We wonder why NServiceBus checks the ConfigureHowToFindSaga() code for anything else than mapping logic. It would be quite stupid to write logic that avoids the mapping setup, and besides that we are convinced that our visual code reviews on Pull Requests would catch any one doing that.
To be able to build our code we now set the flags CodeAnalysisTreatWarningsAsErrors and MsBuildTreatWarningsAsErrors to false as a work around.
This will avoid breaking the build but might result in builds containing warnings we should have fixed.