Skip to content

Commit

Permalink
Add quick assist for splitting try-with-resources (eclipse-jdt#1226)
Browse files Browse the repository at this point in the history
- add new SplitTryResourceFixCore.java
- add new logic to QuickAssistProcessor
- add new tests to AssistQuickFixTest1d8
- fixes eclipse-jdt#1220
  • Loading branch information
jjohnstn authored Mar 9, 2024
1 parent cb1cc46 commit 09b7866
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<VariableDeclarationExpression> 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<Statement> 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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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<IJavaCompletionProposal> 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<IJavaCompletionProposal> 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<String, String> options= JavaCore.getOptions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()]);
}
Expand Down Expand Up @@ -3697,6 +3700,20 @@ private boolean getSplitSwitchLabelProposal(IInvocationContext context, ASTNode
return true;
}

private static boolean getSplitTryResourceProposal(IInvocationContext context, ASTNode coveringNode, Collection<ICommandAccess> 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<ICommandAccess> resultingCollections) {
if (!(node instanceof SimpleName)) {
return false;
Expand Down

0 comments on commit 09b7866

Please sign in to comment.