OPM_OVERLY_PERMISSIVE_METHOD False Positive on Interface Method

[kev22257]

When using the org.springframework.jdbc.core.SqlReturnType interface I get a SpotBugs warning for OPM_OVERLY_PERMISSIVE_METHOD for my implementation of getTypeValue because I have it defined as public when the interface requires it to be. I am wondering if the detector is confused because the interface method is not explicitly denoted public, but by definition IS.

A method in the body of an interface declaration may be declared public or private (§6.6). If no access modifier is given, the method is implicitly public. It is permitted, but discouraged as a matter of style, to redundantly specify the public modifier for a method declaration in an interface declaration.

Snip of the Spring class:

public interface SqlReturnType {

	/**
	 * Constant that indicates an unknown (or unspecified) SQL type.
	 * Passed into setTypeValue if the original operation method does
	 * not specify an SQL type.
	 * @see java.sql.Types
	 * @see JdbcOperations#update(String, Object[])
	 */
	int TYPE_UNKNOWN = Integer.MIN_VALUE;


	/**
	 * Get the type value from the specific object.
	 * @param cs the CallableStatement to operate on
	 * @param paramIndex the index of the parameter for which we need to set the value
	 * @param sqlType the SQL type of the parameter we are setting
	 * @param typeName the type name of the parameter (optional)
	 * @return the target value
	 * @throws SQLException if an SQLException is encountered setting parameter values
	 * (that is, there's no need to catch SQLException)
	 * @see java.sql.Types
	 * @see java.sql.CallableStatement#getObject
	 */
	Object getTypeValue(CallableStatement cs, int paramIndex, int sqlType, @Nullable String typeName)
			throws SQLException;

}

And my class (specifically the getTypeValue method):

public class OracleSqlReturnIntegerArray implements SqlReturnType {
    static Integer[] toIntegerArray(Array array) throws SQLException {
        BigDecimal[] bdArray = (BigDecimal[]) array.getArray();
        return Arrays.stream(bdArray)
                .map(BigDecimal::intValue)
                .toArray(Integer[]::new);
    }

    @Override
    @SuppressFBWarnings(value = "OPM_OVERLY_PERMISSIVE_METHOD",
                        justification = "Cannot reduce the visibility of the inherited method")
    public Integer[] getTypeValue(CallableStatement cs, int paramIndex,
            int sqlType, String typeName) throws SQLException {
        Array array = cs.getArray(paramIndex);
        return (array == null) ? new Integer[0] : toIntegerArray(array);
    }
}

Without public (or with any other modifier) I get the following warning:

Multiple markers at this line
	- Cannot reduce the visibility of the inherited method from SqlReturnType
	- implements org.springframework.jdbc.core.SqlReturnType.getTypeValue

[kev22257]

Was anyone able to look at this and confirm?

[kev22257]

I looked into this a little, and found that because the interface return type is Object and my return type is Integer[] the test on line 342 in OverlyPermissiveMethod fails.

fb-contrib/src/main/java/com/mebigfatguy/fbcontrib/detect/OverlyPermissiveMethod.java

Lines 337 to 344 in 28c23d6

	for (JavaClass inf : fqCls.getAllInterfaces()) {
	for (Method infMethod : inf.getMethods()) {
	if (infMethod.getName().equals(fqMethod.getMethodName())) {
	String infMethodSig = infMethod.getSignature();
	String fqMethodSig = fqMethod.getSignature();
	if (infMethodSig.equals(fqMethodSig)) {
	return true;
	}

The logic then ends up in isConstrainedByInterface and tries to match the signatures. When it checks the second argument it compares int to int (really "I" to "I") and returns false; this seems incorrect, @mebigfatguy, should there be a ! on the equals test on line 354?

fb-contrib/src/main/java/com/mebigfatguy/fbcontrib/detect/OverlyPermissiveMethod.java

Lines 349 to 358 in 28c23d6

	if (infTypes.size() == fqTypes.size()) {
	boolean matches = true;
	for (int i = 0; i < infTypes.size(); i++) {
	String infParmType = infTypes.get(i);
	String fqParmType = fqTypes.get(i);
	if (infParmType.equals(fqParmType)) {
	if (infParmType.charAt(0) != 'L' || fqParmType.charAt(0) != 'L') {
	matches = false;
	break;
	}

[mebigfatguy]

targetted for 7.6.1