From f95be40d78263ce894496e793ff6ca470c682606 Mon Sep 17 00:00:00 2001 From: anna Date: Mon, 14 Mar 2011 15:55:19 +0100 Subject: [PATCH] do not warn about side effects on simple getter/setter (IDEA-65545) --- .../quickfix/RemoveUnusedVariableFix.java | 8 +- .../refactoring/psi/PropertyUtils.java | 170 +++++++++++++++++- .../classmetrics/MethodCountInspection.java | 6 +- .../CallToSimpleGetterInClassInspection.java | 4 +- .../CallToSimpleSetterInClassInspection.java | 4 +- .../com/siyeh/ig/psiutils/MethodUtils.java | 167 ----------------- .../ig/threading/VariableAccessVisitor.java | 6 +- 7 files changed, 182 insertions(+), 183 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveUnusedVariableFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveUnusedVariableFix.java index c93be0bc78c0..b1fbc12df379 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveUnusedVariableFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveUnusedVariableFix.java @@ -30,6 +30,7 @@ import com.intellij.psi.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.PsiUtilBase; +import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.IncorrectOperationException; import gnu.trove.THashSet; import org.jetbrains.annotations.NonNls; @@ -290,8 +291,11 @@ public class RemoveUnusedVariableFix implements IntentionAction { public static boolean checkSideEffects(PsiElement element, PsiVariable variable, List sideEffects) { if (sideEffects == null || element == null) return false; if (element instanceof PsiMethodCallExpression) { - sideEffects.add(element); - return true; + final PsiMethod psiMethod = ((PsiMethodCallExpression)element).resolveMethod(); + if (psiMethod == null || !PropertyUtils.isSimpleGetter(psiMethod) && !PropertyUtils.isSimpleSetter(psiMethod)) { + sideEffects.add(element); + return true; + } } if (element instanceof PsiNewExpression) { PsiNewExpression newExpression = (PsiNewExpression)element; diff --git a/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java b/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java index 1c72b8a1b638..dc1b741c2717 100644 --- a/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java +++ b/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java @@ -16,11 +16,11 @@ package com.intellij.refactoring.psi; import com.intellij.openapi.project.Project; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiField; -import com.intellij.psi.PsiMethod; -import com.intellij.psi.PsiModifier; +import com.intellij.psi.*; +import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PropertyUtil; +import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.Nullable; public class PropertyUtils { private PropertyUtils() { @@ -41,4 +41,166 @@ public class PropertyUtils { final boolean isStatic = field.hasModifierProperty(PsiModifier.STATIC); return PropertyUtil.findPropertyGetter(containingClass, propertyName, isStatic, true); } + + @Nullable + public static PsiField getFieldOfGetter(PsiMethod method) { + if (method == null) { + return null; + } + final PsiParameterList parameterList = method.getParameterList(); + if (parameterList.getParametersCount() != 0) { + return null; + } + @NonNls final String name = method.getName(); + if (!name.startsWith("get") && !name.startsWith("is")) { + return null; + } + if (method.hasModifierProperty(PsiModifier.SYNCHRONIZED)) { + return null; + } + final PsiCodeBlock body = method.getBody(); + if (body == null) { + return null; + } + final PsiStatement[] statements = body.getStatements(); + if (statements.length != 1) { + return null; + } + final PsiStatement statement = statements[0]; + if (!(statement instanceof PsiReturnStatement)) { + return null; + } + final PsiReturnStatement returnStatement = + (PsiReturnStatement)statement; + final PsiExpression value = returnStatement.getReturnValue(); + if (value == null) { + return null; + } + if (!(value instanceof PsiReferenceExpression)) { + return null; + } + final PsiReferenceExpression reference = (PsiReferenceExpression)value; + final PsiExpression qualifier = reference.getQualifierExpression(); + if (qualifier != null && !(qualifier instanceof PsiThisExpression) && !(qualifier instanceof PsiSuperExpression)) { + return null; + } + final PsiElement referent = reference.resolve(); + if (referent == null) { + return null; + } + if (!(referent instanceof PsiField)) { + return null; + } + final PsiField field = (PsiField)referent; + final PsiType fieldType = field.getType(); + final PsiType returnType = method.getReturnType(); + if (returnType == null) { + return null; + } + if (!fieldType.equalsToText(returnType.getCanonicalText())) { + return null; + } + final PsiClass fieldContainingClass = field.getContainingClass(); + final PsiClass methodContainingClass = method.getContainingClass(); + if (InheritanceUtil.isInheritorOrSelf(methodContainingClass, fieldContainingClass, true)) { + return field; + } + else { + return null; + } + } + + public static boolean isSimpleGetter(PsiMethod method) { + return getFieldOfGetter(method) != null; + } + + @Nullable + public static PsiField getFieldOfSetter(PsiMethod method) { + if (method == null) { + return null; + } + final PsiParameterList parameterList = method.getParameterList(); + if (parameterList.getParametersCount() != 1) { + return null; + } + @NonNls final String name = method.getName(); + if (!name.startsWith("set")) { + return null; + } + if (method.hasModifierProperty(PsiModifier.SYNCHRONIZED)) { + return null; + } + final PsiCodeBlock body = method.getBody(); + if (body == null) { + return null; + } + final PsiStatement[] statements = body.getStatements(); + if (statements.length != 1) { + return null; + } + final PsiStatement statement = statements[0]; + if (!(statement instanceof PsiExpressionStatement)) { + return null; + } + final PsiExpressionStatement possibleAssignmentStatement = (PsiExpressionStatement)statement; + final PsiExpression possibleAssignment = possibleAssignmentStatement.getExpression(); + if (!(possibleAssignment instanceof PsiAssignmentExpression)) { + return null; + } + final PsiAssignmentExpression assignment = (PsiAssignmentExpression)possibleAssignment; + final PsiJavaToken sign = assignment.getOperationSign(); + if (!JavaTokenType.EQ.equals(sign.getTokenType())) { + return null; + } + final PsiExpression lhs = assignment.getLExpression(); + if (!(lhs instanceof PsiReferenceExpression)) { + return null; + } + final PsiReferenceExpression reference = (PsiReferenceExpression)lhs; + final PsiExpression qualifier = reference.getQualifierExpression(); + if (qualifier != null && !(qualifier instanceof PsiThisExpression) && !(qualifier instanceof PsiSuperExpression)) { + return null; + } + final PsiElement referent = reference.resolve(); + if (referent == null) { + return null; + } + if (!(referent instanceof PsiField)) { + return null; + } + final PsiField field = (PsiField)referent; + final PsiClass fieldContainingClass = field.getContainingClass(); + final PsiClass methodContainingClass = method.getContainingClass(); + if (!InheritanceUtil.isInheritorOrSelf(methodContainingClass, fieldContainingClass, true)) { + return null; + } + final PsiExpression rhs = assignment.getRExpression(); + if (!(rhs instanceof PsiReferenceExpression)) { + return null; + } + final PsiReferenceExpression rReference = (PsiReferenceExpression)rhs; + final PsiExpression rQualifier = rReference.getQualifierExpression(); + if (rQualifier != null) { + return null; + } + final PsiElement rReferent = rReference.resolve(); + if (rReferent == null) { + return null; + } + if (!(rReferent instanceof PsiParameter)) { + return null; + } + final PsiType fieldType = field.getType(); + final PsiType parameterType = ((PsiVariable)rReferent).getType(); + if (fieldType.equalsToText(parameterType.getCanonicalText())) { + return field; + } + else { + return null; + } + } + + public static boolean isSimpleSetter(PsiMethod method) { + return getFieldOfSetter(method) != null; + } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/classmetrics/MethodCountInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/classmetrics/MethodCountInspection.java index dcf31b69fb16..9b1dedb1467f 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/classmetrics/MethodCountInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/classmetrics/MethodCountInspection.java @@ -17,10 +17,10 @@ package com.siyeh.ig.classmetrics; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiMethod; +import com.intellij.refactoring.psi.PropertyUtils; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; -import com.siyeh.ig.psiutils.MethodUtils; import com.siyeh.ig.ui.CheckBox; import org.jetbrains.annotations.NotNull; @@ -117,8 +117,8 @@ public class MethodCountInspection extends BaseInspection { continue; } if (ignoreGettersAndSetters) { - if (MethodUtils.isSimpleGetter(method) || - MethodUtils.isSimpleSetter(method)) { + if (PropertyUtils.isSimpleGetter(method) || + PropertyUtils.isSimpleSetter(method)) { continue; } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleGetterInClassInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleGetterInClassInspection.java index a3254c1396bf..6ba894eb84b0 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleGetterInClassInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleGetterInClassInspection.java @@ -20,6 +20,7 @@ import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.search.searches.OverridingMethodsSearch; +import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.IncorrectOperationException; import com.intellij.util.Query; import com.siyeh.InspectionGadgetsBundle; @@ -27,7 +28,6 @@ import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.psiutils.ClassUtils; -import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -183,7 +183,7 @@ public class CallToSimpleGetterInClassInspection extends BaseInspection { return; } } - if(!MethodUtils.isSimpleGetter(method)){ + if(!PropertyUtils.isSimpleGetter(method)){ return; } final Query query = OverridingMethodsSearch.search( diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleSetterInClassInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleSetterInClassInspection.java index 8e29c978c4ce..5dfdb7724f01 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleSetterInClassInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/performance/CallToSimpleSetterInClassInspection.java @@ -20,6 +20,7 @@ import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.search.searches.OverridingMethodsSearch; +import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.IncorrectOperationException; import com.intellij.util.Query; import com.siyeh.InspectionGadgetsBundle; @@ -27,7 +28,6 @@ import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; import com.siyeh.ig.psiutils.ClassUtils; -import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -180,7 +180,7 @@ public class CallToSimpleSetterInClassInspection extends BaseInspection { return; } } - if(!MethodUtils.isSimpleSetter(method)){ + if(!PropertyUtils.isSimpleSetter(method)){ return; } final Query query = OverridingMethodsSearch.search( diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/MethodUtils.java b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/MethodUtils.java index 1ce3326f6f22..18fccbf954ae 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/MethodUtils.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/MethodUtils.java @@ -275,171 +275,4 @@ public class MethodUtils{ final PsiStatement[] statements = body.getStatements(); return statements.length == 0; } - - @Nullable - public static PsiField getFieldOfGetter(PsiMethod method) { - if (method == null) { - return null; - } - final PsiParameterList parameterList = method.getParameterList(); - if (parameterList.getParametersCount() != 0){ - return null; - } - @NonNls final String name = method.getName(); - if (!name.startsWith("get") && !name.startsWith("is")) { - return null; - } - if(method.hasModifierProperty(PsiModifier.SYNCHRONIZED)){ - return null; - } - final PsiCodeBlock body = method.getBody(); - if(body == null){ - return null; - } - final PsiStatement[] statements = body.getStatements(); - if(statements.length != 1){ - return null; - } - final PsiStatement statement = statements[0]; - if(!(statement instanceof PsiReturnStatement)){ - return null; - } - final PsiReturnStatement returnStatement = - (PsiReturnStatement) statement; - final PsiExpression value = returnStatement.getReturnValue(); - if(value == null){ - return null; - } - if(!(value instanceof PsiReferenceExpression)){ - return null; - } - final PsiReferenceExpression reference = (PsiReferenceExpression) value; - final PsiExpression qualifier = reference.getQualifierExpression(); - if(qualifier != null && !(qualifier instanceof PsiThisExpression) - && !(qualifier instanceof PsiSuperExpression)){ - return null; - } - final PsiElement referent = reference.resolve(); - if(referent == null){ - return null; - } - if(!(referent instanceof PsiField)){ - return null; - } - final PsiField field = (PsiField) referent; - final PsiType fieldType = field.getType(); - final PsiType returnType = method.getReturnType(); - if(returnType == null){ - return null; - } - if(!fieldType.equalsToText(returnType.getCanonicalText())){ - return null; - } - final PsiClass fieldContainingClass = field.getContainingClass(); - final PsiClass methodContainingClass = method.getContainingClass(); - if (InheritanceUtil.isInheritorOrSelf(methodContainingClass, - fieldContainingClass, true)) { - return field; - } else { - return null; - } - } - - public static boolean isSimpleGetter(PsiMethod method){ - return getFieldOfGetter(method) != null; - } - - @Nullable - public static PsiField getFieldOfSetter(PsiMethod method) { - if (method == null) { - return null; - } - final PsiParameterList parameterList = method.getParameterList(); - if (parameterList.getParametersCount() != 1){ - return null; - } - @NonNls final String name = method.getName(); - if (!name.startsWith("set")) { - return null; - } - if(method.hasModifierProperty(PsiModifier.SYNCHRONIZED)){ - return null; - } - final PsiCodeBlock body = method.getBody(); - if(body == null){ - return null; - } - final PsiStatement[] statements = body.getStatements(); - if(statements.length != 1){ - return null; - } - final PsiStatement statement = statements[0]; - if(!(statement instanceof PsiExpressionStatement)){ - return null; - } - final PsiExpressionStatement possibleAssignmentStatement = - (PsiExpressionStatement) statement; - final PsiExpression possibleAssignment = - possibleAssignmentStatement.getExpression(); - if(!(possibleAssignment instanceof PsiAssignmentExpression)){ - return null; - } - final PsiAssignmentExpression assignment = - (PsiAssignmentExpression) possibleAssignment; - final PsiJavaToken sign = assignment.getOperationSign(); - if(!JavaTokenType.EQ.equals(sign.getTokenType())){ - return null; - } - final PsiExpression lhs = assignment.getLExpression(); - if(!(lhs instanceof PsiReferenceExpression)){ - return null; - } - final PsiReferenceExpression reference = (PsiReferenceExpression) lhs; - final PsiExpression qualifier = reference.getQualifierExpression(); - if(qualifier != null && !(qualifier instanceof PsiThisExpression) && - !(qualifier instanceof PsiSuperExpression)){ - return null; - } - final PsiElement referent = reference.resolve(); - if(referent == null){ - return null; - } - if(!(referent instanceof PsiField)){ - return null; - } - final PsiField field = (PsiField) referent; - final PsiClass fieldContainingClass = field.getContainingClass(); - final PsiClass methodContainingClass = method.getContainingClass(); - if(!InheritanceUtil.isInheritorOrSelf(methodContainingClass, - fieldContainingClass, true)){ - return null; - } - final PsiExpression rhs = assignment.getRExpression(); - if(!(rhs instanceof PsiReferenceExpression)){ - return null; - } - final PsiReferenceExpression rReference = (PsiReferenceExpression) rhs; - final PsiExpression rQualifier = rReference.getQualifierExpression(); - if(rQualifier != null){ - return null; - } - final PsiElement rReferent = rReference.resolve(); - if(rReferent == null){ - return null; - } - if(!(rReferent instanceof PsiParameter)){ - return null; - } - final PsiType fieldType = field.getType(); - final PsiType parameterType = ((PsiVariable) rReferent).getType(); - if (fieldType.equalsToText(parameterType.getCanonicalText())) { - return field; - } else { - return null; - } - } - - public static boolean isSimpleSetter(PsiMethod method){ - return getFieldOfSetter(method) != null; - } } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/VariableAccessVisitor.java b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/VariableAccessVisitor.java index c2cbba9b943c..5b0c6d0be4c0 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/threading/VariableAccessVisitor.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/threading/VariableAccessVisitor.java @@ -19,8 +19,8 @@ import com.intellij.psi.*; import com.intellij.psi.search.SearchScope; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.containers.HashMap; -import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.NotNull; import java.util.Collection; @@ -91,9 +91,9 @@ class VariableAccessVisitor extends JavaRecursiveElementVisitor{ return; } final PsiMethod method = (PsiMethod)methodExpression.resolve(); - PsiField field = MethodUtils.getFieldOfGetter(method); + PsiField field = PropertyUtils.getFieldOfGetter(method); if(field == null){ - field = MethodUtils.getFieldOfSetter(method); + field = PropertyUtils.getFieldOfSetter(method); } if(field == null){ return;