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 cda47e4ee708..33242e1a4492 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 @@ -23,10 +23,7 @@ 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.*; import java.util.function.Predicate; /** @@ -168,32 +165,23 @@ public final class PlainDescriptor extends PsiVarDescriptor { return getAccessOffset(placeMethod) < getWriteOffset(target); } + private record TargetCallInfo(@NotNull PsiMethod constructor, + @NotNull PsiMethodCallExpression call){} 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) { + + if (target.hasInitializer() || method.isConstructor()) { return false; } - PsiMethod constructor = placeClass.getConstructors()[0]; - PsiCodeBlock constructorBody = constructor.getBody(); - if (constructorBody == null || - constructorBody.getStatements().length == 0) { + TargetCallInfo callInfo = findTargetCallIn(target, placeClass, method); + if (callInfo == null) { return false; } - PsiMethodCallExpression callInConstructor = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(constructor); - if (callInConstructor != null) { - return false; - } - PsiMethodCallExpression methodCallExpression = findCallIn(target, placeClass, method); - if (methodCallExpression == null) { - return false; - } - if (JavaPsiRecordUtil.isCompactConstructor(constructor)) { + if (JavaPsiRecordUtil.isCompactConstructor(callInfo.constructor())) { return true; } - if (!VariableAccessUtils.variableIsAssignedAtPoint(target, constructor, methodCallExpression)) { + if (!VariableAccessUtils.variableIsAssignedAtPoint(target, callInfo.constructor, callInfo.call)) { return true; } return false; @@ -201,9 +189,11 @@ public final class PlainDescriptor extends PsiVarDescriptor { /** * @param methods - grouped calls and corresponded methods by their name + * @param constructor - constructor where these calls are placed * @param hasCallOutside - there are any calls, except instance calls of the same class on `this` qualifier */ - private record ConstructorMethodInfo(Map> methods, + private record ConstructorMethodInfo(@NotNull Map> methods, + @Nullable PsiMethod constructor, boolean hasCallOutside) { } @@ -211,26 +201,49 @@ public final class PlainDescriptor extends PsiVarDescriptor { * Find a method call expression in the given field's context class. * It is supposed that context class contains only one constructor for simplification * - * @param field The field to search for method calls. - * @param contextClass The context class. The first constructor of this class is used to search for method calls. - * @param method The method to look for within the context class. + * @param field The field to search for method calls. + * @param contextClass The context class. The first constructor of this class is used to search for method calls. + * @param method The method to look for within the context class. * @return The found method call expression, or null if not found. */ @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; + private static TargetCallInfo findTargetCallIn(@NotNull PsiField field, + @NotNull PsiClass contextClass, + @NotNull PsiMethod method) { + ConstructorMethodInfo constructorMethodInfo = getConstructorMethodInfo(contextClass); + PsiManager psiManager = contextClass.getManager(); + if (field.hasModifierProperty(PsiModifier.FINAL) || + //simplification, that there is no other way to initialize fields + hasOnlyOneInsideCall(constructorMethodInfo)) { + Map methodsByCall = constructorMethodInfo.methods().get(method.getName()); + if (methodsByCall == null) { + return null; + } + for (PsiMethod methodByCall : methodsByCall.keySet()) { + if (psiManager.areElementsEquivalent(methodByCall, method)) { + PsiMethodCallExpression call = methodsByCall.get(methodByCall); + PsiMethod constructor = constructorMethodInfo.constructor(); + if (call != null && constructor != null) { + return new TargetCallInfo(constructor, call) ; + } + } + } } + return null; + } - ConstructorMethodInfo cacheValue = CachedValuesManager.getCachedValue(contextClass, () -> { - PsiMethod context = contextClass.getConstructors()[0]; + @NotNull + private static ConstructorMethodInfo getConstructorMethodInfo(@NotNull PsiClass contextClass) { + return CachedValuesManager.getCachedValue(contextClass, () -> { + CachedValueProvider.Result emptyResult = + CachedValueProvider.Result.create(new ConstructorMethodInfo(Map.of(), null, false), PsiModificationTracker.MODIFICATION_COUNT); + PsiMethod context = findOnlyOneNotChainConstructor(contextClass); + if(context == null) return emptyResult; PsiManager psiManager = context.getManager(); var visitor = new JavaRecursiveElementWalkingVisitor() { - final ConcurrentHashMap> collectedMethods = new ConcurrentHashMap<>(); + final Map> collectedMethods = new HashMap<>(); boolean callsOutside = false; + @Override public void visitLambdaExpression(@NotNull PsiLambdaExpression expression) { } @@ -262,27 +275,43 @@ public final class PlainDescriptor extends PsiVarDescriptor { } }; context.accept(visitor); - return CachedValueProvider.Result.create(new ConstructorMethodInfo(visitor.collectedMethods, visitor.callsOutside), - PsiModificationTracker.MODIFICATION_COUNT); + ConstructorMethodInfo info = new ConstructorMethodInfo(visitor.collectedMethods, context, visitor.callsOutside); + return CachedValueProvider.Result.create(info, PsiModificationTracker.MODIFICATION_COUNT); }); - PsiManager psiManager = contextClass.getManager(); - if (field.hasModifierProperty(PsiModifier.FINAL) || - //simplification, that there is no another way to initialize fields - hasOnlyOneInsideCall(cacheValue)) { - 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 boolean hasOnlyOneInsideCall(ConstructorMethodInfo constructorMethodInfo) { + /** + * Finds the only constructor in the given context class that is not a chain constructor and doesn't contain super call + * + * @param contextClass The context class to search for constructors. + * @return The found constructor, or null if not found. + */ + @Nullable + private static PsiMethod findOnlyOneNotChainConstructor(@NotNull PsiClass contextClass) { + List notChainConstructors = new ArrayList<>(); + for (PsiMethod constructor : contextClass.getConstructors()) { + PsiMethodCallExpression callInConstructor = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(constructor); + if (JavaPsiConstructorUtil.isChainedConstructorCall(callInConstructor)) { + continue; + } + notChainConstructors.add(constructor); + } + if (notChainConstructors.size() != 1) { + return null; + } + PsiMethod targetConstructor = notChainConstructors.get(0); + PsiCodeBlock constructorBody = targetConstructor.getBody(); + + if (constructorBody == null || constructorBody.getStatements().length == 0) { + return null; + } + if (JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(targetConstructor) != null) { + return null; + } + return targetConstructor; + } + + private static boolean hasOnlyOneInsideCall(@NotNull ConstructorMethodInfo constructorMethodInfo) { Map> methods = constructorMethodInfo.methods(); if (constructorMethodInfo.hasCallOutside() || methods.size() != 1) { return false; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java b/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java index 83409c3d0cdc..a13cbb0f1cc4 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java @@ -122,4 +122,98 @@ class OrdinaryClassWithClassQualifier { throw new IllegalArgumentException(); } } +} + +class Outer { + public void main(String[] args) { + Inner inner = new Inner(); + } + void foo() { + System.out.println("Outer"); + } + + class Inner extends Outer { + final String s; + + Inner() { + Outer.this.foo(); + this.s = "hello"; + } + + @Override + void foo() { + System.out.println("Inner"); + System.out.println(s .trim()); + } + } +} + +class SuperTest { + public static void main(String[] args) { + new Child(); + } + public SuperTest() { + init(); + } + + protected void init() { + } + + static class Child extends SuperTest { + String t; + + public Child() { + super(); + check(); + } + + @Override + protected void init() { + t = "init"; + } + + private void check() { + System.out.println(t.length()); + } + } +} + + +class ChainWithOneTarget { + private final String t; + + public ChainWithOneTarget() { + this("test"); + } + + public ChainWithOneTarget(String t) { + test(); + this.t = t; + } + + private void test() { + System.out.println(t.length()); + } +} + +class ChainWithTwoTarget { + private final String t; + + public ChainWithTwoTarget() { + this("test"); + test(); + } + + public ChainWithTwoTarget(String t) { + this.t = t; + } + + public ChainWithTwoTarget(String t, int i) { + this.t = t; + test(); + } + + private void test() { + System.out.println(t.length()); + } } \ No newline at end of file