Skip to content

Commit

Permalink
Fix extract local for autocloseable expression in try with resources (#…
Browse files Browse the repository at this point in the history 
…1195)

* Fix extract local for autocloseable expression in try with resources

- fixes #1194
- add logic to ExtractTempRefactoring to determine if dealing with
  autocloseable expression taken from resource expression in try
  with resources in which case place the new variable declaration
  in the resource expressions
- add new tests to AssistQuickFixTest1d7
  • Loading branch information
@jjohnstn
jjohnstn committed Feb 14, 2024
1 parent 296f188 commit d843e10
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 11 deletions.
78 changes: 68 additions & 10 deletions .../refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ private boolean hasFinalModifer(List<IExtendedModifier> modifiers) {

private void createAndInsertTempDeclaration() throws CoreException {
Expression initializer= getSelectedExpression().createCopyTarget(fCURewrite.getASTRewrite(), true);
VariableDeclarationStatement vds= createTempDeclaration(initializer);
VariableDeclarationExpression vds= createTempDeclaration(initializer);

boolean insertAtSelection;
if (!fReplaceAllOccurrences) {
Expand All @@ -1140,6 +1140,44 @@ private void createAndInsertTempDeclaration() throws CoreException {
fDeclareFinal= true;
}

if (node instanceof TryStatement tryStatement) {
if (getSelectedExpression().getAssociatedNode() instanceof Expression originalexp) {
ITypeBinding vdsBinding= originalexp.resolveTypeBinding();
if (vdsBinding != null) {
boolean isAutoCloseable= false;
for (ITypeBinding superType : Bindings.getAllSuperTypes(vdsBinding)) {
if ("java.lang.AutoCloseable".equals(superType.getQualifiedName())) { //$NON-NLS-1$
isAutoCloseable= true;
break;
}
}
if (isAutoCloseable) {
List<Expression> resources= tryStatement.resources();
ASTNode originalNode= getSelectedExpression().getAssociatedNode();
boolean resourceFound= false;
for (Expression resource : resources) {
int offset= resource.getStartPosition();
ASTNode parent= originalNode.getParent();
while (parent != null) {
if (parent == resource) {
resourceFound= true;
node= resource;
fReplaceAllOccurrences= false;
break;
} else if (parent.getStartPosition() < offset) {
break;
}
parent= parent.getParent();
}
if (resourceFound) {
break;
}
}
}
}
}
}

IASTFragment[] retainOnlyReplacableMatches= retainOnlyReplacableMatches(getMatchingFragments());
if (retainOnlyReplacableMatches == null) {
return;
Expand All @@ -1153,11 +1191,18 @@ private void createAndInsertTempDeclaration() throws CoreException {
break;
}
}
if (node instanceof SwitchStatement || node instanceof EnhancedForStatement) {
AST ast= fCURewrite.getAST();
if (node instanceof SwitchStatement || node instanceof EnhancedForStatement || node instanceof TryStatement) {
/* must insert above switch statement */
fStartPoint= 0;
fEndPoint= retainOnlyReplacableMatches.length - 1;
insertAt(node, vds);
ExpressionStatement exs= ast.newExpressionStatement(vds);
insertAt(node, exs);
return;
} else if (node != null && node.getLocationInParent() == TryStatement.RESOURCES2_PROPERTY) {
fStartPoint= 0;
fEndPoint= retainOnlyReplacableMatches.length - 1;
insertResourceAt(node, vds);
return;
} else {
if (insertAtSelection) {
Expand All @@ -1167,18 +1212,20 @@ private void createAndInsertTempDeclaration() throws CoreException {
fSeen.add(retainOnlyReplacableMatches[selectNumber]);
}
if (realCommonASTNode != null || retainOnlyReplacableMatches.length == 0) {
insertAt(getSelectedExpression().getAssociatedNode(), vds);
ExpressionStatement exs= ast.newExpressionStatement(vds);
insertAt(getSelectedExpression().getAssociatedNode(), exs);
}
return;
}
}
ASTNode realCommonASTNode= null;
ExpressionStatement exs= ast.newExpressionStatement(vds);
realCommonASTNode= evalStartAndEnd(retainOnlyReplacableMatches, selectNumber, null);
if (realCommonASTNode == null && selectNumber >= 0) {
fSeen.add(retainOnlyReplacableMatches[selectNumber]);
}
if (realCommonASTNode != null) {
insertAt(realCommonASTNode, vds);
insertAt(realCommonASTNode, exs);
}
return;
}
Expand Down Expand Up @@ -1281,14 +1328,14 @@ private ASTNode convertToExtractNode(ASTNode target) {
return target;
}

private VariableDeclarationStatement createTempDeclaration(Expression initializer) throws CoreException {
private VariableDeclarationExpression createTempDeclaration(Expression initializer) throws CoreException {
AST ast= fCURewrite.getAST();

VariableDeclarationFragment vdf= ast.newVariableDeclarationFragment();
vdf.setName(ast.newSimpleName(fTempName));
vdf.setInitializer(initializer);

VariableDeclarationStatement vds= ast.newVariableDeclarationStatement(vdf);
VariableDeclarationExpression vds= ast.newVariableDeclarationExpression(vdf);
if (fDeclareFinal) {
if (!hasFinalModifer(vds.modifiers())) {
vds.modifiers().add(ast.newModifier(ModifierKeyword.FINAL_KEYWORD));
Expand All @@ -1312,12 +1359,22 @@ private VariableDeclarationStatement createTempDeclaration(Expression initialize
return vds;
}

private void insertResourceAt(ASTNode target, VariableDeclarationExpression exp) {
ASTRewrite rewrite= fCURewrite.getASTRewrite();
TextEditGroup groupDescription= fCURewrite.createGroupDescription(RefactoringCoreMessages.ExtractTempRefactoring_declare_local_variable);
StructuralPropertyDescriptor locationInParent= target.getLocationInParent();
ASTNode parent= target.getParent();
ListRewrite listRewrite= rewrite.getListRewrite(parent, (ChildListPropertyDescriptor) locationInParent);
listRewrite.insertBefore(exp, target, groupDescription);
}

private void insertAt(ASTNode target, Statement declaration) {
ASTRewrite rewrite= fCURewrite.getASTRewrite();
TextEditGroup groupDescription= fCURewrite.createGroupDescription(RefactoringCoreMessages.ExtractTempRefactoring_declare_local_variable);
ASTNode parent= target.getParent();
StructuralPropertyDescriptor locationInParent= target.getLocationInParent();
while (locationInParent != Block.STATEMENTS_PROPERTY && locationInParent != SwitchStatement.STATEMENTS_PROPERTY) {
while (locationInParent != Block.STATEMENTS_PROPERTY && locationInParent != SwitchStatement.STATEMENTS_PROPERTY
&& locationInParent != TryStatement.RESOURCES2_PROPERTY) {
if (locationInParent == IfStatement.THEN_STATEMENT_PROPERTY
|| locationInParent == IfStatement.ELSE_STATEMENT_PROPERTY
|| locationInParent == ForStatement.BODY_PROPERTY
Expand Down Expand Up @@ -1695,17 +1752,18 @@ private void replaceSelectedExpressionWithTempDeclaration() throws CoreException
evalStartAndEnd(retainOnlyReplacableMatches(getMatchingFragments()), 0, Integer.valueOf(selectedExpression.getStartPosition() + selectedExpression.getLength()));

Expression initializer= (Expression) rewrite.createMoveTarget(selectedExpression);
VariableDeclarationStatement tempDeclaration= createTempDeclaration(initializer);
VariableDeclarationExpression tempDeclarationExpression= createTempDeclaration(initializer);
ASTNode replacement;

ASTNode parent= selectedExpression.getParent();
boolean isParentLambda= parent instanceof LambdaExpression;
AST ast= rewrite.getAST();
ExpressionStatement tempDeclaration= ast.newExpressionStatement(tempDeclarationExpression);
if (isParentLambda) {
Block blockBody= ast.newBlock();
blockBody.statements().add(tempDeclaration);
if (!Bindings.isVoidType(((LambdaExpression) parent).resolveMethodBinding().getReturnType())) {
List<VariableDeclarationFragment> fragments= tempDeclaration.fragments();
List<VariableDeclarationFragment> fragments= tempDeclarationExpression.fragments();
SimpleName varName= fragments.get(0).getName();
ReturnStatement returnStatement= ast.newReturnStatement();
returnStatement.setExpression(ast.newSimpleName(varName.getIdentifier()));
Expand Down
84 changes: 83 additions & 1 deletion org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/AssistQuickFixTest1d7.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2011, 2020 IBM Corporation and others.
* Copyright (c) 2011, 2024 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -1000,6 +1000,88 @@ public void testUnwrapTryStatement() throws Exception {
assertProposalDoesNotExist(proposals, REMOVE_SURROUNDING_TRY_BLOCK);
}

@Test
public void testExtractLocalInTryWithResource1() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf= new StringBuilder();
buf.append("package test1;\n");
buf.append("import java.io.BufferedReader;\n");
buf.append("import java.io.FileReader;\n");
buf.append("import java.io.Reader;\n");
buf.append("public class E {\n");
buf.append(" void foo() throws Exception {\n");
buf.append(" try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n");
buf.append(" Reader r = new BufferedReader(new FileReader(\"a.b\"))) {\n");
buf.append(" r.read();\n");
buf.append(" }\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);

String str= "new FileReader(\"a.b\")";
AssistContext context= getCorrectionContext(cu, buf.toString().indexOf(str) + str.length(), 0);
List<IJavaCompletionProposal> proposals= collectAssists(context, false);

buf= new StringBuilder();
buf.append("package test1;\n");
buf.append("import java.io.BufferedReader;\n");
buf.append("import java.io.FileReader;\n");
buf.append("import java.io.Reader;\n");
buf.append("public class E {\n");
buf.append(" void foo() throws Exception {\n");
buf.append(" try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n");
buf.append(" FileReader fileReader = new FileReader(\"a.b\");\n");
buf.append(" Reader r = new BufferedReader(fileReader)) {\n");
buf.append(" r.read();\n");
buf.append(" }\n");
buf.append(" }\n");
buf.append("}\n");
String expected= buf.toString();

assertExpectedExistInProposals(proposals, new String[] { expected });
}

@Test
public void testExtractLocalInTryWithResource2() throws Exception {
IPackageFragment pack1= fSourceFolder.createPackageFragment("test1", false, null);
StringBuilder buf= new StringBuilder();
buf.append("package test1;\n");
buf.append("import java.io.BufferedReader;\n");
buf.append("import java.io.FileReader;\n");
buf.append("import java.io.Reader;\n");
buf.append("public class E {\n");
buf.append(" void foo() throws Exception {\n");
buf.append(" try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n");
buf.append(" Reader r = new BufferedReader(new FileReader(\"a.b\"))) {\n");
buf.append(" r.read();\n");
buf.append(" }\n");
buf.append(" }\n");
buf.append("}\n");
ICompilationUnit cu= pack1.createCompilationUnit("E.java", buf.toString(), false, null);

String str= "\"a.b\"";
AssistContext context= getCorrectionContext(cu, buf.toString().indexOf(str) + str.length(), 0);
List<IJavaCompletionProposal> proposals= collectAssists(context, false);

buf= new StringBuilder();
buf.append("package test1;\n");
buf.append("import java.io.BufferedReader;\n");
buf.append("import java.io.FileReader;\n");
buf.append("import java.io.Reader;\n");
buf.append("public class E {\n");
buf.append(" void foo() throws Exception {\n");
buf.append(" String string = \"a.b\";\n");
buf.append(" try (Reader s = new BufferedReader(new FileReader(\"c.d\"));\n");
buf.append(" Reader r = new BufferedReader(new FileReader(string))) {\n");
buf.append(" r.read();\n");
buf.append(" }\n");
buf.append(" }\n");
buf.append("}\n");
String expected= buf.toString();

assertExpectedExistInProposals(proposals, new String[] { expected });
}

@Test
public void testInferDiamondArguments() throws Exception {

Expand Down

0 comments on commit d843e10

Please sign in to comment.