IDEA-235296 Refactor ensureCodeBlock: VariableExtractor updated, more tests

GitOrigin-RevId: f08d2d54594410e441e8367df7a5e246116a90bf
This commit is contained in:
Tagir Valeev
2020-03-17 20:53:55 +00:00
committed by intellij-monorepo-bot
parent cb1731985b
commit 6980ce3a38
7 changed files with 72 additions and 39 deletions
@@ -129,19 +129,14 @@ class VariableExtractor {
return;
}
if (myAnchor instanceof PsiExpression) {
PsiExpression place = RefactoringUtil.ensureCodeBlock(((PsiExpression)myAnchor));
if (place == null) {
CodeBlockSurrounder surrounder = CodeBlockSurrounder.forExpression((PsiExpression)myAnchor);
if (surrounder == null) {
throw new RuntimeExceptionWithAttachments(
"Cannot ensure code block: myAnchor type is " + myAnchor.getClass() + "; parent type is " + myAnchor.getParent().getClass(),
new Attachment("context.txt", myContainer.getText()));
}
PsiElement statement = RefactoringUtil.getParentStatement(place, false);
if (statement == null) {
throw new RuntimeExceptionWithAttachments(
"Cannot find parent statement for " + place.getClass() + "; parent type is " + place.getParent().getClass(),
new Attachment("context.txt", myContainer.getText()));
}
myAnchor = statement;
CodeBlockSurrounder.SurroundResult result = surrounder.surround();
myAnchor = result.getAnchor();
}
}
@@ -328,13 +323,14 @@ class VariableExtractor {
}
}
}
if (firstOccurrence != null && ControlFlowUtils.canExtractStatement(firstOccurrence) &&
if (firstOccurrence != null && CodeBlockSurrounder.canSurround(firstOccurrence) &&
!PsiUtil.isAccessedForWriting(firstOccurrence)) {
PsiExpression ancestorCandidate = ExpressionUtils.getTopLevelExpression(firstOccurrence);
if (PsiTreeUtil.isAncestor(anchor, ancestorCandidate, false)) {
PsiElement statement = RefactoringUtil.getParentStatement(ancestorCandidate, false);
PsiElement extractable = statement == null ? PsiTreeUtil.getParentOfType(ancestorCandidate, PsiField.class) : statement;
if (allOccurrences.stream().allMatch(occurrence ->
PsiTreeUtil.isAncestor(statement, occurrence, false) &&
PsiTreeUtil.isAncestor(extractable, occurrence, false) &&
(!PsiTreeUtil.isAncestor(ancestorCandidate, occurrence, false) ||
ReorderingUtils.canExtract(ancestorCandidate, occurrence) == ThreeState.NO))) {
return firstOccurrence;
@@ -0,0 +1,15 @@
class Test {
String s1 = getString();
String s;
{
if (s1 != null) {
String temp = s1.trim();
s = temp;
} else {
s = "";
}
}
native String getString();
}
@@ -0,0 +1,6 @@
class Test {
String s1 = getString();
String s = s1 != null ? <selection>s1.trim()</selection> : "";
native String getString();
}
@@ -0,0 +1,8 @@
class Test {
boolean test(String s1, String s2) {
if (s1 == null) return true;
if (s2 == null) return false;
String temp = s2.trim();
return s1.equals(temp);
}
}
@@ -0,0 +1,5 @@
class Test {
boolean test(String s1, String s2) {
return s1 == null || s2 != null && s1.equals(<selection>s2.trim()</selection>);
}
}
@@ -145,7 +145,9 @@ public class IntroduceVariableTest extends LightJavaCodeInsightTestCase {
public void testIfConditionAndChain() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testReturnAndChain() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testReturnOrChain() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testReturnOrAndChain() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testReturnTernary() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testFieldInitializer() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testAssignTernary() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testEnsureCodeBlockAroundBreakStatementJava13Preview() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
public void testEnsureCodeBlockForThrowsJava13Preview() { doTest("temp", true, false, false, JAVA_LANG_STRING); }
@@ -34,10 +34,10 @@ public class ReorderingUtils {
public static ThreeState canExtract(@NotNull PsiExpression ancestor, @NotNull PsiExpression expression) {
if (expression == ancestor) return ThreeState.YES;
if (PsiUtil.isConstantExpression(expression)) return ThreeState.YES;
PsiExpression parent = ObjectUtils.tryCast(expression.getParent(), PsiExpression.class);
PsiElement parent = expression.getParent();
if (parent instanceof PsiExpressionList) {
PsiExpression gParent = ObjectUtils.tryCast(parent.getParent(), PsiExpression.class);
if (gParent instanceof PsiCallExpression) {
PsiExpression gParent = ObjectUtils.tryCast(parent.getParent(), PsiCallExpression.class);
if (gParent != null) {
PsiExpression[] args = ((PsiExpressionList)parent).getExpressions();
int index = ArrayUtil.indexOf(args, expression);
ThreeState result = ThreeState.YES;
@@ -47,29 +47,30 @@ public class ReorderingUtils {
break;
}
}
return and(result, () -> canExtract(ancestor, parent));
return and(result, () -> canExtract(ancestor, gParent));
}
}
if (parent == null) {
PsiExpression expressionParent = ObjectUtils.tryCast(parent, PsiExpression.class);
if (expressionParent == null) {
if (PsiTreeUtil.isAncestor(ancestor, expression, true)) {
return ThreeState.UNSURE;
}
throw new IllegalArgumentException("Should be an ancestor");
}
if (parent instanceof PsiParenthesizedExpression || parent instanceof PsiInstanceOfExpression ||
parent instanceof PsiTypeCastExpression) {
return canExtract(ancestor, parent);
if (expressionParent instanceof PsiParenthesizedExpression || expressionParent instanceof PsiInstanceOfExpression ||
expressionParent instanceof PsiTypeCastExpression) {
return canExtract(ancestor, expressionParent);
}
if (parent instanceof PsiReferenceExpression) {
if (((PsiReferenceExpression)parent).getQualifierExpression() == expression) {
return canExtract(ancestor, parent);
if (expressionParent instanceof PsiReferenceExpression) {
if (((PsiReferenceExpression)expressionParent).getQualifierExpression() == expression) {
return canExtract(ancestor, expressionParent);
}
}
if (parent instanceof PsiConditionalExpression) {
PsiConditionalExpression ternary = (PsiConditionalExpression)parent;
if (expressionParent instanceof PsiConditionalExpression) {
PsiConditionalExpression ternary = (PsiConditionalExpression)expressionParent;
PsiExpression condition = ternary.getCondition();
if (condition == expression) {
return canExtract(ancestor, parent);
return canExtract(ancestor, expressionParent);
}
ThreeState result;
if (isSideEffectFree(condition, false) &&
@@ -80,25 +81,25 @@ public class ReorderingUtils {
areConditionsNecessaryFor(new PsiExpression[]{condition}, expression, ternary.getElseExpression() == expression);
result = isNecessary ? ThreeState.NO : ThreeState.UNSURE;
}
return and(result, () -> canExtract(ancestor, parent));
return and(result, () -> canExtract(ancestor, expressionParent));
}
if (parent instanceof PsiLambdaExpression) {
if (expressionParent instanceof PsiLambdaExpression) {
return ThreeState.NO;
}
if (parent instanceof PsiUnaryExpression) {
if (PsiUtil.isIncrementDecrementOperation(parent)) return ThreeState.NO;
return canExtract(ancestor, parent);
if (expressionParent instanceof PsiUnaryExpression) {
if (PsiUtil.isIncrementDecrementOperation(expressionParent)) return ThreeState.NO;
return canExtract(ancestor, expressionParent);
}
if (parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent;
if (expressionParent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadic = (PsiPolyadicExpression)expressionParent;
PsiExpression[] operands = polyadic.getOperands();
int index = ArrayUtil.indexOf(operands, expression);
if (index == 0) {
return canExtract(ancestor, parent);
return canExtract(ancestor, expressionParent);
}
IElementType tokenType = polyadic.getOperationTokenType();
if (tokenType.equals(JavaTokenType.ANDAND) || tokenType.equals(JavaTokenType.OROR)) {
return and(canMoveToStart(polyadic, index), () -> canExtract(ancestor, parent));
return and(canMoveToStart(polyadic, index), () -> canExtract(ancestor, expressionParent));
}
ThreeState result = ThreeState.YES;
for (int i=0; i<index; i++) {
@@ -107,13 +108,13 @@ public class ReorderingUtils {
break;
}
}
return and(result, () -> canExtract(ancestor, parent));
return and(result, () -> canExtract(ancestor, expressionParent));
}
if (parent instanceof PsiAssignmentExpression) {
if (expression == ((PsiAssignmentExpression)parent).getLExpression()) return ThreeState.NO;
return canExtract(ancestor, parent);
if (expressionParent instanceof PsiAssignmentExpression) {
if (expression == ((PsiAssignmentExpression)expressionParent).getLExpression()) return ThreeState.NO;
return canExtract(ancestor, expressionParent);
}
return and(ThreeState.UNSURE, () -> canExtract(ancestor, parent));
return and(ThreeState.UNSURE, () -> canExtract(ancestor, expressionParent));
}
@NotNull