Bug 539589 - Different behaviour when generating hashCode and equalsI20181007-1800

Added missing super call when generating hashCode in J7+. Fixed broken
outer type usage when generating hashCode and equals in J7+. Refactored
some code.

Change-Id: I4ba0ba1e0c6d48b8d2fa3980378cef0790023d58
Signed-off-by: Pierre-Yves B. <pyvesdev@gmail.com>

2 files changed, 210 insertions, 68 deletions

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/core/source/GenerateHashCodeEqualsTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/core/source/GenerateHashCodeEqualsTest.java
index bf3bdbcc65..641aac0794 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/core/source/GenerateHashCodeEqualsTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/core/source/GenerateHashCodeEqualsTest.java
@@ -11,15 +11,13 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Pierre-Yves B. <pyvesdev@gmail.com> - Generation of equals and hashcode with java 7 Objects.equals and Objects.hashcode - https://bugs.eclipse.org/424214
+ *     Pierre-Yves B. <pyvesdev@gmail.com> - Different behaviour when generating hashCode and equals - https://bugs.eclipse.org/539589
  *******************************************************************************/
 package org.eclipse.jdt.ui.tests.core.source;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import junit.framework.Test;
-import junit.framework.TestSuite;
-
 import org.junit.Assert;
 
 import org.eclipse.jdt.testplugin.JavaProjectHelper;
@@ -40,13 +38,15 @@ import org.eclipse.jdt.core.dom.IVariableBinding;
 import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
 
 import org.eclipse.jdt.internal.corext.codemanipulation.GenerateHashCodeEqualsOperation;
+import org.eclipse.jdt.internal.corext.dom.IASTSharedValues;
 import org.eclipse.jdt.internal.corext.refactoring.structure.ASTNodeSearchUtil;
 import org.eclipse.jdt.internal.corext.refactoring.util.RefactoringASTParser;
 import org.eclipse.jdt.internal.corext.util.JavaModelUtil;
 
 import org.eclipse.jdt.ui.tests.core.ProjectTestSetup;
 
-import org.eclipse.jdt.internal.corext.dom.IASTSharedValues;
+import junit.framework.Test;
+import junit.framework.TestSuite;
 
 /**
  * Tests generation of delegate methods
@@ -1309,8 +1309,8 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.hashCode(anArrayOfInts);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1384,8 +1384,8 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.deepHashCode(anArrayOfCloneables);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1461,8 +1461,8 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.deepHashCode(anArrayOfSerializables);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1536,8 +1536,8 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.deepHashCode(anArrayOfObjects);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1669,8 +1669,8 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.deepHashCode(anArrayOfInts);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1747,9 +1747,9 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 				"	public int hashCode() {\r\n" +
 				"		final int prime = 31;\r\n" +
 				"		int result = 1;\r\n" +
-				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		result = prime * result + Arrays.hashCode(anArrayOfInts);\r\n" +
 				"		result = prime * result + Arrays.deepHashCode(anArrayOfStrings);\r\n" +
+				"		result = prime * result + Objects.hash(aBool, aByte, aChar, anInt, aDouble, aFloat, aLong, aString, aListOfStrings);\r\n" +
 				"		return result;\r\n" +
 				"	}\r\n" +
 				"	/* (non-Javadoc)\r\n" +
@@ -1831,7 +1831,7 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 
 		compareSource(expected, a.getSource());
 	}
-
+	
 	/**
 	 * Test member types
 	 *
@@ -1900,6 +1900,73 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 	}
 
 	/**
+	 * Test member types with J7+ Objects.hash and Objects.equals method calls
+	 *
+	 * @throws Exception
+	 */
+	public void testMemberTypeIn17() throws Exception {
+
+		ICompilationUnit a= fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
+				"\r\n" +
+				"public class A {\r\n" +
+				"	public class Inner {\r\n" +
+				"		int x;\r\n" +
+				"	}\r\n" +
+				"}\r\n" +
+				"", true, null);
+
+		IType type= a.getType("A").getType("Inner");
+		IField[] fields= getFields(type, new String[] { "x" });
+		runJ7Operation(type, fields, false);
+
+		String expected= "package p;\r\n" +
+				"\r\n" +
+				"import java.util.Objects;\r\n" +
+				"\r\n" +
+				"public class A {\r\n" +
+				"	public class Inner {\r\n" +
+				"		int x;\r\n" +
+				"\r\n" +
+				"		/* (non-Javadoc)\r\n" +
+				"		 * @see java.lang.Object#hashCode()\r\n" +
+				"		 */\r\n" +
+				"		@Override\r\n" +
+				"		public int hashCode() {\r\n" +
+				"			final int prime = 31;\r\n" +
+				"			int result = 1;\r\n" +
+				"			result = prime * result + getOuterType().hashCode();\r\n" +
+				"			result = prime * result + Objects.hash(x);\r\n" +
+				"			return result;\r\n" +
+				"		}\r\n" +
+				"\r\n" +
+				"		/* (non-Javadoc)\r\n" +
+				"		 * @see java.lang.Object#equals(java.lang.Object)\r\n" +
+				"		 */\r\n" +
+				"		@Override\r\n" +
+				"		public boolean equals(Object obj) {\r\n" +
+				"			if (this == obj)\r\n" +
+				"				return true;\r\n" +
+				"			if (obj == null)\r\n" +
+				"				return false;\r\n" +
+				"			if (getClass() != obj.getClass())\r\n" +
+				"				return false;\r\n" +
+				"			Inner other = (Inner) obj;\r\n" +
+				"			if (!getOuterType().equals(other.getOuterType()))\r\n" +
+				"				return false;\r\n" +
+				"			return x == other.x;\r\n" +
+				"		}\r\n" +
+				"\r\n" +
+				"		private A getOuterType() {\r\n" +
+				"			return A.this;\r\n" +
+				"		}\r\n" +
+				"	}\r\n" +
+				"}\r\n" +
+				"";
+
+		compareSource(expected, a.getSource());
+	}
+
+	/**
 	 * Test non-reference types in a direct subclass of Object
 	 *
 	 * @throws Exception
@@ -2083,6 +2150,77 @@ public class GenerateHashCodeEqualsTest extends SourceTestCase {
 		}
 	}
 
+	/**
+	 * Test with J7+ Objects.hash and Objects.equals method calls Using sub-type
+	 * 
+	 * @throws Exception
+	 */
+	public void testSubTypeIn17() throws Exception {
+		fPackageP.createCompilationUnit("B.java", "package p;\r\n" +
+				"\r\n" +
+				"public class B {\r\n" +
+				"	public int hashCode() {\r\n" +
+				"		return 1;\r\n" +
+				"	}\r\n" +
+				"	public boolean equals(Object obj) {\r\n" +
+				"		return obj instanceof B;\r\n" +
+				"	}\r\n" +
+				"\r\n" +
+				"}\r\n" +
+				"", true, null);
+
+		ICompilationUnit a= fPackageP.createCompilationUnit("A.java", "package p;\r\n" +
+				"\r\n" +
+				"public class A extends B {\r\n" +
+				"\r\n" +
+				"	String aString;\r\n" +
+				"\r\n" +
+				"}\r\n" +
+				"", true, null);
+
+		IField[] fields= getFields(a.getType("A"), new String[] { "aString" });
+		runJ7Operation(a.getType("A"), fields, false);
+
+		String expected= "package p;\r\n" +
+				"\r\n" +
+				"import java.util.Objects;\r\n" +
+				"\r\n" +
+				"public class A extends B {\r\n" +
+				"\r\n" +
+				"	String aString;\r\n" +
+				"\r\n" +
+				"	/* (non-Javadoc)\r\n" +
+				"	 * @see java.lang.Object#hashCode()\r\n" +
+				"	 */\r\n" +
+				"	@Override\r\n" +
+				"	public int hashCode() {\r\n" +
+				"		final int prime = 31;\r\n" +
+				"		int result = super.hashCode();\r\n" +
+				"		result = prime * result + Objects.hash(aString);\r\n" +
+				"		return result;\r\n" +
+				"	}\r\n" +
+				"\r\n" +
+				"	/* (non-Javadoc)\r\n" +
+				"	 * @see java.lang.Object#equals(java.lang.Object)\r\n" +
+				"	 */\r\n" +
+				"	@Override\r\n" +
+				"	public boolean equals(Object obj) {\r\n" +
+				"		if (this == obj)\r\n" +
+				"			return true;\r\n" +
+				"		if (!super.equals(obj))\r\n" +
+				"			return false;\r\n" +
+				"		if (getClass() != obj.getClass())\r\n" +
+				"			return false;\r\n" +
+				"		A other = (A) obj;\r\n" +
+				"		return Objects.equals(aString, other.aString);\r\n" +
+				"	}\r\n" +
+				"\r\n" +
+				"}\r\n" +
+				"";
+
+		compareSource(expected, a.getSource());
+	}
+
 	public void testInsertAt() throws Exception {
 		StringBuffer buf= new StringBuffer();
 		buf.append("package p;\n");
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/codemanipulation/GenerateHashCodeEqualsOperation.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/codemanipulation/GenerateHashCodeEqualsOperation.java
index ca66ef02cc..76974c2b0e 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/codemanipulation/GenerateHashCodeEqualsOperation.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/codemanipulation/GenerateHashCodeEqualsOperation.java
@@ -11,10 +11,12 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Pierre-Yves B. <pyvesdev@gmail.com> - Generation of equals and hashcode with java 7 Objects.equals and Objects.hashcode - https://bugs.eclipse.org/424214
+ *     Pierre-Yves B. <pyvesdev@gmail.com> - Different behaviour when generating hashCode and equals - https://bugs.eclipse.org/539589
  *******************************************************************************/
 package org.eclipse.jdt.internal.corext.codemanipulation;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -420,67 +422,52 @@ public final class GenerateHashCodeEqualsOperation implements IWorkspaceRunnable
 		Block body= fAst.newBlock();
 		hashCodeMethod.setBody(body);
 	
-		// PRIME NUMBER
-		VariableDeclarationFragment frag= fAst.newVariableDeclarationFragment();
-		frag.setName(fAst.newSimpleName(VARIABLE_NAME_PRIME));
-		frag.setInitializer(fAst.newNumberLiteral(PRIME_NUMBER));
-
-		VariableDeclarationStatement primeNumberDeclaration= fAst.newVariableDeclarationStatement(frag);
-		primeNumberDeclaration.modifiers().add(fAst.newModifier(ModifierKeyword.FINAL_KEYWORD));
-		primeNumberDeclaration.setType(fAst.newPrimitiveType(PrimitiveType.INT));
-		if (!fUseJ7HashEquals)
+		boolean needsNoSuperCall= needsNoSuperCall(fType, METHODNAME_HASH_CODE, new ITypeBinding[0]);
+		boolean memberType= isMemberType();
+		ReturnStatement endReturn= fAst.newReturnStatement();
+		if (fUseJ7HashEquals && needsNoSuperCall && !memberType && containsNoArrays()) {
+			// return Objects.hash(...);
+			endReturn.setExpression(createStandaloneJ7HashCall());
+		} else {
+			// final int prime = 31;
+			VariableDeclarationFragment frag= fAst.newVariableDeclarationFragment();
+			frag.setName(fAst.newSimpleName(VARIABLE_NAME_PRIME));
+			frag.setInitializer(fAst.newNumberLiteral(PRIME_NUMBER));
+
+			VariableDeclarationStatement primeNumberDeclaration= fAst.newVariableDeclarationStatement(frag);
+			primeNumberDeclaration.modifiers().add(fAst.newModifier(ModifierKeyword.FINAL_KEYWORD));
+			primeNumberDeclaration.setType(fAst.newPrimitiveType(PrimitiveType.INT));
 			body.statements().add(primeNumberDeclaration);
 
-		VariableDeclarationFragment fragment= fAst.newVariableDeclarationFragment();
-		fragment.setName(fAst.newSimpleName(VARIABLE_NAME_RESULT));
+			VariableDeclarationFragment fragment= fAst.newVariableDeclarationFragment();
+			fragment.setName(fAst.newSimpleName(VARIABLE_NAME_RESULT));
 
-		VariableDeclarationStatement resultDeclaration= fAst.newVariableDeclarationStatement(fragment);
-		resultDeclaration.setType(fAst.newPrimitiveType(PrimitiveType.INT));
-		if (!fUseJ7HashEquals)
+			VariableDeclarationStatement resultDeclaration= fAst.newVariableDeclarationStatement(fragment);
+			resultDeclaration.setType(fAst.newPrimitiveType(PrimitiveType.INT));
 			body.statements().add(resultDeclaration);
 
-		if (needsNoSuperCall(fType, METHODNAME_HASH_CODE, new ITypeBinding[0])) {
-			fragment.setInitializer(fAst.newNumberLiteral(INITIAL_HASHCODE_VALUE));
-		} else {
-			SuperMethodInvocation invoc= fAst.newSuperMethodInvocation();
-			invoc.setName(fAst.newSimpleName(METHODNAME_HASH_CODE));
-			fragment.setInitializer(invoc);
-		}
+			if (needsNoSuperCall) {
+				// int result = 1;
+				fragment.setInitializer(fAst.newNumberLiteral(INITIAL_HASHCODE_VALUE));
+			} else {
+				// int result = super.hashCode();
+				SuperMethodInvocation invoc= fAst.newSuperMethodInvocation();
+				invoc.setName(fAst.newSimpleName(METHODNAME_HASH_CODE));
+				fragment.setInitializer(invoc);
+			}
 
-		if (isMemberType()) {
-			body.statements().add(createAddOuterHashCode());
-		}
+			if (memberType) {
+				// result = prime * result + getOuterType().hashCode();
+				body.statements().add(createAddOuterHashCode());
+			}
 
-		ReturnStatement endReturn= fAst.newReturnStatement();
-		if (fUseJ7HashEquals) {
 			MethodInvocation j7Invoc= fAst.newMethodInvocation();
-			List<Statement> arrayStatements= new ArrayList<>();
-
 			for (int i= 0; i < fFields.length; i++) {
 				if (fFields[i].getType().isArray())
-					arrayStatements.add(createAddArrayHashCode(fFields[i]));
-				else
+					body.statements().add(createAddArrayHashCode(fFields[i]));
+				else if (fUseJ7HashEquals)
 					j7Invoc.arguments().add(fAst.newSimpleName(fFields[i].getName()));
-			}
-
-			if (!j7Invoc.arguments().isEmpty()) {
-				j7Invoc.setExpression(getQualifiedName(JAVA_UTIL_OBJECTS));
-				j7Invoc.setName(fAst.newSimpleName(METHODNAME_HASH));
-			}
-
-			if (arrayStatements.isEmpty()) {
-				endReturn.setExpression(j7Invoc);
-			} else {
-				body.statements().add(primeNumberDeclaration);
-				body.statements().add(resultDeclaration);
-				if (!j7Invoc.arguments().isEmpty())
-					body.statements().add(prepareAssignment(j7Invoc));
-				body.statements().addAll(arrayStatements);
-				endReturn.setExpression(fAst.newSimpleName(VARIABLE_NAME_RESULT));
-			}
-		} else {
-			for (int i= 0; i < fFields.length; i++) {
-				if (fFields[i].getType().isPrimitive()) {
+				else if (fFields[i].getType().isPrimitive()) {
 					Statement[] sts= createAddSimpleHashCode(fFields[i].getType(), new IHashCodeAccessProvider() {
 
 						@Override
@@ -492,11 +479,14 @@ public final class GenerateHashCodeEqualsOperation implements IWorkspaceRunnable
 					for (int j= 0; j < sts.length; j++) {
 						body.statements().add(sts[j]);
 					}
-				} else if (fFields[i].getType().isArray())
-					body.statements().add(createAddArrayHashCode(fFields[i]));
-				else
+				} else
 					body.statements().add(createAddQualifiedHashCode(fFields[i]));
 			}
+			if (!j7Invoc.arguments().isEmpty()) {
+				j7Invoc.setExpression(getQualifiedName(JAVA_UTIL_OBJECTS));
+				j7Invoc.setName(fAst.newSimpleName(METHODNAME_HASH));
+				body.statements().add(prepareAssignment(j7Invoc));
+			}
 			endReturn.setExpression(fAst.newSimpleName(VARIABLE_NAME_RESULT));
 		}
 
@@ -518,6 +508,20 @@ public final class GenerateHashCodeEqualsOperation implements IWorkspaceRunnable
 		return hashCodeMethod;
 	}
 
+	private boolean containsNoArrays() {
+		return Arrays.stream(fFields).map(IVariableBinding::getType).noneMatch(ITypeBinding::isArray);
+	}
+
+	private MethodInvocation createStandaloneJ7HashCall() {
+		MethodInvocation j7Invoc= fAst.newMethodInvocation();
+		for (int i= 0; i < fFields.length; i++) {
+			j7Invoc.arguments().add(fAst.newSimpleName(fFields[i].getName()));
+		}
+		j7Invoc.setExpression(getQualifiedName(JAVA_UTIL_OBJECTS));
+		j7Invoc.setName(fAst.newSimpleName(METHODNAME_HASH));
+		return j7Invoc;
+	}
+
 	private Statement createAddOuterHashCode() {
 		MethodInvocation outer= fAst.newMethodInvocation();
 		outer.setName(fAst.newSimpleName(METHODNAME_OUTER_TYPE));
@@ -912,13 +916,13 @@ public final class GenerateHashCodeEqualsOperation implements IWorkspaceRunnable
 
 		body.statements().add(otherDeclaration);
 
+		if (isMemberType()) { // test outer type
+			body.statements().add(createOuterComparison());
+		}
+
 		if (fUseJ7HashEquals) {
 			body.statements().add(createJ7EqualsStatement());
 		} else {
-			if (isMemberType()) { // test outer type
-				body.statements().add(createOuterComparison());
-			}
-
 			for (int i= 0; i < fFields.length; i++) {
 				IVariableBinding field= fFields[i];
 				ITypeBinding type= field.getType();