Is there way to avoid 'Prefer java.util.Optional.orElseThrow' if 'isPresent()' is right before

[romani]

detected at: checkstyle/checkstyle#16014 (comment)

if (inlineTag.isPresent()) {
            final DetailNode inlineTagNode = inlineTag.get();
....

from modernizer: Prefer java.util.Optional.orElseThrow

I mam not sure why we should do it here as we did inlineTag.isPresent().

I agree that without isPresent() right before is it better to use orElseThrow as we do not know context, but if there is context .... can we somehow to avoid violation of such code ?

[romani]

is there a way to disable specific validator ?

I did:

          <exclusionPatterns>
            <exclusionPattern>java/util/Optional.get:.*</exclusionPattern>
          </exclusionPatterns>

based on https://github.com/gaul/modernizer-maven-plugin/blob/a553adf84d6b16b9e18f3d1365afc68cea950fb5/modernizer-maven-plugin/src/main/resources/modernizer.xml#L1293C13-L1293C56

[gaul]

Modernizer has only simple bytecode-level checks with no flow control so it cannot understand if isPresent { get } sequences. But I would encourage you to call orElseThrow instead of get for reasons that Brian Goetz explains here: https://stackoverflow.com/a/49159955/2800111. It is possible that a future Java version will deprecate get which seems like something Modernizer should warn about today.

exclusionPattern is a good way to address this if you disagree with the suggestion and Modernizer supports custom violation files if you only want to use a small subset or custom checks.

I do appreciate any suggestions you have from the Checkstyle project as this project may benefit from better configuration.

[rnveach]

Following if statements from the bytecode may be near the same level of complexity as it is for us to track variable usage by the name. Even we can't do that correctly all the time for all our checks and there has been issues opened to build such a utility in CS.

It doesn't help that each compiler will write the byte code in different ways for the same Java code. I've seen code block off logic in the if statements to the end of method's code, or inline it with the rest of the other logic by jumping around it. That doesn't even account for determining if it is a simple if or really a loop. And all this would have to be worked backwards from the Optional.get, which may be another complexity.

pitest also works on the byte code only, and it used to write weird things in the report (from the source perspective) which made more sense looking at the bytecode.

[romani]

I understand general benefits of new methods, It is first time we run into this, and when I did suggested code it looked weird, we over care to readability of code in checkstyle project, and we try to find equilibrium between all best practices, and readability for regular humans(not very professional engineers).

For now I will probably disable this valuation, and we might come back to it later on, maybe new jdk will not give us option anymore or something else come up as good pattern to write such code

[rnveach]

From the executing code perspective, there is no difference between get and orElseThrow. It is really just the name of the method difference between the 2 in my Java 11 JDK.