Rule S1201 "Methods named "equals" should override Object.equals(Object)" flags code that is required when working with Hibernate

[gordon.d...@gmail.com]

We have some projects that use Hibernate. We occasionally need to extend the "org.hibernate.type.EnumType" class or implement the "org.hibernate.usertype.UserType" interface. In both cases our code ends up overriding an equals method and S1201 raises an issue. Rather than mark these as false-positives I'm hoping that we can improve the rule.

As I see it our options for doing that are:

A) Loosen the rule to allow these confusing equals methods when the @Override annotation is present. The assumption here is that the API you're overriding was carefully considered and they made the choice to allow the confusing equals method signature. Note that the class or interface that defines the method signature that you're overriding will still get flagged by S1201.

B) Add special-case code to disable this rule for classes that extend/implement these Hibernate types. (not preferred but safer if you think that option A is too broad)

C) This one allows Sonarqube instance admins to decide how strict they want to be. Implement option "A" PLUS create another rule that acts only on equals methods that are annotated with @Override. This preserves the current level of rule 'sensitivity' while allowing admins to disable the second rule.

I prefer option A. It would result in issues continuing to be raised on the initial API that introduces the confusing method signature and eliminate issues on any code that's forced to override those methods.

[gordon.d...@gmail.com]

Here's an example of the code that this rule currently flags.

Sorry about the verbosity here but I'm unable to use pastebin due to corporate restrictions to prevent data exfiltration:

package com.mycorp.myapp.dto;

import java.io.Serializable;

import java.lang.reflect.Method;

import java.sql.PreparedStatement;

import java.sql.ResultSet;

import java.sql.SQLException;

import java.sql.Types;

import java.util.Properties;

import org.hibernate.HibernateException;

import org.hibernate.engine.spi.SessionImplementor;

import org.hibernate.type.StringType;

import org.hibernate.usertype.ParameterizedType;

import org.hibernate.usertype.UserType;

@Deprecated

/**

 * use org.hibernate.type.EnumType

 * Unfortunately this is here to a Category enum being lowercase of 'default' which is a reserved word and can't be used.

 */

public class EnumType implements UserType, ParameterizedType

{

private static final String     DEFAULT_IDENTIFIER_METHOD_NAME = "name";

private static final String     DEFAULT_VALUE_OF_METHOD_NAME = "valueOf";

private Class<? extends Enum<?>> enumClass;

private Class<?>             identifierType;

private Method                 identifierMethod;

private Method                 valueOfMethod;

private int[]                 sqlTypes;

public static final StringType TYPE                        = new StringType();

@Override

@SuppressWarnings("unchecked")

public void setParameterValues(final Properties parameters)

{

final String enumClassName = parameters.getProperty("enumClass");

try

{

enumClass = (Class<? extends Enum<?>>) Class.forName(enumClassName).asSubclass(Enum.class);

}

catch (final ClassNotFoundException cfne)

{

throw new HibernateException("Enum class not found", cfne);

}

final String identifierMethodName = parameters.getProperty("identifierMethod", DEFAULT_IDENTIFIER_METHOD_NAME);

try

{

identifierMethod = enumClass.getMethod(identifierMethodName, new Class[0]);

identifierType = identifierMethod.getReturnType();

}

catch (final Exception e)

{

throw new HibernateException("Failed to obtain identifier method", e);

}

sqlTypes = new int[] { Types.VARCHAR };

final String valueOfMethodName = parameters.getProperty("valueOfMethod", DEFAULT_VALUE_OF_METHOD_NAME);

try

{

valueOfMethod = enumClass.getMethod(valueOfMethodName, new Class[] { identifierType });

}

catch (final Exception e)

{

throw new HibernateException("Failed to obtain valueOf method", e);

}

}

@Override

public Class<? extends Enum<?>> returnedClass()

{

return enumClass;

}

@Override

public int[] sqlTypes()

{

return sqlTypes;

}

@Override

public Object assemble(final Serializable cached, final Object owner) throws HibernateException

{

return cached;

}

@Override

public Object deepCopy(final Object value) throws HibernateException

{

return value;

}

@Override

public Serializable disassemble(final Object value) throws HibernateException

{

return (Serializable) value;

}

  // Sonar rule S1201 flags this method. The developer has to use this method signature unless/until we convince Hibernate to change their API or quit using Hibernate.

@Override

public boolean equals(final Object x, final Object y) throws HibernateException

{

return x == y;

}

@Override

public int hashCode(final Object x) throws HibernateException

{

return x.hashCode();

}

@Override

public boolean isMutable()

{

return false;

}

@Override

public Object replace(final Object original, final Object target, final Object owner) throws HibernateException

{

return original;

}

@Override

public Object nullSafeGet(final ResultSet rs, final String[] names, final SessionImplementor session, final Object owner) throws HibernateException, SQLException

{

final Object identifier = TYPE.get(rs, names[0], session);

if (rs.wasNull())

{

return null;

}

try

{

return valueOfMethod.invoke(enumClass, new Object[] { identifier });

}

catch (final Exception e)

{

throw new HibernateException("Exception while invoking valueOf method '" + valueOfMethod.getName() + "' of " + "enumeration class '" + enumClass + "'", e);

}

}

@Override

public void nullSafeSet(final PreparedStatement st, final Object value, final int index, final SessionImplementor session) throws HibernateException, SQLException

{

try

{

if (value == null)

{

st.setNull(index, TYPE.sqlType());

}

else

{

final Object identifier = identifierMethod.invoke(value, new Object[0]);

TYPE.set(st, (String) identifier, index, session);

}

}

catch (final Exception e)

{

throw new HibernateException("Exception while invoking identifierMethod '" + identifierMethod.getName() + "' of " + "enumeration class '" + enumClass + "'", e);

}

}

}

[Freddy Mallet]

+1 for option A:

• Relax the rule to not raise an issue when the equals(...) method really override another one and even if this one is not Object.equals(Object o) 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/deacab10-ba08-4035-b4a6-c29fa08ecefa%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Freddy MALLET | SonarSource

Product Director & Co-Founder
http://sonarsource.com

[Michael Gumowski]

Hey Gordon, Freddy,

I just created the following ticket: https://jira.sonarsource.com/browse/SONARJAVA-1467

It is indeed judicious to relax the rule and not raise issue on such cases! As long as we are overriding, the implementation of such method is legit.

Regards,

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/CACfeEodwdzNWr3bvATUNhAUGYtPt5cy0BB_h%2BStq91-eKGU-Bw%40mail.gmail.com.