ConvertToLocalInspection: inline created local variable if this variable is a copy of initializer (IDEA-33733)

This commit is contained in:
Artemiy Sartakov
2019-01-17 20:26:42 +07:00
parent 351aa48485
commit 52f49b5c5f
11 changed files with 241 additions and 146 deletions
@@ -29,16 +29,23 @@ import com.intellij.psi.*;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.refactoring.util.InlineUtil;
import com.intellij.util.IJSwingUtilities;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.NotNullFunction;
import com.siyeh.ig.psiutils.CommentTracker;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.VariableAccessUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
/**
* refactored from {@link FieldCanBeLocalInspection}
@@ -61,16 +68,54 @@ public abstract class BaseConvertToLocalQuickFix<V extends PsiVariable> implemen
final PsiFile myFile = variable.getContainingFile();
try {
final PsiElement newDeclaration = moveDeclaration(project, variable);
if (newDeclaration == null) return;
final List<PsiElement> newDeclarations = moveDeclaration(project, variable).stream()
.map(declaration -> inlineRedundant(declaration))
.filter(Objects::nonNull)
.collect(Collectors.toList());
positionCaretToDeclaration(project, myFile, newDeclaration);
if (newDeclarations.isEmpty()) return;
positionCaretToDeclaration(project, myFile, newDeclarations.get(newDeclarations.size() - 1));
}
catch (IncorrectOperationException e) {
LOG.error(e);
}
}
@Nullable
private static PsiElement inlineRedundant(@Nullable PsiElement declaration) {
if (declaration == null) return null;
final PsiLocalVariable newVariable = extractDeclared(declaration);
if (newVariable != null) {
final PsiExpression initializer = ParenthesesUtils.stripParentheses(newVariable.getInitializer());
if (VariableAccessUtils.localVariableIsCopy(newVariable, initializer)) {
WriteAction.run(() -> {
InlineUtil.inlineVariable(newVariable, initializer);
declaration.delete();
});
return null;
}
}
return declaration;
}
@Nullable
private static PsiLocalVariable extractDeclared(@NotNull PsiElement declaration) {
if (!(declaration instanceof PsiDeclarationStatement)) return null;
final PsiElement[] declaredElements = ((PsiDeclarationStatement)declaration).getDeclaredElements();
if (declaredElements.length != 1) return null;
final PsiElement declared = declaredElements[0];
if (!(declared instanceof PsiLocalVariable)) return null;
return (PsiLocalVariable)declared;
}
@Nullable
protected abstract V getVariable(@NotNull ProblemDescriptor descriptor);
@@ -88,12 +133,12 @@ public abstract class BaseConvertToLocalQuickFix<V extends PsiVariable> implemen
protected void beforeDelete(@NotNull Project project, @NotNull V variable, @NotNull PsiElement newDeclaration) {
}
@Nullable
protected PsiElement moveDeclaration(@NotNull Project project, @NotNull V variable) {
@NotNull
protected List<PsiElement> moveDeclaration(@NotNull Project project, @NotNull V variable) {
final Collection<PsiReference> references = ReferencesSearch.search(variable).findAll();
if (references.isEmpty()) return null;
if (references.isEmpty()) return Collections.emptyList();
return moveDeclaration(project, variable, references, true);
return Collections.singletonList(moveDeclaration(project, variable, references, true));
}
protected PsiElement moveDeclaration(Project project, V variable, final Collection<? extends PsiReference> references, boolean delete) {
@@ -323,20 +323,24 @@ public class FieldCanBeLocalInspection extends AbstractBaseJavaLocalInspectionTo
}
private static class ConvertFieldToLocalQuickFix extends BaseConvertToLocalQuickFix<PsiField> {
@Nullable
@NotNull
@Override
protected PsiElement moveDeclaration(@NotNull final Project project, @NotNull final PsiField variable) {
protected List<PsiElement> moveDeclaration(@NotNull final Project project, @NotNull final PsiField variable) {
final Map<PsiCodeBlock, Collection<PsiReference>> refs = new HashMap<>();
if (!groupByCodeBlocks(ReferencesSearch.search(variable).findAll(), refs)) return null;
PsiElement element = null;
final List<PsiElement> newDeclarations = new ArrayList<>();
if (!groupByCodeBlocks(ReferencesSearch.search(variable).findAll(), refs)) return newDeclarations;
PsiElement declaration;
for (Collection<PsiReference> psiReferences : refs.values()) {
element = super.moveDeclaration(project, variable, psiReferences, false);
declaration = super.moveDeclaration(project, variable, psiReferences, false);
if (declaration != null) newDeclarations.add(declaration);
}
if (element != null) {
final PsiElement finalElement = element;
ApplicationManager.getApplication().runWriteAction(() -> deleteSourceVariable(project, variable, finalElement));
if (!newDeclarations.isEmpty()) {
final PsiElement lastDeclaration = newDeclarations.get(newDeclarations.size() - 1);
ApplicationManager.getApplication().runWriteAction(() -> deleteSourceVariable(project, variable, lastDeclaration));
}
return element;
return newDeclarations;
}
private static boolean groupByCodeBlocks(final Collection<? extends PsiReference> allReferences, Map<PsiCodeBlock, Collection<PsiReference>> refs) {
@@ -183,16 +183,25 @@ public class ParameterCanBeLocalInspection extends AbstractBaseJavaLocalInspecti
final JavaChangeInfo changeInfo = new JavaChangeInfoImpl(visibilityModifier, method, method.getName(),
returnType != null ? CanonicalTypes.createTypeWrapper(returnType) : null,
newParams, null, false, ContainerUtil.newHashSet(), ContainerUtil.newHashSet());
final ChangeSignatureProcessor cp = new ChangeSignatureProcessor(project, changeInfo) {
class ParameterToLocalProcessor extends ChangeSignatureProcessor {
private PsiElement newDeclaration;
ParameterToLocalProcessor(Project project, JavaChangeInfo changeInfo) {
super(project, changeInfo);
}
@Override
protected void performRefactoring(@NotNull UsageInfo[] usages) {
final PsiElementFactory elementFactory = JavaPsiFacade.getElementFactory(project);
final PsiElement newDeclaration = moveDeclaration(elementFactory, localName, parameter, initializer, action, references);
newDeclaration = moveDeclaration(elementFactory, localName, parameter, initializer, action, references);
super.performRefactoring(usages);
positionCaretToDeclaration(project, newDeclaration.getContainingFile(), newDeclaration);
}
};
cp.run();
}
final ParameterToLocalProcessor processor = new ParameterToLocalProcessor(project, changeInfo);
processor.run();
return processor.newDeclaration;
}
return null;
}
@@ -49,6 +49,24 @@ public class InlineUtil {
private InlineUtil() {}
/**
* Replace variable with its initializer for every variable reference.
*
* @return references after replacement
*/
@NotNull
public static Collection<PsiElement> inlineVariable(@NotNull PsiVariable variable, @NotNull PsiExpression initializer) {
final Collection<PsiElement> replacedElements = new ArrayList<>();
final Collection<PsiReference> references = ReferencesSearch.search(variable).findAll();
for (PsiReference reference : references) {
final PsiExpression expression = inlineVariable(variable, initializer, (PsiJavaCodeReferenceElement)reference);
replacedElements.add(expression);
}
return replacedElements;
}
@NotNull
public static PsiExpression inlineVariable(PsiVariable variable, PsiExpression initializer, PsiJavaCodeReferenceElement ref) throws IncorrectOperationException {
return inlineVariable(variable, initializer, ref, null);
@@ -0,0 +1,8 @@
// "Convert to local" "true"
class Test {
public Test(String param) {
System.out.println(param == null ? "null" : param);
}
}
@@ -12,6 +12,5 @@ class MyClassTest {
}
public void setEditable1(final boolean editable1) {
boolean editable11 = editable1;
}
}
@@ -0,0 +1,11 @@
// "Convert to local" "true"
class Test {
private final String f<caret>ield;
public Test(String param) {
field = param;
System.out.println(field == null ? "null" : field);
}
}
@@ -2,8 +2,7 @@
class Temp {
public Temp() {
for (int i = 0; i < 10; i++) {
<caret>int p = i;
System.out.print(p);
System.out.print(i);
}
}
}
@@ -15,11 +15,16 @@
*/
package com.siyeh.ig.psiutils;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.openapi.util.Comparing;
import com.intellij.psi.*;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.RedundantCastUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.Processor;
import org.jetbrains.annotations.Contract;
@@ -395,6 +400,121 @@ public class VariableAccessUtils {
return visitor.isAssigned();
}
/**
* Check if local variable has the same behavior as its initializer.
*/
public static boolean localVariableIsCopy(@NotNull PsiLocalVariable variable) {
return localVariableIsCopy(variable, ParenthesesUtils.stripParentheses(variable.getInitializer()));
}
/**
* Check if local variable has the same behavior as given expression.
*/
public static boolean localVariableIsCopy(@NotNull PsiLocalVariable variable, @Nullable PsiExpression expression) {
if (expression instanceof PsiTypeCastExpression) {
PsiExpression operand = ((PsiTypeCastExpression)expression).getOperand();
if (operand instanceof PsiReferenceExpression && RedundantCastUtil.isCastRedundant((PsiTypeCastExpression)expression)) {
expression = operand;
}
}
if (!(expression instanceof PsiReferenceExpression)) {
return false;
}
final PsiReferenceExpression reference = (PsiReferenceExpression)expression;
final PsiVariable initialization = ObjectUtils.tryCast(reference.resolve(), PsiVariable.class);
if (initialization == null) {
return false;
}
if (!(initialization instanceof PsiResourceVariable) && variable instanceof PsiResourceVariable) {
return false;
}
if (!(initialization instanceof PsiLocalVariable || initialization instanceof PsiParameter)) {
if (!isFinalChain(reference) || ReferencesSearch.search(variable).findAll().size() != 1) {
// only warn when variable is referenced once, to avoid warning when a field is cached in local variable
// as in e.g. gnu.trove.TObjectHash#forEach()
return false;
}
}
final PsiCodeBlock containingScope = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class);
if (containingScope == null) {
return false;
}
if (variableMayChange(containingScope, null, variable)) {
return false;
}
if (variableMayChange(containingScope, PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression()), initialization)) {
return false;
}
final PsiResolveHelper resolveHelper = JavaPsiFacade.getInstance(containingScope.getProject()).getResolveHelper();
final String initializationName = initialization.getName();
if (initializationName == null) {
return false;
}
final boolean finalVariableIntroduction =
!initialization.hasModifierProperty(PsiModifier.FINAL) && variable.hasModifierProperty(PsiModifier.FINAL) ||
PsiUtil.isLanguageLevel8OrHigher(initialization) &&
!HighlightControlFlowUtil.isEffectivelyFinal(initialization, containingScope, null) &&
HighlightControlFlowUtil.isEffectivelyFinal(variable, containingScope, null);
final PsiType variableType = variable.getType();
final PsiType initializationType = initialization.getType();
final boolean sameType = Comparing.equal(variableType, initializationType);
for (PsiReference ref : ReferencesSearch.search(variable, new LocalSearchScope(containingScope))) {
final PsiElement refElement = ref.getElement();
if (finalVariableIntroduction) {
final PsiElement element = PsiTreeUtil.getParentOfType(refElement, PsiClass.class, PsiLambdaExpression.class);
if (element != null && PsiTreeUtil.isAncestor(containingScope, element, true)) {
return false;
}
}
if (resolveHelper.resolveReferencedVariable(initializationName, refElement) != initialization) {
return false;
}
if (!sameType) {
final PsiElement parent = refElement.getParent();
if (parent instanceof PsiReferenceExpression) {
final PsiElement resolve = ((PsiReferenceExpression)parent).resolve();
if (resolve instanceof PsiMember &&
((PsiMember)resolve).hasModifierProperty(PsiModifier.PRIVATE)) {
return false;
}
}
}
}
return !TypeConversionUtil.boxingConversionApplicable(variableType, initializationType);
}
private static boolean isFinalChain(PsiReferenceExpression reference) {
while (true) {
PsiElement element = reference.resolve();
if (!(element instanceof PsiField)) return true;
if (!((PsiField)element).hasModifierProperty(PsiModifier.FINAL)) return false;
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression());
if (qualifier == null || qualifier instanceof PsiThisExpression) return true;
if (!(qualifier instanceof PsiReferenceExpression)) return false;
reference = (PsiReferenceExpression)qualifier;
}
}
private static boolean variableMayChange(PsiCodeBlock containingScope, PsiExpression qualifier, PsiVariable variable) {
while (variable != null) {
if (!variable.hasModifierProperty(PsiModifier.FINAL) &&
variableIsAssigned(variable, containingScope, false)) {
return true;
}
if (!(qualifier instanceof PsiReferenceExpression)) break;
PsiReferenceExpression qualifierReference = (PsiReferenceExpression)qualifier;
qualifier = PsiUtil.skipParenthesizedExprDown(qualifierReference.getQualifierExpression());
variable = ObjectUtils.tryCast(qualifierReference.resolve(), PsiVariable.class);
}
return false;
}
private static class VariableCollectingVisitor extends JavaRecursiveElementWalkingVisitor {
private final Set<PsiVariable> usedVariables = new HashSet<>();
@@ -15,19 +15,12 @@
*/
package com.siyeh.ig.dataflow;
import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
import com.intellij.codeInspection.JavaSuppressionUtil;
import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
import com.intellij.openapi.util.Comparing;
import com.intellij.openapi.util.WriteExternalException;
import com.intellij.psi.*;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.RedundantCastUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
@@ -118,7 +111,7 @@ public class UnnecessaryLocalVariableInspection extends BaseInspection {
}
}
}
if (isCopyVariable(variable)) {
if (VariableAccessUtils.localVariableIsCopy(variable)) {
registerVariableError(variable);
}
else if (!m_ignoreImmediatelyReturnedVariables && isImmediatelyReturned(variable)) {
@@ -138,112 +131,6 @@ public class UnnecessaryLocalVariableInspection extends BaseInspection {
}
}
private boolean isCopyVariable(PsiVariable variable) {
PsiExpression initializer = ParenthesesUtils.stripParentheses(variable.getInitializer());
if (initializer instanceof PsiTypeCastExpression) {
PsiExpression operand = ((PsiTypeCastExpression)initializer).getOperand();
if (operand instanceof PsiReferenceExpression && RedundantCastUtil.isCastRedundant((PsiTypeCastExpression)initializer)) {
initializer = operand;
}
}
if (!(initializer instanceof PsiReferenceExpression)) {
return false;
}
final PsiReferenceExpression reference = (PsiReferenceExpression)initializer;
final PsiVariable initialization = tryCast(reference.resolve(), PsiVariable.class);
if (initialization == null) {
return false;
}
if (!(initialization instanceof PsiResourceVariable) && variable instanceof PsiResourceVariable) {
return false;
}
if (!(initialization instanceof PsiLocalVariable || initialization instanceof PsiParameter)) {
if (!isFinalChain(reference) || ReferencesSearch.search(variable).findAll().size() != 1) {
// only warn when variable is referenced once, to avoid warning when a field is cached in local variable
// as in e.g. gnu.trove.TObjectHash#forEach()
return false;
}
}
final PsiCodeBlock containingScope = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class);
if (containingScope == null) {
return false;
}
if (variableMayChange(containingScope, null, variable)) {
return false;
}
if (variableMayChange(containingScope, PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression()), initialization)) {
return false;
}
final PsiResolveHelper resolveHelper = JavaPsiFacade.getInstance(containingScope.getProject()).getResolveHelper();
final String initializationName = initialization.getName();
if (initializationName == null) {
return false;
}
final boolean finalVariableIntroduction =
!initialization.hasModifierProperty(PsiModifier.FINAL) && variable.hasModifierProperty(PsiModifier.FINAL) ||
PsiUtil.isLanguageLevel8OrHigher(initialization) &&
!HighlightControlFlowUtil.isEffectivelyFinal(initialization, containingScope, null) &&
HighlightControlFlowUtil.isEffectivelyFinal(variable, containingScope, null);
final PsiType variableType = variable.getType();
final PsiType initializationType = initialization.getType();
final boolean sameType = Comparing.equal(variableType, initializationType);
for (PsiReference ref : ReferencesSearch.search(variable, new LocalSearchScope(containingScope))) {
final PsiElement refElement = ref.getElement();
if (finalVariableIntroduction) {
final PsiElement element = PsiTreeUtil.getParentOfType(refElement, PsiClass.class, PsiLambdaExpression.class);
if (element != null && PsiTreeUtil.isAncestor(containingScope, element, true)) {
return false;
}
}
if (resolveHelper.resolveReferencedVariable(initializationName, refElement) != initialization) {
return false;
}
if (!sameType) {
final PsiElement parent = refElement.getParent();
if (parent instanceof PsiReferenceExpression) {
final PsiElement resolve = ((PsiReferenceExpression)parent).resolve();
if (resolve instanceof PsiMember &&
((PsiMember)resolve).hasModifierProperty(PsiModifier.PRIVATE)) {
return false;
}
}
}
}
return !TypeConversionUtil.boxingConversionApplicable(variableType, initializationType);
}
private boolean isFinalChain(PsiReferenceExpression reference) {
while (true) {
PsiElement element = reference.resolve();
if (!(element instanceof PsiField)) return true;
if (!((PsiField)element).hasModifierProperty(PsiModifier.FINAL)) return false;
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression());
if (qualifier == null || qualifier instanceof PsiThisExpression) return true;
if (!(qualifier instanceof PsiReferenceExpression)) return false;
reference = (PsiReferenceExpression)qualifier;
}
}
private boolean variableMayChange(PsiCodeBlock containingScope, PsiExpression qualifier, PsiVariable variable) {
while (variable != null) {
if (!variable.hasModifierProperty(PsiModifier.FINAL) &&
VariableAccessUtils.variableIsAssigned(variable, containingScope, false)) {
return true;
}
if (!(qualifier instanceof PsiReferenceExpression)) break;
PsiReferenceExpression qualifierReference = (PsiReferenceExpression)qualifier;
qualifier = PsiUtil.skipParenthesizedExprDown(qualifierReference.getQualifierExpression());
variable = tryCast(qualifierReference.resolve(), PsiVariable.class);
}
return false;
}
private boolean isImmediatelyReturned(PsiVariable variable) {
final PsiCodeBlock containingScope = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class, true, PsiClass.class);
if (containingScope == null) {
@@ -18,14 +18,12 @@ package com.siyeh.ig.fixes;
import com.intellij.codeInspection.ProblemDescriptor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.refactoring.util.InlineUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.psiutils.HighlightUtils;
import org.jetbrains.annotations.NotNull;
import java.util.ArrayList;
import java.util.Collection;
public class InlineVariableFix extends InspectionGadgetsFix {
@@ -44,12 +42,9 @@ public class InlineVariableFix extends InspectionGadgetsFix {
if (initializer == null) {
return;
}
final Collection<PsiReference> references = ReferencesSearch.search(variable).findAll();
final Collection<PsiElement> replacedElements = new ArrayList<>();
for (PsiReference reference : references) {
final PsiExpression expression = InlineUtil.inlineVariable(variable, initializer, (PsiJavaCodeReferenceElement)reference);
replacedElements.add(expression);
}
final Collection<PsiElement> replacedElements = InlineUtil.inlineVariable(variable, initializer);
if (isOnTheFly()) {
HighlightUtils.highlightElements(replacedElements);
}