[java-refactoring] Inline method: do not inline parameters that have non-pure method call in initializer

To compensate somewhat, inline parameters that are executed as the first expression inside the method (or previous expressions are harmless)

GitOrigin-RevId: ff4bc5ce45bfb3e12a83dc8ccb97b1767558bcbe
This commit is contained in:
Tagir Valeev
2022-10-07 17:17:52 +00:00
committed by intellij-monorepo-bot
parent 458f6442cb
commit bd411cb482
6 changed files with 122 additions and 16 deletions
@@ -755,18 +755,6 @@ public class InlineMethodProcessor extends BaseRefactoringProcessor {
return null;
}
@NotNull
private PsiExpression inlineParameterReference(@NotNull PsiReferenceExpression expression, BlockData blockData) {
if (expression.getQualifierExpression() != null) return expression;
PsiElement resolve = expression.resolve();
if (!(resolve instanceof PsiParameter)) return expression;
int paramIdx = ArrayUtil.find(myMethod.getParameterList().getParameters(), resolve);
if (paramIdx < 0) return expression;
PsiExpression initializer = blockData.parmVars[paramIdx].getInitializer();
if (initializer == null) return expression;
return InlineUtil.inlineInitializer((PsiVariable)resolve, initializer, expression);
}
private void substituteMethodTypeParams(PsiElement scope, final PsiSubstitutor substitutor) {
InlineUtil.substituteTypeParams(scope, substitutor, myFactory);
}
@@ -5,6 +5,7 @@ import com.intellij.codeInsight.BlockUtils;
import com.intellij.codeInsight.ChangeContextUtil;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInsight.daemon.impl.quickfix.SimplifyBooleanExpressionFix;
import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.codeInspection.redundantCast.RemoveRedundantCastUtil;
import com.intellij.java.refactoring.JavaRefactoringBundle;
import com.intellij.openapi.diagnostic.Logger;
@@ -476,12 +477,86 @@ public final class InlineUtil implements CommonJavaInlineUtil {
isAccessedForWriting = true;
}
}
if (refs.size() == 1 && !isAccessedForWriting && isFirstUse(variable, refs.get(0))) return true;
PsiExpression initializer = variable.getInitializer();
return canInlineParameterOrThisVariable(variable.getProject(), initializer, false, false,
refs.size(), isAccessedForWriting);
}
private static boolean isFirstUse(PsiLocalVariable variable, PsiReferenceExpression expression) {
if (!(variable.getParent() instanceof PsiDeclarationStatement decl)) return false;
PsiStatement statement = PsiTreeUtil.getNextSiblingOfType(decl, PsiStatement.class);
if (statement == null) return false;
PsiElement parent;
PsiExpression cur = expression;
while (true) {
parent = cur.getParent();
if (parent instanceof PsiPolyadicExpression poly) {
if (poly.getOperands()[0] != cur) return false;
cur = poly;
} else if (parent instanceof PsiParenthesizedExpression ||
parent instanceof PsiTypeCastExpression ||
parent instanceof PsiUnaryExpression ||
parent instanceof PsiInstanceOfExpression ||
parent instanceof PsiSwitchExpression) {
cur = (PsiExpression)parent;
} else if (parent instanceof PsiConditionalExpression cond) {
if (cond.getCondition() != cur) return false;
cur = cond;
} else if (parent instanceof PsiExpressionList list) {
for (PsiExpression expr : list.getExpressions()) {
if (expr == cur) break;
if (!ExpressionUtils.isSafelyRecomputableExpression(expr)) return false;
}
if (!(list.getParent() instanceof PsiCallExpression call)) return false;
PsiExpression qualifier = call instanceof PsiMethodCallExpression methodCall ? methodCall.getMethodExpression().getQualifierExpression() :
call instanceof PsiNewExpression newExpression ? newExpression.getQualifier() : null;
if (qualifier != null && !ExpressionUtils.isSafelyRecomputableExpression(qualifier)) return false;
cur = call;
}
else if (parent instanceof PsiReferenceExpression ref) {
if (parent.getParent() instanceof PsiMethodCallExpression call) {
cur = call;
} else {
cur = ref;
}
} else if (parent instanceof PsiArrayAccessExpression arr) {
if (arr.getIndexExpression() == cur && !ExpressionUtils.isSafelyRecomputableExpression(arr)) return false;
cur = arr;
} else if (parent instanceof PsiArrayInitializerExpression init) {
for (PsiExpression initializer : init.getInitializers()) {
if (initializer == cur) break;
if (!ExpressionUtils.isSafelyRecomputableExpression(initializer)) return false;
}
cur = init;
}
else if (parent instanceof PsiNewExpression newExpression) {
cur = newExpression;
}
else if (parent instanceof PsiLocalVariable var) {
return var.getParent() == statement;
}
else if (parent instanceof PsiStatement) {
if (parent != statement) return false;
if (parent instanceof PsiAssertStatement ||
parent instanceof PsiReturnStatement ||
parent instanceof PsiIfStatement ||
parent instanceof PsiExpressionStatement ||
parent instanceof PsiSynchronizedStatement ||
parent instanceof PsiSwitchStatement ||
parent instanceof PsiWhileStatement ||
parent instanceof PsiForeachStatement) {
return true;
}
return false;
}
else {
return false;
}
}
}
private static boolean canInlineParameterOrThisVariable(Project project,
PsiExpression initializer,
boolean shouldBeFinal,
@@ -572,7 +647,8 @@ public final class InlineUtil implements CommonJavaInlineUtil {
return false;
}
}
return true; //TODO: "suspicious" places to review by user!
PsiMethod method = ((PsiCallExpression)initializer).resolveMethod();
return method == null || JavaMethodContractUtil.isPure(method);
}
else if (initializer instanceof PsiLiteralExpression) {
return true;
@@ -648,7 +724,9 @@ public final class InlineUtil implements CommonJavaInlineUtil {
boolean shouldBeFinal = variable.hasModifierProperty(PsiModifier.FINAL) && strictlyFinal;
Project project = variable.getProject();
if (canInlineParameterOrThisVariable(project, initializer, shouldBeFinal, strictlyFinal, refs.size(), isAccessedForWriting)) {
boolean canInline = refs.size() == 1 && !isAccessedForWriting && isFirstUse(variable, refs.get(0)) ||
canInlineParameterOrThisVariable(project, initializer, shouldBeFinal, strictlyFinal, refs.size(), isAccessedForWriting);
if (canInline) {
if (shouldBeFinal) {
declareUsedLocalsFinal(initializer, true);
}
@@ -0,0 +1,21 @@
public class Foo {
public static void main(String[] args) {
<caret>foo(new X());
}
static void foo(X x) {
System.out.println(1);
System.out.println(x);
}
static class X {
X() {
System.out.println(0);
}
@Override
public String toString() {
return "2";
}
}
}
@@ -0,0 +1,18 @@
public class Foo {
public static void main(String[] args) {
X x = new X();
System.out.println(1);
System.out.println(x);
}
static class X {
X() {
System.out.println(0);
}
@Override
public String toString() {
return "2";
}
}
}
@@ -1,7 +1,6 @@
class AAA {
void fff(Project myProject) {
final String[] strings = new String[1];
ensureFilesWritable(strings).hasReadonlyFiles();
ensureFilesWritable(new String[1]).hasReadonlyFiles();
}
private Status ensureFilesWritable(final String[] strings) {
@@ -563,6 +563,8 @@ public class InlineMethodTest extends LightRefactoringTestCase {
public void testTernaryBranch() { doTest(); }
public void testTernaryBranchCollapsible() { doTest(); }
public void testNewWithSideEffect() { doTest(); }
@Override
protected Sdk getProjectJDK() {
return getTestName(false).contains("Src") ? IdeaTestUtil.getMockJdk17() : super.getProjectJDK();