The Below function is falsely stated to be returning same value. The method here has two outputs.
Code Example:
boolean foo(int a)
{
switch(a)
{
case 1:
throw new Exception("trial");
case 2:
return true;
default:
return true;
}
}
Hi,
Welcome to the community!
What language is this?
Ann
Oh missed adding that. The source language is Java
Hello @vishal1991k,
Two things:
<yourSonarQubeInstanceURl>/deploy/plugins/index.txt. For instance, on my local machine, it would be: http://localhost:9000/deploy/plugins/index.txt
Cheers,
Michael
Hi Michael,
I could find the below from the sonar deployment.
Can you provide the rule key of the rule raising a false positive? Is it java:S3516 (also known as squid:S3516)?
–> yes It is java:S3516
Can you please specify the version java analyzer embedded in your SonarQube 7.9 version. I can not reproduce the issue using your reproducer and the latest released version. In order to get the version of the java analyzer (sonar-java-plugin), you can navigate to this address: <yourSonarQubeInstanceURl>/deploy/plugins/index.txt. For instance, on my local machine, it would be: http://localhost:9000/deploy/plugins/index.txt
–> sonar-java-plugin-6.1.0.20866.jar
Okay, Thank you for the update.
First, I would recommend to update your sonar-java-plugin to its latest released version compatible with LTS (version 6.3.1). It will improve greatly performance, and fix numerous issues.
Secondly, can you please double-check if your reproducer is indeed raising the FP you are mentioning when analyzed? Currently, it does not compile (missing the throws declaration for instance) and I can not reproduce the FP on my side when completing the code. I feel that there might be something missing here (your example might be over-simplified?). The best way would be to put in place a small maven project, with a single class containing the issue reproducing the issue, and upload it on GitHub.
Thanks,
Michael
Hi Michael,
Here is the code as you asked for
/* package whatever; // don't place package name! */
import java.util.*;
import java.lang.*;
import java.io.*;
/* Name of the class has to be "Main" only if the class is public. */
class Ideone
{
public static void main (String[] args) throws java.lang.Exception
{
// your code goes here
FP fp = new FP();
fp.foo(4);
fp.foo(1);
}
}
class FP
{
public boolean foo(int a) throws Exception
{
switch(a)
{
case 1:
throw new Exception("trial");
case 2:
return true;
default:
return true;
}
}
}
Hello @vishal1991k,
I’m a little bit confused, the code you posted still does not raise an issue on my side either.
By playing a bit with it, I changed throw new Exception("trial"); in your code by a runtime exception, and it does raise an issue. So I believe we already have a situation needing clarification. I created a ticket (SONARJAVA-3473) for it.
Now, the remaining question is to clarify if we are facing a false-negative (your example does not raise an issue while it should) or a false-positive (my example is raising an issue while it should not).
My feeling is that it’s a false-negative, the method may have “two outputs” as you said, but the rule still apply as returns are still invariant.
Feel free to continue the discussion if I’m missing something.
Hi @Quentin,
This kind of switch is a classic case of handling behaviour when a code fails due to some errors. The example here shows that for some known errors we want the behaviour to be of throwing an exception while for some other cases we just want to return false. So i feel it to be a false positive. Incase this doesnt feel the right way to handle errors then this can be a false negative(here will need to re think a different strategy). Let me know if that’s the case.
And yes thanks for the new repro(that is exactly the case faced by me). I should have run a sonar lint before updating it here.
while for some other cases we just want to return false.
Out of curiosity, why not removing case 2:? Or use fallthrough if you want to be explicit?
I know that you simplified the example, but I fail to see a case where this could be a legitimate use case.
yes a fall-through would make it look cleaner if it is the only action being taken in the case.But if each case has some task that is being performed extra like say logging the error varies case by case.
It would be need some rework but yes it can be done. Thanks for the inputs 
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.