Using null conditional in assertion should be reported as code smell

[krajek]

In main FA repository, there was an issue reported fluentassertions/fluentassertions#569.

To keep it short, the user used ?. operator:

int? val = null;
val?.Should().Be(400);

instead of regular

val.Should().Be(400);

Do you think it is possible to detect such scenario and suggest code fix?

[jnyrup]

That is not only a code smell for Nullable but for all reference types where null conditional operator is used before calling Should()

In this example Should() is never called, as the null conditional operator short-circuits the expression.

string str = null;
str?.Should().NotBeNull();

There is SyntaxKind.ConditionalAccessExpression to check for usages of a?.b and a?[1].

[Meir017]

I have a working sample for this.

class ConditionalAccessExpressionVisitor : CSharpSyntaxWalker
{
    public bool CodeSmells { get; private set; }
    public Location ConditionalAccess { get; private set; }

    public override void VisitConditionalAccessExpression(ConditionalAccessExpressionSyntax node)
    {
        if (CodeSmells) return;

        CodeSmells = node.WhenNotNull is InvocationExpressionSyntax invocation
            && invocation.Expression is MemberAccessExpressionSyntax memberAccess && memberAccess.IsKind(SyntaxKind.SimpleMemberAccessExpression)
            && memberAccess.Expression is InvocationExpressionSyntax shouldInvocation
            && shouldInvocation.Expression is MemberBindingExpressionSyntax memberBinding
            && memberBinding.Name.Identifier.ValueText == "Should";
        if (CodeSmells)
        {
            ConditionalAccess = node.GetLocation();
        }
    }
}