We are using Sonarqube 8.7.
The following code detects runtime exceptions in Collections.emptyList()
The match in the constructor itself is correct as it throws a runtime exception, but the 2 matches with Collections.emptyList() are wrong, because also the cast inside can not fail in my oppinion.
Kind regards,
Michael
Hey @reitzmichnicht
Your report is a bit light in information. Can I suggest you look at the following post, and add more details?
I hope its obvious, here is some code example:
// given
public FooService(List<String> a, List<String> b) {
Objects.requireNonNull(a, "a must not be null");
Objects.requireNonNull(b, "b must not be null");
if (a.isEmpty() && b.isEmpty()) {
throw new IllegalArgumentException();
}
}
// then this should not fail in java:S5778
assertThatThrownBy(() -> new FooService(Collections.emptyList(), Collections.emptyList())).isInstanceOf(IllegalArgumentException.class);
So from my understanding Sonar thinks that Collections.emptyList() can throw an exception, I think it can not.
Kind regards,
Michael
Hello @reitzmichnicht
The problem with this assertion is that it is not clear at first glance which method call/new class is supposed to throw the expected Runtime Exception. Even if I can guess that Collections.emptyList() is not throwing any Exception and that it should be the class creation, I had to manually check emptyList() code to be 100% sure about it.
That’s exactly the spirit of the rule: we want to avoid guesses and manual verifications in tests.
That being said, I understand your concern, there are indeed situations where it could seem superfluous. If you strongly believe the code is fine as it is, you can mark it as “Won’t fix”.
Hope it clarifies the situation.
Quentin
That clarifies it.
I had the assumption that the rule really analyzes if multiple exceptions can occur, but then it’s more related to readability.
Maybe it makes sense to whitelist some methods that can be used safely.
Kind regardsm
Michael
This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.