MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

[java-inspection] IDEA-264141 Warn if compact constructor calls methods that access fields

Browse Source 
- added in DFA logic to check if it is possible to have initialized variable if there is a call in constructor

GitOrigin-RevId: b5b1647df7e4ed116fef095037f48441c36f71a8
This commit is contained in:
Mikhail Pyltsin
2023-11-17 11:27:48 +00:00
committed by intellij-monorepo-bot
parent de0e95a46b
commit f02fd0e658
4 changed files with 204 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java
+113 -2
View File
@@ -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<String, Map<PsiMethod, PsiMethodCallExpression>> 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<String, Map<PsiMethod, PsiMethodCallExpression>> collectedMethods = new ConcurrentHashMap<>();
Ref<Boolean> 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<PsiMethod, PsiMethodCallExpression> 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,
java/java-tests/testData/inspection/dataFlow/fixture/CallsBeforeFieldInitializing.java
+85
View File
@@ -0,0 +1,85 @@
record MyRecord(String name, int id) {
public MyRecord {
validateMyFields();
}
private void validateMyFields() {
if (this.name.<warning descr="Method invocation 'isEmpty' may produce 'NullPointerException'">isEmpty</warning>()) {
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.<warning descr="Method invocation 'isEmpty' may produce 'NullPointerException'">isEmpty</warning>()) {
throw new IllegalArgumentException();
}
}
}
class OrdinaryClassNotNull {
String name;
int id;
public OrdinaryClassNotNull() {
validateMyFields();
name = "";
}
private void validateMyFields() {
if (this.name.<warning descr="Method invocation 'isEmpty' may produce 'NullPointerException'">isEmpty</warning>()) {
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();
}
}
}
java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldNotDuringInitialization.java
+1 -1
View File
@@ -124,6 +124,6 @@ class Test8 {
void other() {
System.out.println(s.hashCode());
System.out.println(s2.<warning descr="Method invocation 'hashCode' may produce 'NullPointerException' (unknown nullability)">hashCode</warning>());
System.out.println(s2.<warning descr="Method invocation 'hashCode' may produce 'NullPointerException'">hashCode</warning>());
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java
+5
View File
@@ -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(); }