diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index d37d2dd0ea17..405c6f637197 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -18,7 +18,10 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.instructions.*; -import com.intellij.codeInspection.dataFlow.value.*; +import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; +import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressManager; import com.intellij.psi.*; @@ -27,6 +30,7 @@ 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.refactoring.psi.PropertyUtils; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.Stack; import org.jetbrains.annotations.NonNls; @@ -1191,7 +1195,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, createChainedVariableValue(expression))); if (!myCatchStack.isEmpty()) { addMethodThrows(expression.resolveMethod()); @@ -1357,7 +1361,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PopInstruction()); } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, (DfaValue)null)); } else { final PsiExpressionList args = expression.getArgumentList(); @@ -1374,7 +1378,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } } - addInstruction(new MethodCallInstruction(expression)); + addInstruction(new MethodCallInstruction(expression, (DfaValue)null)); if (!myCatchStack.isEmpty()) { addMethodThrows(ctr); @@ -1492,7 +1496,7 @@ class ControlFlowAnalyzer extends JavaElementVisitor { private DfaValue createDfaValueForAnotherInstanceMemberAccess(PsiReferenceExpression expression, PsiField field) { DfaValue dfaValue = null; if (expression.getQualifierExpression() != null) { - dfaValue = createChainedVariableValue(expression, field); + dfaValue = createChainedVariableValue(expression); } if (dfaValue == null) { return myFactory.getTypeFactory().create(field.getType(), NullableNotNullManager.isNullable(field)); @@ -1501,20 +1505,50 @@ class ControlFlowAnalyzer extends JavaElementVisitor { } @Nullable - private DfaVariableValue createChainedVariableValue(@NotNull PsiReferenceExpression expression, @NotNull PsiVariable target) { - PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier == null) { - return myFactory.getVarFactory().createVariableValue(target, false, null); + private DfaVariableValue createChainedVariableValue(@Nullable PsiExpression expression) { + if (expression instanceof PsiParenthesizedExpression) { + return createChainedVariableValue(((PsiParenthesizedExpression)expression).getExpression()); } - if (qualifier instanceof PsiReferenceExpression && target instanceof PsiField && target.hasModifierProperty(PsiModifier.FINAL)) { - PsiElement qTarget = ((PsiReferenceExpression)qualifier).resolve(); - if (qTarget instanceof PsiVariable) { - DfaVariableValue qualifierValue = createChainedVariableValue((PsiReferenceExpression)qualifier, (PsiVariable)qTarget); - return qualifierValue == null ? null : myFactory.getVarFactory().createVariableValue(target, false, qualifierValue); + PsiReferenceExpression refExpr; + if (expression instanceof PsiMethodCallExpression) { + refExpr = ((PsiMethodCallExpression)expression).getMethodExpression(); + } + else if (expression instanceof PsiReferenceExpression) { + refExpr = (PsiReferenceExpression)expression; + } + else { + return null; + } + + PsiVariable var = resolveToVariable(refExpr); + if (var == null) { + return null; + } + + PsiExpression qualifier = refExpr.getQualifierExpression(); + if (qualifier == null) { + return myFactory.getVarFactory().createVariableValue(var, false, null); + } + + if (var instanceof PsiField && var.hasModifierProperty(PsiModifier.FINAL)) { + DfaVariableValue qualifierValue = createChainedVariableValue(qualifier); + if (qualifierValue != null) { + return myFactory.getVarFactory().createVariableValue(var, false, qualifierValue); } } + return null; + } + @Nullable + private static PsiVariable resolveToVariable(PsiReferenceExpression refExpr) { + PsiElement target = refExpr.resolve(); + if (target instanceof PsiVariable) { + return (PsiVariable)target; + } + if (target instanceof PsiMethod) { + return PropertyUtils.getSimplyReturnedField((PsiMethod)target, PropertyUtils.getSingleReturnValue((PsiMethod)target)); + } return null; } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 2db2e72ecf1c..31fd7d552009 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -232,14 +232,12 @@ public class DataFlowInspection extends BaseLocalInspectionTool { createSimplifyToAssignmentFix() ); } - else { - boolean report = !(psiAnchor.getParent() instanceof PsiAssertStatement) || !DONT_REPORT_TRUE_ASSERT_STATEMENTS || !evaluatesToTrue; - if (report) { - final LocalQuickFix localQuickFix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue); - holder.registerProblem(psiAnchor, InspectionsBundle.message(underBinary ? "dataflow.message.constant.condition.whenriched" : "dataflow.message.constant.condition", - Boolean.toString(evaluatesToTrue)), - localQuickFix == null ? null : new LocalQuickFix[]{localQuickFix}); - } + else if (shouldReportConditionAlwaysTrueOrFalse(psiAnchor, evaluatesToTrue)) { + final LocalQuickFix fix = createSimplifyBooleanExpressionFix(psiAnchor, evaluatesToTrue); + String message = InspectionsBundle.message(underBinary ? + "dataflow.message.constant.condition.whenriched" : + "dataflow.message.constant.condition", Boolean.toString(evaluatesToTrue)); + holder.registerProblem(psiAnchor, message, fix == null ? null : new LocalQuickFix[]{fix}); } reportedAnchors.add(psiAnchor); } @@ -287,6 +285,13 @@ public class DataFlowInspection extends BaseLocalInspectionTool { } } + private boolean shouldReportConditionAlwaysTrueOrFalse(PsiElement psiAnchor, boolean evaluatesToTrue) { + if (psiAnchor.getParent() instanceof PsiAssertStatement && DONT_REPORT_TRUE_ASSERT_STATEMENTS && evaluatesToTrue) { + return false; + } + return true; + } + private static boolean isAtRHSOfBooleanAnd(PsiElement expr) { PsiElement cur = expr; diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f7de8aa2b4c5..7d7d19813359 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -198,6 +198,11 @@ public class StandardInstructionVisitor extends InstructionVisitor { @NotNull private DfaValue getMethodResultValue(MethodCallInstruction instruction, @NotNull DfaValue qualifierValue, DfaValueFactory factory) { + DfaValue precalculated = instruction.getPrecalculatedReturnValue(); + if (precalculated != null) { + return precalculated; + } + final PsiType type = instruction.getResultType(); final MethodCallInstruction.MethodType methodType = instruction.getMethodType(); if (type != null && (type instanceof PsiClassType || type.getArrayDimensions() > 0)) { diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java index 079e0f0ba4e9..6643afd59e6a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/instructions/MethodCallInstruction.java @@ -28,6 +28,7 @@ import com.intellij.codeInspection.dataFlow.DataFlowRunner; import com.intellij.codeInspection.dataFlow.DfaInstructionState; import com.intellij.codeInspection.dataFlow.DfaMemoryState; import com.intellij.codeInspection.dataFlow.InstructionVisitor; +import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.psi.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -40,15 +41,17 @@ public class MethodCallInstruction extends Instruction { private boolean myShouldFlushFields; @NotNull private final PsiExpression myContext; private final MethodType myMethodType; + @Nullable private DfaValue myPrecalculatedReturnValue; public static enum MethodType { BOXING, UNBOXING, REGULAR_METHOD_CALL, CAST } - public MethodCallInstruction(@NotNull PsiCallExpression callExpression) { + public MethodCallInstruction(@NotNull PsiCallExpression callExpression, @Nullable DfaValue precalculatedReturnValue) { this(callExpression, MethodType.REGULAR_METHOD_CALL); + myPrecalculatedReturnValue = precalculatedReturnValue; } - public MethodCallInstruction(@NotNull PsiExpression context, MethodType methodType, PsiType resultType) { + public MethodCallInstruction(@NotNull PsiExpression context, MethodType methodType, @Nullable PsiType resultType) { this(context, methodType); myType = resultType; myShouldFlushFields = false; @@ -102,6 +105,11 @@ public class MethodCallInstruction extends Instruction { return myContext; } + @Nullable + public DfaValue getPrecalculatedReturnValue() { + return myPrecalculatedReturnValue; + } + public String toString() { return myMethodType == MethodType.UNBOXING ? "UNBOX" 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 8de979f2166e..9d188b706b76 100644 --- a/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java +++ b/java/java-impl/src/com/intellij/refactoring/psi/PropertyUtils.java @@ -52,84 +52,68 @@ public class PropertyUtils { */ @Nullable public static PsiExpression getGetterReturnExpression(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; - } + return method != null && hasGetterSignature(method) ? getSingleReturnValue(method) : null; + } + + private static boolean hasGetterSignature(@NotNull PsiMethod method) { + return PropertyUtil.isSimplePropertyGetter(method) && !method.hasModifierProperty(PsiModifier.SYNCHRONIZED); + } + + @Nullable + public static PsiExpression getSingleReturnValue(@NotNull PsiMethod method) { 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; - } - return value; + final PsiStatement statement = statements.length != 1 ? null : statements[0]; + return statement instanceof PsiReturnStatement ? ((PsiReturnStatement)statement).getReturnValue() : null; } @Nullable public static PsiField getFieldOfGetter(PsiMethod method) { - final PsiExpression value = getGetterReturnExpression(method); - if (value == null) return null; + PsiField field = getSimplyReturnedField(method, getGetterReturnExpression(method)); + if (field != null) { + final PsiType returnType = method.getReturnType(); + if (returnType != null && field.getType().equalsToText(returnType.getCanonicalText())) { + return field; + } + } + return null; + } + + @Nullable + public static PsiField getSimplyReturnedField(PsiMethod method, @Nullable PsiExpression value) { if (!(value instanceof PsiReferenceExpression)) { return null; } + final PsiReferenceExpression reference = (PsiReferenceExpression)value; - final PsiExpression qualifier = reference.getQualifierExpression(); - if (qualifier instanceof PsiReferenceExpression) { - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; - final PsiElement target = referenceExpression.resolve(); - if (!(target instanceof PsiClass)) { - return null; - } - } - else if (qualifier != null && !(qualifier instanceof PsiThisExpression) && !(qualifier instanceof PsiSuperExpression)) { + if (hasSubstantialQualifier(reference)) { 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; + return InheritanceUtil.isInheritorOrSelf(method.getContainingClass(), field.getContainingClass(), true) ? field : null; + } + + private static boolean hasSubstantialQualifier(PsiReferenceExpression reference) { + final PsiExpression qualifier = reference.getQualifierExpression(); + if (qualifier == null) return false; + + if (qualifier instanceof PsiThisExpression || qualifier instanceof PsiSuperExpression) { + return false; } - 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; + + if (qualifier instanceof PsiReferenceExpression) { + return !(((PsiReferenceExpression)qualifier).resolve() instanceof PsiClass); } + return true; } public static boolean isSimpleGetter(PsiMethod method) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java new file mode 100644 index 000000000000..5eb2ef10dcc4 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ChainedFinalFieldAccessorsDfa.java @@ -0,0 +1,86 @@ +import org.jetbrains.annotations.Nullable; + +import java.lang.String; + +public class BrokenAlignment { + + void main(Data data) { + if (data.getText() != null) { + System.out.println(data.getText().hashCode()); + } + + data = new Data(null, null); + System.out.println(data.getText().hashCode()); + + if (data.inner() != null) { + System.out.println(data.inner().hashCode()); + System.out.println(data.inner().getText().hashCode()); + /* + if (data.inner() != null) { + System.out.println(data.inner().hashCode()); + } + */ + + data = new Data(null, null); + System.out.println(data.inner().hashCode()); + } + } + + void main2(Data data) { + if (data.inner() != null && data.inner().getText() != null) { + System.out.println(data.inner().hashCode()); + System.out.println(data.inner().getText().hashCode()); + } + } + + void main3(Data data) { + if (data.innerOverridden() != null) { + System.out.println(data.innerOverridden().hashCode()); + } + if (data.something() != null) { + System.out.println(data.something().hashCode()); + } + } + + private static class Data { + @Nullable final String text; + @Nullable final Data inner; + + Data(@Nullable String text, Data inner) { + this.text = text; + this.inner = inner; + } + + @Nullable + public String getText() { + return text; + } + + @Nullable + public Data inner() { + return inner; + } + + @Nullable + public Data innerOverridden() { + return inner; + } + + @Nullable + public String something() { + return new String(); + } + } + + class DataImpl extends Data { + DataImpl(@Nullable String text, Data inner) { + super(text, inner); + } + + @Nullable + @Override + public Data innerOverridden() { + return super.innerOverridden(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index cf55cb61a4f3..0e7f4ec099c0 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -79,5 +79,6 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testGreaterIsNotEquals() throws Throwable { doTest(); } public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } + public void testChainedFinalFieldAccessorsDfa() throws Throwable { doTest(); } }