From 09b78663a1eec2c0a1ecbd14048fae8c057817ee Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Sat, 9 Mar 2024 10:30:14 -0500 Subject: [PATCH] Add quick assist for splitting try-with-resources (#1226) - add new SplitTryResourceFixCore.java - add new logic to QuickAssistProcessor - add new tests to AssistQuickFixTest1d8 - fixes #1220 --- .../text/correction/CorrectionMessages.java | 1 + .../correction/CorrectionMessages.properties | 1 + .../corext/fix/SplitTryResourceFixCore.java | 113 +++++++++++++++++ .../tests/quickfix/AssistQuickFixTest1d8.java | 116 ++++++++++++++++++ .../text/correction/QuickAssistProcessor.java | 17 +++ 5 files changed, 248 insertions(+) create mode 100644 org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SplitTryResourceFixCore.java diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java index 8d89adfdce5..91108604dcb 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.java @@ -332,6 +332,7 @@ private CorrectionMessages() { public static String QuickAssistProcessor_unwrap_synchronizedstatement; public static String QuickAssistProcessor_split_case_labels; public static String QuickAssistProcessor_splitdeclaration_description; + public static String QuickAssistProcessor_splittryresource_description; public static String QuickAssistProcessor_joindeclaration_description; public static String QuickAssistProcessor_add_inferred_lambda_parameter_types; public static String QuickAssistProcessor_replace_var_with_inferred_lambda_parameter_types; diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties index a05ff59b444..2a4899cd9e2 100644 --- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties +++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/text/correction/CorrectionMessages.properties @@ -392,6 +392,7 @@ QuickAssistProcessor_unwrap_synchronizedstatement=Remove surrounding 'synchroniz QuickAssistProcessor_split_case_labels=Split case labels QuickAssistProcessor_splitdeclaration_description=Split variable declaration +QuickAssistProcessor_splittryresource_description=Split try resource expressions into inner try-with-resources QuickAssistProcessor_exceptiontothrows_description=Replace exception with throws QuickAssistProcessor_extract_to_local_all_description=Extract to local variable (replace all occurrences) QuickAssistProcessor_extract_to_local_all_preview=Extract to local variable (replace all occurrences) if possible diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SplitTryResourceFixCore.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SplitTryResourceFixCore.java new file mode 100644 index 00000000000..42baa836dae --- /dev/null +++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/SplitTryResourceFixCore.java @@ -0,0 +1,113 @@ +/******************************************************************************* + * Copyright (c) 2024 Red Hat Inc. and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat Inc. - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.internal.corext.fix; + +import java.util.List; + +import org.eclipse.core.runtime.CoreException; + +import org.eclipse.text.edits.TextEditGroup; + +import org.eclipse.jdt.core.ICompilationUnit; +import org.eclipse.jdt.core.dom.AST; +import org.eclipse.jdt.core.dom.ASTNode; +import org.eclipse.jdt.core.dom.Block; +import org.eclipse.jdt.core.dom.CompilationUnit; +import org.eclipse.jdt.core.dom.Statement; +import org.eclipse.jdt.core.dom.TryStatement; +import org.eclipse.jdt.core.dom.VariableDeclarationExpression; +import org.eclipse.jdt.core.dom.rewrite.ASTRewrite; +import org.eclipse.jdt.core.dom.rewrite.ListRewrite; + +import org.eclipse.jdt.internal.corext.dom.ASTNodes; +import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite; +import org.eclipse.jdt.internal.corext.util.JavaModelUtil; + +import org.eclipse.jdt.internal.ui.text.correction.CorrectionMessages; + +public final class SplitTryResourceFixCore extends CompilationUnitRewriteOperationsFixCore { + + public SplitTryResourceFixCore(String name, CompilationUnit compilationUnit, CompilationUnitRewriteOperation[] operations) { + super(name, compilationUnit, operations); + } + + public static boolean initialConditionsCheck(ICompilationUnit compilationUnit, ASTNode node) { + if (!JavaModelUtil.is1d8OrHigher(compilationUnit.getJavaProject())) + return false; + ASTNode foundNode= ASTNodes.getFirstAncestorOrNull(node, VariableDeclarationExpression.class, Statement.class); + if (!(foundNode instanceof VariableDeclarationExpression varDeclExp) || varDeclExp.getLocationInParent() != TryStatement.RESOURCES2_PROPERTY) { + return false; + } + ASTNode parent= foundNode.getParent(); + if (!(parent instanceof TryStatement tryStatement) || tryStatement.resources().size() < 2) { + return false; + } + return true; + } + + public static SplitTryResourceFixCore createSplitVariableFix(CompilationUnit compilationUnit, ASTNode node) { + ASTNode foundNode= ASTNodes.getFirstAncestorOrNull(node, VariableDeclarationExpression.class, Statement.class); + if (!(foundNode instanceof VariableDeclarationExpression varDeclExp) || varDeclExp.getLocationInParent() != TryStatement.RESOURCES2_PROPERTY) { + return null; + } + ASTNode parent= foundNode.getParent(); + if (!(parent instanceof TryStatement tryStatement) || tryStatement.resources().size() < 2) { + return null; + } + return new SplitTryResourceFixCore(CorrectionMessages.QuickAssistProcessor_splittryresource_description, compilationUnit, + new CompilationUnitRewriteOperation[] { new SplitTryResourceProposalOperation(tryStatement, varDeclExp) }); + } + + private static class SplitTryResourceProposalOperation extends CompilationUnitRewriteOperation { + + private final TryStatement tryStatement; + + private final VariableDeclarationExpression expression; + + public SplitTryResourceProposalOperation(TryStatement statement, VariableDeclarationExpression expression) { + this.tryStatement= statement; + this.expression= expression; + } + + @Override + public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModelCore linkedModel) throws CoreException { + TextEditGroup group= new TextEditGroup("abc"); //$NON-NLS-1$ + final ASTRewrite rewrite= cuRewrite.getASTRewrite(); + final AST ast= cuRewrite.getAST(); + + + List resources= tryStatement.resources(); + int expIndex; + for (expIndex= 0; expIndex < resources.size(); ++expIndex) { + if (resources.get(expIndex) == expression) { + break; + } + } + TryStatement newTryStatement= ast.newTryStatement(); + Block newBlock= ast.newBlock(); + newTryStatement.setBody(newBlock); + ListRewrite listRewrite= rewrite.getListRewrite(newTryStatement, TryStatement.RESOURCES2_PROPERTY); + ListRewrite oldResourcesListRewrite= rewrite.getListRewrite(tryStatement, TryStatement.RESOURCES2_PROPERTY); + listRewrite.insertFirst(oldResourcesListRewrite.createMoveTarget(expression, (ASTNode)oldResourcesListRewrite.getOriginalList().get(resources.size() - 1)), group); + Block originalBlock= tryStatement.getBody(); + List originalStatements= originalBlock.statements(); + int size= originalStatements.size(); + ListRewrite originalBlockListRewrite= rewrite.getListRewrite(originalBlock, Block.STATEMENTS_PROPERTY); + originalBlockListRewrite.insertFirst(newTryStatement, group); + ListRewrite newBlockListRewrite= rewrite.getListRewrite(newBlock, Block.STATEMENTS_PROPERTY); + newBlockListRewrite.insertLast(originalBlockListRewrite.createMoveTarget(originalStatements.get(0), originalStatements.get(size - 1)), group); + } + } + +} diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java index 73f32045df3..a0e684b5bee 100644 --- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java +++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d8.java @@ -6766,6 +6766,122 @@ public void testAssignInTryWithResources_04() throws Exception { assertExpectedExistInProposals(proposals, new String[] { expected }); } + @Test + public void testSplitTryWithResources1() throws Exception { // https://bugs.eclipse.org/bugs/show_bug.cgi?id=530208 + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String src= + "package test1;\n" + + "\n" + + "import java.io.BufferedReader;\n" + + "import java.io.FileNotFoundException;\n" + + "import java.io.FileReader;\n" + + "import java.io.IOException;\n" + + "import java.io.Reader;\n" + + "\n" + + "class E {\n" + + " public void foo() {\n" + + " try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n" + + " Reader r = new BufferedReader(new FileReader(\"a.b\"));\n" + + " Reader t = new BufferedReader(new FileReader(\"e.f\"))) {\n" + + " r.read();\n" + + " System.out.println(\"abc\");\n" + + " } catch (FileNotFoundException e) {\n" + + " e.printStackTrace();\n" + + " } catch (IOException e) {\n" + + " e.printStackTrace();\n" + + " }\n" + + " }\n" + + "}"; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", src, false, null); + + int offset= src.indexOf("\"a.b\""); + AssistContext context= getCorrectionContext(cu, offset, 0); + List proposals= collectAssists(context, false); + + String expected= + "package test1;\n" + + "\n" + + "import java.io.BufferedReader;\n" + + "import java.io.FileNotFoundException;\n" + + "import java.io.FileReader;\n" + + "import java.io.IOException;\n" + + "import java.io.Reader;\n" + + "\n" + + "class E {\n" + + " public void foo() {\n" + + " try (Reader s = new BufferedReader(new FileReader(\"c.d\"))) {\n" + + " try (Reader r = new BufferedReader(new FileReader(\"a.b\"));\n" + + " Reader t = new BufferedReader(new FileReader(\"e.f\"))){r.read();\n" + + " System.out.println(\"abc\");} \n" + + " } catch (FileNotFoundException e) {\n" + + " e.printStackTrace();\n" + + " } catch (IOException e) {\n" + + " e.printStackTrace();\n" + + " }\n" + + " }\n" + + "}"; + + assertExpectedExistInProposals(proposals, new String[] { expected }); + } + + @Test + public void testSplitTryWithResources2() throws Exception { // https://bugs.eclipse.org/bugs/show_bug.cgi?id=530208 + IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null); + String src= + "package test1;\n" + + "\n" + + "import java.io.BufferedReader;\n" + + "import java.io.FileNotFoundException;\n" + + "import java.io.FileReader;\n" + + "import java.io.IOException;\n" + + "import java.io.Reader;\n" + + "\n" + + "class E {\n" + + " public void foo() {\n" + + " try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n" + + " Reader r = new BufferedReader(new FileReader(\"a.b\"));\n" + + " Reader t = new BufferedReader(new FileReader(\"e.f\"))) {\n" + + " r.read();\n" + + " System.out.println(\"abc\");\n" + + " } catch (FileNotFoundException e) {\n" + + " e.printStackTrace();\n" + + " } catch (IOException e) {\n" + + " e.printStackTrace();\n" + + " }\n" + + " }\n" + + "}"; + ICompilationUnit cu= pack1.createCompilationUnit("E.java", src, false, null); + + int offset= src.indexOf("\"e.f\""); + AssistContext context= getCorrectionContext(cu, offset, 0); + List proposals= collectAssists(context, false); + + String expected= + "package test1;\n" + + "\n" + + "import java.io.BufferedReader;\n" + + "import java.io.FileNotFoundException;\n" + + "import java.io.FileReader;\n" + + "import java.io.IOException;\n" + + "import java.io.Reader;\n" + + "\n" + + "class E {\n" + + " public void foo() {\n" + + " try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n" + + " Reader r = new BufferedReader(new FileReader(\"a.b\"))) {\n" + + " try (Reader t = new BufferedReader(new FileReader(\"e.f\"))){r.read();\n" + + " System.out.println(\"abc\");} \n" + + " } catch (FileNotFoundException e) {\n" + + " e.printStackTrace();\n" + + " } catch (IOException e) {\n" + + " e.printStackTrace();\n" + + " }\n" + + " }\n" + + "}"; + + assertExpectedExistInProposals(proposals, new String[] { expected }); + } + @Test public void testInlineDeprecated_1() throws Exception { Hashtable options= JavaCore.getOptions(); diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java index 665a22f7cc0..3e6050fd52a 100644 --- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java +++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/QuickAssistProcessor.java @@ -168,6 +168,7 @@ import org.eclipse.jdt.internal.corext.fix.LambdaExpressionsFixCore; import org.eclipse.jdt.internal.corext.fix.LinkedProposalModelCore; import org.eclipse.jdt.internal.corext.fix.RemoveVarOrInferredLambdaParameterTypesFixCore; +import org.eclipse.jdt.internal.corext.fix.SplitTryResourceFixCore; import org.eclipse.jdt.internal.corext.fix.SplitVariableFixCore; import org.eclipse.jdt.internal.corext.fix.StringConcatToTextBlockFixCore; import org.eclipse.jdt.internal.corext.fix.SwitchExpressionsFixCore; @@ -340,6 +341,7 @@ public boolean hasAssists(IInvocationContext context) throws CoreException { || getStringConcatToTextBlockProposal(context, coveringNode, null) || getAddStaticMemberFavoritesProposals(coveringNode, null) || getSplitSwitchLabelProposal(context, coveringNode, null) + || getSplitTryResourceProposal(context, coveringNode, null) || getDeprecatedProposal(context, coveringNode, null, null); } return false; @@ -416,6 +418,7 @@ public IJavaCompletionProposal[] getAssists(IInvocationContext context, IProblem getConvertToSwitchExpressionProposals(context, coveringNode, resultingCollections); getDoWhileRatherThanWhileProposal(context, coveringNode, resultingCollections); getStringConcatToTextBlockProposal(context, coveringNode, resultingCollections); + getSplitTryResourceProposal(context, coveringNode, resultingCollections); } return resultingCollections.toArray(new IJavaCompletionProposal[resultingCollections.size()]); } @@ -3697,6 +3700,20 @@ private boolean getSplitSwitchLabelProposal(IInvocationContext context, ASTNode return true; } + private static boolean getSplitTryResourceProposal(IInvocationContext context, ASTNode coveringNode, Collection proposals) { + if (proposals == null) { + return SplitTryResourceFixCore.initialConditionsCheck(context.getCompilationUnit(), coveringNode); + } + SplitTryResourceFixCore fix= SplitTryResourceFixCore.createSplitVariableFix(context.getASTRoot(), coveringNode); + if (fix != null) { + Image image= JavaPluginImages.get(JavaPluginImages.IMG_CORRECTION_CHANGE); + FixCorrectionProposal proposal= new FixCorrectionProposal(fix, null, IProposalRelevance.INVERT_EQUALS, image, context); + proposals.add(proposal); + return true; + } + return false; + } + private boolean getConvertFieldNamingConventionProposal(IInvocationContext context, ASTNode node, Collection resultingCollections) { if (!(node instanceof SimpleName)) { return false;