Call "Optional#isPresent()" before accessing the value -> False Positive?

[tobias.v...@nepatec.de]

I have a similar problem like discussed here:
https://groups.google.com/forum/#!topic/sonarqube/A3V23ywJKg0

"Call "Optional#isPresent()" before accessing the value."

I added an isPresent( ) method as described in the sonar "compliant solution" suggestion, however sonar still complains (thus the issue text should also be improved...):

if(sourceunit.getPackageDeclaration().isPresent()){
    sourcePackage = sourceunit.getPackageDeclaration().get().getNameAsString();
}

Now the answer in the other thread states that in case of an multithreaded environment the value of getPackageDeclaration( ) may change in between due to the member access which is valid.
In my case that is not the issue:

private void method(){
    ...
    Map<SimpleName, com.github.javaparser.ast.CompilationUnit> units = new HashMap<>();
    for(...){
     units.put(...);
    }

    CompilationUnit sourceunit = units.get(clazzName);
    ....
    String sourcePackage = null;
    if(sourceunit.getPackageDeclaration().isPresent()){
        sourcePackage = sourceunit.getPackageDeclaration().get().getNameAsString();
    }
    ....
}

I even tried to make the method synchronized to see whether the issue disappeasrs  which didn't help.
So can I fix the sonar issue?

[Scott B.]

Hi.

I would say that SonarJava doesn't know whether sourceunit.getPackageDeclaration() returns always the same Optional instance or not.

A better solution is:

Optional<PackageDeclaration> packageDeclaration = sourceunit.getPackageDeclaration();
if(packageDeclaration .isPresent()){
    sourcePackage = packageDeclaration.get().getNameAsString();
}

[Nicolas Peru]

Hi, 

Scott analysis is correct : we don't do X.files analysis (yet) and as such we consider that each call to a method can potentially return a different value. 

So storing the value in a variable will change the reporting of that issue. 

Cheers, 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/bfa47dba-b268-49ca-a591-6c750459fbc2%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Nicolas Peru | SonarSource

Senior Developer

https://twitter.com/benzonico

https://sonarsource.com

[tobias.v...@nepatec.de]

Hi,

thanks for the clarification.
Sorry for posting my question here - did read the intro text too late ;-)

Best regards!

[Nicolas Peru]

Hi, 

no worry.

Your question here is fine and welcome ! 

Cheers, 

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/6f0dc200-effb-49d8-b9cb-1c4154d5311d%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[simon schrottner]

hi

i have to say i still think that isPresent - get combinations are a codestyle, why dont you use map?

String sourcepackage = sourceunit.getPackageDeclaration().map(s -> s.getNameAsString()).orElse(null)

- if you are returning an object in orElse, please use .orElseGet -> as you can introduce a memory leak

- you can also use a method reference within the map, to make it even "shorter"

Br

Simon