java:S2583 Conditionally executed code should be reachable

We are using Sonarqube 8.9 LTS.

We have the following code which produces a lot of FPs for the rule java:S2583

  private final KafkaListenerEndpointRegistry kafkaListenerEndpointRegistry;

  private void startListeningToAppTopic() {
    MessageListenerContainer appTopicListenerContainer = kafkaListenerEndpointRegistry.getListenerContainer(APP_TOPIC_LISTENER);
    if (appTopicListenerContainer == null) {
      throw new IllegalStateException(KAFKA_LISTENER_NOT_FOUND_MESSAGE);
    }
  }

It complains about the null check, but having a look at the implementation of getListenerContainer, it’s like this:

   private final Map<String, MessageListenerContainer> listenerContainers = new ConcurrentHashMap<>();

   public MessageListenerContainer getListenerContainer(String id) {
       Assert.hasText(id, "Container identifier must not be empty");
       return this.listenerContainers.get(id);
   }

And a map will return null for unknown keys.

Kind regards,
Michael

Hello @reitzmichnicht

During the analysis, we are not going inside getListenerContainer to know exactly what is returned. Instead, we rely on Nullable annotations.

It means that the expected behavior is the following:

  • If no annotation is used at all: the rule should not report an issue.
  • If the package is annotated with a non-null by default annotation (ex: org.eclipse.jdt.annotation.NonNullByDefault), then report an issue.
    In a way, this is correct: the method getListenerContainer would violate the contract as it can return null.
  • To respect the contract, you therefore have to annotate getListenerContainer with @Nullable. By doing this, the issue disappears.

At this point, does it already clarify the situation? If not, please come back to us with more information concerning your code (are you using package-level/class level annotations? Are the two snippets in the same file? package?).

Thanks Quentin,

looks like the package is annotated with @org.springframework.lang.NonNullApi
Would be great if the rule description would be enhanced that it relies on such annotations and which ones are supported.

Good point. We are aware that our support of Nullability-related features is currently a bit inconsistent and not well documented. We plan to come back to it at some point, it will hopefully improve the situation.

Best,
Quentin

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