diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java index db6f05af217e..c7521a956175 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java @@ -14,15 +14,19 @@ import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.codeInspection.dataFlow.value.VariableDescriptor; +import com.intellij.openapi.util.Ref; import com.intellij.psi.*; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; +import com.intellij.psi.util.*; import com.intellij.util.ObjectUtils; import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.VariableAccessUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; /** @@ -156,9 +160,116 @@ public final class PlainDescriptor extends PsiVarDescriptor { PsiClass placeClass = placeMethod.getContainingClass(); if (placeClass == null || placeClass != target.getContainingClass()) return false; if (!placeMethod.hasModifierProperty(PsiModifier.STATIC) && target.hasModifierProperty(PsiModifier.STATIC)) return false; + if (!target.hasModifierProperty(PsiModifier.STATIC) && + !placeMethod.hasModifierProperty(PsiModifier.STATIC) && + methodCanBeCalledFromConstructorBeforeFieldInitializing(target, placeMethod, placeClass)) { + return true; + } return getAccessOffset(placeMethod) < getWriteOffset(target); } + private static boolean methodCanBeCalledFromConstructorBeforeFieldInitializing(@NotNull PsiField target, + @NotNull PsiMethod method, + @NotNull PsiClass placeClass) { + if (target.hasInitializer() || method.isConstructor() || + //consider cases with only one constructor to do it faster + placeClass.getConstructors().length != 1) { + return false; + } + PsiMethod constructor = placeClass.getConstructors()[0]; + if (constructor.getBody() == null || + constructor.getBody().getStatements().length == 0 || + constructor.getBody().getStatements()[0] instanceof PsiSuperExpression) { + return false; + } + PsiMethodCallExpression methodCallExpression = findCallIn(target, placeClass, method); + if (methodCallExpression == null) { + return false; + } + if (JavaPsiRecordUtil.isCompactConstructor(constructor)) { + return true; + } + if (!VariableAccessUtils.variableIsAssignedAtPoint(target, constructor, methodCallExpression)) { + return true; + } + return false; + } + + private record MethodInfo(Map> methods, boolean hasCallOutside){} + + @Nullable + private static PsiMethodCallExpression findCallIn(@NotNull PsiField field, + @NotNull PsiClass contextClass, + @NotNull PsiMethod method) { + PsiMethod constructor = contextClass.getConstructors()[0]; + if (!constructor.isPhysical() || constructor.getBody() == null) { + return null; + } + + MethodInfo cacheValue = CachedValuesManager.getCachedValue(contextClass, () -> { + PsiMethod context = contextClass.getConstructors()[0]; + ConcurrentHashMap> collectedMethods = new ConcurrentHashMap<>(); + Ref callsOutside = new Ref<>(false); + PsiManager psiManager = context.getManager(); + JavaRecursiveElementWalkingVisitor visitor = new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitLambdaExpression(@NotNull PsiLambdaExpression expression) {} + @Override + public void visitClass(@NotNull PsiClass aClass) {} + + @Override + public void visitAnonymousClass(@NotNull PsiAnonymousClass aClass) {} + + @Override + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression methodCallExpression) { + PsiExpression qualifier = methodCallExpression.getMethodExpression().getQualifierExpression(); + if (qualifier == null || qualifier instanceof PsiThisExpression) { + PsiMethod resolvedMethod = methodCallExpression.resolveMethod(); + if (resolvedMethod != null && + resolvedMethod.getContainingClass() != null && + psiManager.areElementsEquivalent(resolvedMethod.getContainingClass(), contextClass)) { + collectedMethods.compute(resolvedMethod.getName(), + (ignoreKey, methodsByCall) -> { + if (methodsByCall == null) { + methodsByCall = new HashMap<>(); + } + if (!methodsByCall.containsKey(resolvedMethod)) { + methodsByCall.put(resolvedMethod, methodCallExpression); + } + return methodsByCall; + }); + } + else { + callsOutside.set(true); + } + } + else { + callsOutside.set(true); + } + super.visitMethodCallExpression(methodCallExpression); + } + }; + context.accept(visitor); + return CachedValueProvider.Result.create( new MethodInfo(collectedMethods, callsOutside.get()), PsiModificationTracker.MODIFICATION_COUNT); + }); + PsiManager psiManager = contextClass.getManager(); + if (field.hasModifierProperty(PsiModifier.FINAL) || + (!cacheValue.hasCallOutside() && + cacheValue.methods().size() == 1 && + cacheValue.methods().entrySet().iterator().next().getValue().size() == 1)) { + Map methodsByCall = cacheValue.methods().get(method.getName()); + if (methodsByCall == null) { + return null; + } + for (PsiMethod methodByCall : methodsByCall.keySet()) { + if (psiManager.areElementsEquivalent(methodByCall, method)) { + return methodsByCall.get(methodByCall); + } + } + } + return null; + } + private static int getWriteOffset(PsiField target) { // Final field: written either in field initializer or in class initializer block which directly writes this field // Non-final field: written either in field initializer, in class initializer which directly writes this field or calls any method, diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java b/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java new file mode 100644 index 000000000000..7971f9b4cb7c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java @@ -0,0 +1,85 @@ +record MyRecord(String name, int id) { + + public MyRecord { + validateMyFields(); + } + + private void validateMyFields() { + if (this.name.isEmpty()) { + throw new IllegalArgumentException(); + } + } +} + +class OrdinaryClassInitBefore { + + final String name; + int id; + + public OrdinaryClassInitBefore() { + name = ""; + validateMyFields(); + } + + private void validateMyFields() { + if (this.name.isEmpty()) { + throw new IllegalArgumentException(); + } + } +} + +class OrdinaryClassInitAfter { + + final String name; + int id; + + public OrdinaryClassInitAfter() { + validateMyFields(); + name = ""; + } + + private void validateMyFields() { + if (this.name.isEmpty()) { + throw new IllegalArgumentException(); + } + } +} + +class OrdinaryClassNotNull { + + String name; + int id; + + public OrdinaryClassNotNull() { + validateMyFields(); + name = ""; + } + + private void validateMyFields() { + if (this.name.isEmpty()) { + throw new IllegalArgumentException(); + } + } +} + +class OrdinaryClassNotNullSeveralCalls { + + String name; + int id; + + public OrdinaryClassNotNullSeveralCalls() { + initName(); + validateMyFields(); + name = ""; + } + + private void initName() { + name = ""; + } + + private void validateMyFields() { + if (this.name.isEmpty()) { + throw new IllegalArgumentException(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldNotDuringInitialization.java b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldNotDuringInitialization.java index 6adf82977826..57224dd3a9e3 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldNotDuringInitialization.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldNotDuringInitialization.java @@ -124,6 +124,6 @@ class Test8 { void other() { System.out.println(s.hashCode()); - System.out.println(s2.hashCode()); + System.out.println(s2.hashCode()); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 50718606d6fe..3d79a4009468 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -7,9 +7,11 @@ import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInspection.dataFlow.ConstantValueInspection; import com.intellij.codeInspection.dataFlow.DataFlowInspection; import com.intellij.openapi.application.impl.NonBlockingReadActionImpl; +import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiField; +import com.intellij.testFramework.IdeaTestUtil; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.PsiTestUtil; import com.intellij.testFramework.fixtures.JavaCodeInsightTestFixture; @@ -707,6 +709,9 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testGetterNullityAfterCheck() { doTest(); } public void testInferenceNullityMismatch() { doTestWith((insp, __) -> insp.SUGGEST_NULLABLE_ANNOTATIONS = false); } public void testFieldInInstanceInitializer() { doTest(); } + public void testCallsBeforeFieldInitializing() { + IdeaTestUtil.withLevel(getModule(), LanguageLevel.JDK_17, () -> doTest()); + } public void testNullableCallWithPrecalculatedValueAndSpecialField() { doTest(); } public void testJoinConstantAndSubtype() { doTest(); } public void testDereferenceInThrowMessage() { doTest(); }