public int Compare(... left, ... right)
{
string leftName = ...;
string rightName = ...;
if (string.Equals(leftName, rightName))
return 0;
if (leftName == null)
{
ERROR: 'rightName' is null on at least one execution path.
if (rightName.EndsWith("module.js"))
To me we are in one of two cases here:
SonarQube simply doesn’t know, but then it shouldn’t report it with the wording it does, instead it should be a warning with “rightName may be null”…
SonarQube think it knows that rightName can be null but actually gets that completely wrong.
IF leftName is null in this block, rightName cannot be null, otherwise the “string.Equals” would have returned from the method, hence we KNOW that rightName holds a value in this context.
I tried to reproduce with the latest SonarCsharp analyzer and could not. This is my reproducer attempt:
class TestCommunityFP
{
public int Compare(string leftName, string rightName)
{
if (string.Equals(leftName, rightName))
return 0;
if (leftName == null)
{
if (rightName.EndsWith("module.js")) // ok
{
return 1;
}
}
return 0;
}
}
The scanner downloads the plugin (which is used to analyze the code) from your SonarQube instance. The plugin version can be checked either in the scanning logs for the Begin step, or in your SonarQube instance.
What do you mean you ignored it? Did you mark it as FP or Won't fix in SQ, and now it appears again as a fresh problem?
Sorry that I have taken a long time to get back to you on this.
We have migrated from a local - per project - deployment of SonarQube (pilot) to running on a company enterprise server.
Because of that the any previous marking of Won’t fix or False positive is obviously gone. (We just started over on the new server)
This problem still appears for us with SonarCSharp v8.6.1, SQ 7.9.2 (LTS). We are not using SonarLint at this point.
Is there any “Cloud hosted” instances of SonarQube for Open Source, then perhaps I could try and see if I could reproduce it in such an environment, which I could then hand directly of to you (environment, repo and all).
I will just add the full code here, with some comments about the logical process, because maybe simplifying it down is why Sonar doesn’t stumble:
public int Compare(BundleFile left, BundleFile right)
{
string leftName = left?.VirtualFile.VirtualPath;
string rightName = right?.VirtualFile.VirtualPath;
if (string.Equals(leftName, rightName))
return 0;
if (leftName == null)
{
// FACT: rightName is not null, otherwise it would have returned in the first if block.
// ERROR: 'rightName' is null on at least one execution path.
if (rightName.EndsWith("module.js"))
return 1;
if (rightName.Contains("config."))
return -1;
return 0;
}
if (rightName == null)
{
// FACT: leftName is not null, otherwise it would have returned in the first if block.
if (leftName.EndsWith("module.js"))
return -1;
if (leftName.Contains("config."))
return 1;
return 0;
}
// FACT: rightName is not null, leftName is not null, otherwise it would have returned in one of the if blocks above.
if (leftName.EndsWith("module.js") && !rightName.EndsWith("module.js"))
return -1;
if (leftName.Contains("config.") && !rightName.Contains("config."))
return 1;
if (rightName.EndsWith("module.js") && !leftName.EndsWith("module.js"))
return 1;
if (rightName.Contains("config.") && !leftName.Contains("config."))
return -1;
return 0;
}
We have another case of the same “Error”, they are not immediately similar, the only thing that did come to mind was that both cases had a use of the Null-conditional operator, I don’t know if that may throw SonarQube off:
public override bool Matches(JToken token, IJsonValidationContext context)
{
switch (token?.Type)
{
case JTokenType.Array:
// ERROR: 'token' is null on at least one execution path.
// FACT: token is never null here, IF the token was null, token?.Type would return null which would lead to the last case above the default case.
return !token.HasValues;
case JTokenType.Integer:
case JTokenType.Float:
return 0 == (int) token;
case JTokenType.String:
return bool.FalseString.Equals((string) token, StringComparison.OrdinalIgnoreCase);
case JTokenType.Boolean:
return !(bool)token;
case JTokenType.Null:
case JTokenType.Undefined:
case null:
return true;
default:
return false;
}
}
I could not come with a reproducer. This is what I tried:
public enum MyEnum
{
ONE,
TWO,
THREE,
FOUR,
FIVE
}
public class ValueHolder
{
public string Value { get; }
public MyEnum MyEnum { get; }
}
// https://github.com/SonarSource/sonar-dotnet/issues/3416
public class ReproCommunityIssue3416
{
public int Compare(ValueHolder left, ValueHolder right)
{
string leftName = left?.Value;
string rightName = right?.Value;
if (string.Equals(leftName, rightName))
// this will return if both are NULL or if they have equal non-null values
return 0;
// at this point, leftName can be NULL if rightName is not NULL
if (leftName == null)
{
// rightName is not null
if (rightName.EndsWith("foo")) // Noncompliant FP
return 1;
return 0;
}
return 0;
}
public string Foo(ValueHolder valueHolder)
{
switch (valueHolder?.MyEnum)
{
case MyEnum.ONE:
return valueHolder.Value;
case MyEnum.TWO:
case MyEnum.THREE:
return valueHolder.Value;
case MyEnum.FOUR:
return valueHolder.Value;
case MyEnum.FIVE:
case null:
return valueHolder.Value; // Noncompliant Ok
default:
return string.Empty;
}
}
}