Bug 366453 - [quick fix][1.7] "Add exception to existing catch clause"

needs to be conditional

The first gerrit was a pre-cleaning to improve the readability (code
merging and empty newlines).
This current and second gerrit actually fixes the ticket.

Change-Id: Iea016b79b9ff31cebaaff6720d928635c9738cd2
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>

2 files changed, 274 insertions, 37 deletions

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/LocalCorrectionsQuickFixTest1d7.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/LocalCorrectionsQuickFixTest1d7.java
index 0d3275e061..54819c90a6 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/LocalCorrectionsQuickFixTest1d7.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/LocalCorrectionsQuickFixTest1d7.java
@@ -49,9 +49,8 @@ import org.eclipse.jdt.internal.ui.JavaPlugin;
  * Those tests should run on Java Dolphin 1.7 .
  */
 public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
-
 	@Rule
-    public ProjectTestSetup projectSetup= new Java1d7ProjectTestSetup();
+	public ProjectTestSetup projectSetup= new Java1d7ProjectTestSetup();
 
 	private IJavaProject fJProject1;
 
@@ -93,9 +92,11 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
 		StringBuffer buf= new StringBuffer();
 		buf.append("package test1;\n");
+		buf.append("\n");
 		buf.append("import java.io.FileNotFoundException;\n");
-		buf.append("import java.io.IOException;\n");
 		buf.append("import java.io.InterruptedIOException;\n");
+		buf.append("import java.text.ParseException;\n");
+		buf.append("\n");
 		buf.append("public class E {\n");
 		buf.append("    void foo(int a) {\n");
 		buf.append("        try {\n");
@@ -104,7 +105,7 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 		buf.append("            else if (a < 20)\n");
 		buf.append("                throw new InterruptedIOException();\n");
 		buf.append("            else\n");
-		buf.append("                throw new IOException();\n");
+		buf.append("                throw new ParseException(\"bar\", 0);\n");
 		buf.append("        } catch (FileNotFoundException | InterruptedIOException ex) {\n");
 		buf.append("            ex.printStackTrace();\n");
 		buf.append("        } \n");
@@ -119,18 +120,20 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
+		buf.append("\n");
 		buf.append("import java.io.FileNotFoundException;\n");
-		buf.append("import java.io.IOException;\n");
 		buf.append("import java.io.InterruptedIOException;\n");
+		buf.append("import java.text.ParseException;\n");
+		buf.append("\n");
 		buf.append("public class E {\n");
-		buf.append("    void foo(int a) throws IOException {\n");
+		buf.append("    void foo(int a) throws ParseException {\n");
 		buf.append("        try {\n");
 		buf.append("            if (a < 10)\n");
 		buf.append("                throw new FileNotFoundException();\n");
 		buf.append("            else if (a < 20)\n");
 		buf.append("                throw new InterruptedIOException();\n");
 		buf.append("            else\n");
-		buf.append("                throw new IOException();\n");
+		buf.append("                throw new ParseException(\"bar\", 0);\n");
 		buf.append("        } catch (FileNotFoundException | InterruptedIOException ex) {\n");
 		buf.append("            ex.printStackTrace();\n");
 		buf.append("        } \n");
@@ -140,9 +143,11 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
+		buf.append("\n");
 		buf.append("import java.io.FileNotFoundException;\n");
-		buf.append("import java.io.IOException;\n");
 		buf.append("import java.io.InterruptedIOException;\n");
+		buf.append("import java.text.ParseException;\n");
+		buf.append("\n");
 		buf.append("public class E {\n");
 		buf.append("    void foo(int a) {\n");
 		buf.append("        try {\n");
@@ -151,10 +156,10 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 		buf.append("            else if (a < 20)\n");
 		buf.append("                throw new InterruptedIOException();\n");
 		buf.append("            else\n");
-		buf.append("                throw new IOException();\n");
+		buf.append("                throw new ParseException(\"bar\", 0);\n");
 		buf.append("        } catch (FileNotFoundException | InterruptedIOException ex) {\n");
 		buf.append("            ex.printStackTrace();\n");
-		buf.append("        } catch (IOException e) {\n");
+		buf.append("        } catch (ParseException e) {\n");
 		buf.append("        } \n");
 		buf.append("    }\n");
 		buf.append("}\n");
@@ -162,9 +167,11 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 
 		buf= new StringBuffer();
 		buf.append("package test1;\n");
+		buf.append("\n");
 		buf.append("import java.io.FileNotFoundException;\n");
-		buf.append("import java.io.IOException;\n");
 		buf.append("import java.io.InterruptedIOException;\n");
+		buf.append("import java.text.ParseException;\n");
+		buf.append("\n");
 		buf.append("public class E {\n");
 		buf.append("    void foo(int a) {\n");
 		buf.append("        try {\n");
@@ -173,8 +180,8 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 		buf.append("            else if (a < 20)\n");
 		buf.append("                throw new InterruptedIOException();\n");
 		buf.append("            else\n");
-		buf.append("                throw new IOException();\n");
-		buf.append("        } catch (FileNotFoundException | InterruptedIOException | IOException ex) {\n");
+		buf.append("                throw new ParseException(\"bar\", 0);\n");
+		buf.append("        } catch (FileNotFoundException | InterruptedIOException | ParseException ex) {\n");
 		buf.append("            ex.printStackTrace();\n");
 		buf.append("        } \n");
 		buf.append("    }\n");
@@ -185,6 +192,202 @@ public class LocalCorrectionsQuickFixTest1d7 extends QuickFixTest {
 	}
 
 	@Test
+	public void testUncaughtSuperException() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+		String sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("E.java", sample, false, null);
+
+		CompilationUnit astRoot= getASTRoot(cu);
+		ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot);
+		assertNumberOfProposals(proposals, 3);
+		assertCorrectLabels(proposals);
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) throws IOException {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected1= sample;
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } catch (IOException e) {\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected2= sample;
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (IOException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected3= sample;
+
+		assertExpectedExistInProposals(proposals, new String[] { expected1, expected2, expected3 });
+	}
+
+	@Test
+	public void testUncaughtSuperExceptionUnionType() throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
+		String sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "import java.io.InterruptedIOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else if (a < 20)\n" //
+				+ "                throw new InterruptedIOException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException | InterruptedIOException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		ICompilationUnit cu= pack1.createCompilationUnit("E.java", sample, false, null);
+
+		CompilationUnit astRoot= getASTRoot(cu);
+		ArrayList<IJavaCompletionProposal> proposals= collectCorrections(cu, astRoot);
+		assertNumberOfProposals(proposals, 3);
+		assertCorrectLabels(proposals);
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "import java.io.InterruptedIOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) throws IOException {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else if (a < 20)\n" //
+				+ "                throw new InterruptedIOException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException | InterruptedIOException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected1= sample;
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "import java.io.InterruptedIOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else if (a < 20)\n" //
+				+ "                throw new InterruptedIOException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (FileNotFoundException | InterruptedIOException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } catch (IOException e) {\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected2= sample;
+
+		sample= "" //
+				+ "package test1;\n" //
+				+ "\n" //
+				+ "import java.io.FileNotFoundException;\n" //
+				+ "import java.io.IOException;\n" //
+				+ "import java.io.InterruptedIOException;\n" //
+				+ "\n" //
+				+ "public class E {\n" //
+				+ "    void foo(int a) {\n" //
+				+ "        try {\n" //
+				+ "            if (a < 10)\n" //
+				+ "                throw new FileNotFoundException();\n" //
+				+ "            else if (a < 20)\n" //
+				+ "                throw new InterruptedIOException();\n" //
+				+ "            else\n" //
+				+ "                throw new IOException();\n" //
+				+ "        } catch (IOException | InterruptedIOException ex) {\n" //
+				+ "            ex.printStackTrace();\n" //
+				+ "        } \n" //
+				+ "    }\n" //
+				+ "}\n";
+		String expected3= sample;
+
+		assertExpectedExistInProposals(proposals, new String[] { expected1, expected2, expected3 });
+	}
+
+	@Test
 	public void testUncaughtExceptionTryWithResources1() throws Exception {
 
 		IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/LocalCorrectionsSubProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/LocalCorrectionsSubProcessor.java
index 618f303070..87121b85f9 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/LocalCorrectionsSubProcessor.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/LocalCorrectionsSubProcessor.java
@@ -328,26 +328,59 @@ public class LocalCorrectionsSubProcessor {
 					ImportRewriteContext importRewriteContext= new ContextSensitiveImportRewriteContext(decl, imports);
 
 					CatchClause catchClause= catchClauses.get(0);
-					Type type= catchClause.getException().getType();
-					List<Type> addedTypes= new ArrayList<>();
+					Type originalType= catchClause.getException().getType();
 
-					addNewExceptions(ast, filteredExceptions, rewrite, proposal, imports, importRewriteContext, addedTypes);
-
-					if (type instanceof UnionType) {
-						UnionType unionType= (UnionType) type;
+					if (originalType instanceof UnionType) {
+						UnionType unionType= (UnionType) originalType;
 						ListRewrite listRewrite= rewrite.getListRewrite(unionType, UnionType.TYPES_PROPERTY);
+						@SuppressWarnings("unchecked")
+						List<Type> existingTypes= new ArrayList<>(unionType.types());
+
+						for (int i= 0; i < filteredExceptions.size(); i++) {
+							Type addedType= addNewException(ast, filteredExceptions, rewrite, proposal, imports, importRewriteContext, i);
+							boolean isReplaced= false;
+
+							for (Type existingType : existingTypes) {
+								if (existingType.resolveBinding().isSubTypeCompatible(filteredExceptions.get(i))) {
+									listRewrite.replace(existingType, addedType, null);
+									isReplaced= true;
+									break;
+								}
+							}
 
-						for (Type addedType : addedTypes) {
-							listRewrite.insertLast(addedType, null);
+							if (!isReplaced) {
+								listRewrite.insertLast(addedType, null);
+							}
 						}
 					} else {
-						UnionType newUnionType= ast.newUnionType();
-						List<Type> types= newUnionType.types();
+						Type firstType= null;
+						List<Type> typesToAdd= new ArrayList<>();
+
+						for (int i= 0; i < filteredExceptions.size(); i++) {
+							Type addedType= addNewException(ast, filteredExceptions, rewrite, proposal, imports, importRewriteContext, i);
+
+							if (originalType.resolveBinding().isSubTypeCompatible(filteredExceptions.get(i))) {
+								firstType= addedType;
+							} else {
+								typesToAdd.add(addedType);
+							}
+						}
 
-						types.add((Type) rewrite.createCopyTarget(type));
-						types.addAll(addedTypes);
+						if (!typesToAdd.isEmpty()) {
+							UnionType newUnionType= ast.newUnionType();
+							List<Type> types= newUnionType.types();
 
-						rewrite.replace(type, newUnionType, null);
+							if (firstType == null) {
+								types.add(ASTNodes.createMoveTarget(rewrite, originalType));
+							} else {
+								types.add(firstType);
+							}
+							types.addAll(typesToAdd);
+
+							rewrite.replace(originalType, newUnionType, null);
+						} else if (firstType != null) {
+							rewrite.replace(originalType, firstType, null);
+						}
 					}
 
 					proposals.add(proposal);
@@ -369,7 +402,9 @@ public class LocalCorrectionsSubProcessor {
 					UnionType newUnionType= ast.newUnionType();
 					List<Type> types= newUnionType.types();
 
-					addNewExceptions(ast, filteredExceptions, rewrite, proposal, imports, importRewriteContext, types);
+					for (int i= 0; i < filteredExceptions.size(); i++) {
+						types.add(addNewException(ast, filteredExceptions, rewrite, proposal, imports, importRewriteContext, i));
+					}
 
 					String nameKey= "name"; //$NON-NLS-1$
 					proposal.addLinkedPosition(rewrite.track(var.getName()), false, nameKey);
@@ -460,17 +495,16 @@ public class LocalCorrectionsSubProcessor {
 		}
 	}
 
-	private static void addNewExceptions(AST ast, List<ITypeBinding> filteredExceptions, ASTRewrite rewrite, LinkedCorrectionProposal proposal, ImportRewrite imports,
-			ImportRewriteContext importRewriteContext, List<Type> types) {
-		for (int i= 0; i < filteredExceptions.size(); i++) {
-			ITypeBinding excBinding= filteredExceptions.get(i);
-			Type type2= imports.addImport(excBinding, ast, importRewriteContext, TypeLocation.EXCEPTION);
-			types.add(type2);
+	private static Type addNewException(AST ast, List<ITypeBinding> filteredExceptions, ASTRewrite rewrite, LinkedCorrectionProposal proposal, ImportRewrite imports,
+			ImportRewriteContext importRewriteContext, int i) {
+		ITypeBinding excBinding= filteredExceptions.get(i);
+		Type type= imports.addImport(excBinding, ast, importRewriteContext, TypeLocation.EXCEPTION);
 
-			String typeKey= "type" + i; //$NON-NLS-1$
-			proposal.addLinkedPosition(rewrite.track(type2), false, typeKey);
-			addExceptionTypeLinkProposals(proposal, excBinding, typeKey);
-		}
+		String typeKey= "type" + i; //$NON-NLS-1$
+		proposal.addLinkedPosition(rewrite.track(type), false, typeKey);
+		addExceptionTypeLinkProposals(proposal, excBinding, typeKey);
+
+		return type;
 	}
 
 	private static void addAdditionalCatchProposal(IInvocationContext context, Collection<ICommandAccess> proposals, ICompilationUnit cu, ASTNode selectedNode, int offset, int length, BodyDeclaration decl,