Spring Security Return type of oauth2.core.ClaimAccessor#containsClaim(String) could be a primitive boolean

Current Behavior

In org.springframework.security.oauth2.core.ClaimAccessor interface there is a containsClaim method:

    /**
     * Returns {@code true} if the claim exists in {@link #getClaims()}, otherwise {@code false}.
     *
     * @param claim the name of the claim
     * @return {@code true} if the claim exists, otherwise {@code false}
     */
    default Boolean containsClaim(String claim) {
        Assert.notNull(claim, "claim cannot be null");
        return getClaims().containsKey(claim);
    }

Return type of this method is a nullable Boolean.

Expected Behavior

It seems the method could return a non-nullable primitive boolean. This is supported by all of: - javadoc - not mentioning null return value - default implementation - Map#containsKey returning primitive boolean - use in other methods assumes non-null values, e.g. return !containsClaim(claim) ? ... : ...

Context

Noticed this when SonarQube marked if (!accessTokenJwt.containsClaim(CUSTOM_CLAIM)) { code as non-compliant for rule Boxed "Boolean" should be avoided in boolean expressions.

Comment From: jgrandja

@grimsa It's not clear to me if you are reporting an issue?

Return type of this method is a nullable Boolean.

null is never returned. It's always a boolean?

Which code are you referencing in if (!accessTokenJwt.containsClaim(CUSTOM_CLAIM))?

Comment From: grimsa

@jgrandja I'm referencing ClaimAccessor#containsClaim (in the provided example accessTokenJwt is instance of ClaimAccessor).

My observation is - if null is never returned from containsClaim method, then why is the return type Boolean (which leads to false positives during static analysis) and not boolean?

It is definitely not a bug, just a possibility for minor enhancement.

Comment From: jgrandja

@grimsa What enhancement are you proposing? Change return type from Boolean to boolean OR improve javadoc?

Comment From: grimsa

Changing to boolean would be preferable. If that is not possible, then Javadoc could be improved to explain why.

Comment From: jgrandja

Changing return type to boolean would break compatibility for any 5.x version. We could update the javadoc. Would you be interested in submitting a PR for this?

Comment From: grimsa

Sure. Let me know what the javadoc should be and I'll create a PR with that change. Or maybe this should target 6.x instead?

Comment From: jgrandja

@grimsa On second thought...

Let's @Deprecated:

Boolean containsClaim(String claim)

And add:

boolean hasClaim(String claim)

We should also ensure there are no reference to containsClaim() within the codebase.

Sound good?