java:S3655 false positive when used with assertj

Clean Code Report False-positive / False-negative...
1

We are running SonarQube DE 8.9.2

There is another rule (java:S5838) to advice to change our test code (which uses assertj)
from
assertThat(optionalValue.isPresent()).isTrue(); to
assertThat(optionalValue).isPresent();

after doing this change, we got another issue:

assertThat(optionalValue).isPresent();
Object value = optionalValue.get(); // here is the FP java:S3655
// do more assertions based on value, f.e. query db again
2

Hi there - any feedback about this?

7

Hey @barclay-reg ,

Thank you for your patience, and sorry, we completely forgot to answer your message.

Okay, so there is a good reason why there is an FP here.

  • Rule java:S3655 (Optional value access) is supposed to be executed only on MAIN code, and not on TEST code.
  • Rule java:S5838 (Chained AssertJ assertions), is supposed to be executed only on TEST code, and not on MAIN code.

Consequently, and due to their different scope of execution, the implementation of rule S3655 has not been thought to handle assertion frameworks that you would usually find in test cases. It therefore do not consider the isPresent() call as being as legitimate presence test (it is).

Now, I don’t understand how the rule S3655 can be executed on what seems to be test code in your example (and recognized as such, since S5838 is executed). Did you change your default configuration to handle TEST code as MAIN code, and execute all rules on it?

Cheers,
Michael

11

Hi @Michael ,

thanks for the reply. You were right, the questioned code lies within src/main/java (it’s common/reusable test code) so the problem is caused by our bad code structure.

But actually I didn’t noticed, that SonarQube rules have an association to TEST or MAIN. Is this a general concept or just implementation detail of the specific rules?
What about highlighting this - if general in the docu or issue browser, or if rule-specific within the rule description?

Anyway - many thanks to help me solving this.
kind regards
Robert

12

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.

15

Hey @barclay-reg ,

Sorry to get back to your thread so late, and thank you for your patience. I feel you deserve an answer.
This is not per se a general concept. However, our rules are assigned a scope, being MAIN or TEST. The users can not change it and it’s hardcoded in the analyzers rules. This scope will define which category of code the rule will be executed. Note that these categories are mutually exclusive for Java (a rule targeting MAIN code will not be executed in TEST, and vice-versa).

Regarding highlighting, I believe that all java rules targeting test should have the test tag, as visible here. Except that, this is currently not documented much. Note that this might actually change in the near future, as we work on re-classifying our rules.

Cheers,
Michael

1 Like