Java S2201 The return value of "orElseThrow" must be used

The explanation for the rule states:

When the call to a function doesn’t have any side effects, what is the point of making the call if the results are ignored?

However, orElseThrow does indeed have side effects. It throws an exception if the result is null.

  • Are you using
    • SonarLint - IntelliJ 8.5.1.75093
  • How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)
    Optional.ofNullable(null).orElseThrow();

Hey @lbenedetto ,

Thanks for the reproducer. I’m however not sure that I agree with you.

For me, your code does not make sense in practice (I might need more context), because not using the value of the Optional seems like an antipattern, defeating the purpose of the Optional class. Is it a hack to avoid manually throw an exception?

Also, why even instantiate an Optional for the sole purpose of a simple (and less costly in memory) null verification? Are you using this syntax solely to avoid the code described in bar() below?

void foo(@Nullable String o) {
   Optional.ofNullable(o).orElseThrow();
}

void bar(@Nullable String o) {
   if (o == null) {
     // allows a much more specific and contextual Exception message
     // compared to the generic "No value present" from Optional 
     throw new NoSuchElementException("parameter 'o' is null and it should not have been the case!");
   }
}

In my mind, I would expect at least to assign the value to something of that the optional will be accessed at some point later, the orElseThrow() anyway making sure that any missing values break the flow. Summarized, the following code of foo() feels absurd compared to bar():

void foo() {
  Optional<String> o = getOptional();
  o.orElseThrow(); // issue raised by S2201
  // ...
  // do some stuff
  // ...
  String s = o.get();
  // use 's' later on
}

void bar() {
  Optional<String> o = getOptional();
  String s = o.orElseThrow(); // do not raise issue by S2201
  // ...
  // do some stuff
  // ...
  // use 's' later on ... at worth the variable will be garbage collected
}

I would appreciate your opinion or a detailed use case, in case I missed the point! :slight_smile:

Thanks,
Michael

I thought the point was to provide a minimal reproducing example, not a realistic use case.

Of course I’m not just instantiating an optional to immediately call orElseThrow. It’s more like a method that accepts an optional and does some mapping/filtering and finally terminates with an orElseThrow. And that’s the end of a void return method.

The point is, the reasoning for the rule is that you shouldn’t call methods without side effects and not use the return value. This method does have side effects, so it can make sense to call it without using the return value.

I came upon this discussion and I believe the reporter has a valid issue. Since you took exception to their minimal example, here’s a use case that I believe is valid but fails Sonar:

        Optional.ofNullable(dbConnector.getMapper().findCustomerId(customerId)).orElseThrow(DataNotFoundException::new);

,The sole purpose of this line of code is to ascertain the existence of a duplicate ID in a database. We do not care about the returned record. We simply want to use the declarative API of java.util.Optional to catch this value and throw an exception when it does not exist.

Sonar incorrectly flags this code as non compliant and “useless” in java:S2201