Fix S3236 FP: CallerArgumentExpressionAttribute should be overridable when using Nullable<>

[rjgotten]

Description

S3236 is raised in cases where the user passes a caller expression explicitly, and the rule advises against doing this.
However, there are cases wrt the expanded set of 'throw helpers' introduced by Microsoft in later .NET versions where passing a caller expression name explicitly is exactly the right thing to do, such as when having to check for negative value on nullable numeric properties.

Cases like ArgumentOutOfRangeException.ThrowIfNegative when used with nullables must use some form of guarded access to the underlying value to pass to the throw helper, because Nullable<> does not implement the requisite interface and the throw helper is not overloaded to understand Nullable<INumberBase>.

But in this case, the actual parameter that should be reported as in error, should still be the actual nullable numeric property - and not whatever inbetween value is used.

Repro steps

public class Test
{
  private readonly int _amount;
  public int? Amount
  {
     get => _amount;
     init
     {
       if (value is int val)
         // The exception actually *SHOULD* be reported for 'value', not for 'val' or for 'value.Value'
         ArgumentOutOfRangeException.ThrowIfNegative(val, nameof(value));
         
       _amount = value;
     }
  }

Expected behavior

S3236 is not raised in situations that could be identified as explicitly passing the related parameter name to a throw helper.

Actual behavior

S3236 is raised in situations that could be identified as explicitly passing the related parameter name to a throw helper.

Known workarounds

None. Suppress the rule.

Related information

• C#/VB.NET Plugins version -- whatever ships with SonarLint 8.1.0.95039
• Visual Studio version -- 17.10.5
• MSBuild / dotnet version -- dotnet SDK 8.0.303
• SonarScanner for .NET version (if used) -- N/A
• Operating System -- Windows 11

[sebastien-marichal]

Hello @rjgotten,

This is indeed quite inconvenient; I confirm this to be a false positive.

I think it is difficult to decide whether it makes sense or not to override caller information arguments.
We would need to add a very specific exception for this scenario:

• the argument is marked with CallerArgumentExpression attribute
• the tested variable has been created from a Nullable<>

I am not sure if there could be scenarios where it would produce false negatives.