New Rule Idea: Null forgiving operator and conditional access should not be gratuitous/detrimental

[gregory-paidis-sonarsource]

There are two mechanisms to either check for null or ignore a null check:

• Conditional access via (?)
• Null-forgiving operator (!)

There are some cases that one of them might be either redundant, or actually making the program crash every time:

string neverNull = "not null";
_ = neverNull?.ToString(); // Noncompliant: conditional access is redundant, always valid
_ = neverNull!.ToString(); // Noncompliant: null-forgiving is redundant, no reason to check

string alwaysNull = null;
_ = alwaysNull?.ToString(); // always evaluates to null. Most likely we should not raise here, as it prevents a NRE 
_ = alwaysNull!.ToString(); // Noncompliant: always NRE, operator is detrimental

It might be two rules as well, one for null-forgiving ! and one for conditional access ?.

[pavel-mikula-sonarsource]

S2583 and S2589 might take care of the ? part after MMF-2563

[luizfbicalho]

Thanks for the issue

I think that one problem when implementing nullable reference types, is that after a ?. in a non nullable code

for example this code

public string? GetString()
{
    return DateTime.Now.Seconds>10?"Bigger":null;
}

then After that your code in the example

string? neverNull = GetString();
_ = neverNull?.ToString(); // This is OK
_ = neverNull!.ToString(); // This shoudn't be ok, I think that should validate and not use this

But the problem is that after each code, the nullable reference type misses the nullable information

in my example, the fact that I used an ?. n the line

var y = x?.ToString();

then after that x becomes nullable

and the same happens with the exclamation

so this problem is bad because the null reference type information flows through the code after one misuse

[luizfbicalho]

Same issue is thacked in dotnet Roslyn
dotnet/roslyn#68267

[luizfbicalho]

Any information on this rule?

[luizfbicalho]

I created a roslyn analyzer to implement this, but its not working as expected, but if anyone is interested in add some thoughts https://github.com/luizfbicalho/NullableAnalyzer

[mary-georgiou-sonarsource]

After considering this rule we decided to specify the following:
Conditional access should not be gratuitous.

string s = Test(true);
_ = s?.ToString();  // Noncompliant, you should remove ? and if you want to suppress a warning use !

string Test(bool condition) => condition ? "Hello": null;

and add a new rule idea
Don't use ! if you are in non-nullable context

[mary-georgiou-sonarsource]

One more update:
We realized that the following rule:

After considering this rule we decided to specify the following: Conditional access should not be gratuitous.

string s = Test(true);
_ = s?.ToString();  // Noncompliant, you should remove ? and if you want to suppress a warning use !

string Test(bool condition) => condition ? "Hello": null;

I overlapping with S2583/S2589 so we decided to not proceed with its implementation.