[java-dfa] IDEA-361346 Getters should be handled identically to direct field accesses

GitOrigin-RevId: 7e1a03688b341fc211d5f4c98018d04986b8f12b

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/CFGBuilder.java

@@ -193,6 +193,17 @@ public class CFGBuilder {
     return add(new JvmPushInstruction(value, expression == null ? null : new JavaExpressionAnchor(expression)));
   }
 
+  /**
+   * Add a custom null-check
+   * 
+   * @param problem a nullcheck to add
+   * @return this builder
+   */
+  public CFGBuilder nullCheck(NullabilityProblemKind.NullabilityProblem<?> problem) {
+    myAnalyzer.addNullCheck(problem);
+    return this;
+  }
+
   /**
    * Generate instructions to push given DfType on stack.
    * <p>

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java

@@ -410,7 +410,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     finishElement(statement);
   }
 
-  private void addNullCheck(@NotNull PsiExpression expression) {
+  void addNullCheck(@NotNull PsiExpression expression) {
     addNullCheck(NullabilityProblemKind.fromContext(expression, myCustomNullabilityProblems));
   }
 
@@ -1152,8 +1152,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
               PsiPattern patternComponent = components[i];
               PsiMethod accessor = JavaPsiRecordUtil.getAccessorForRecordComponent(recordComponent);
               if (accessor == null) continue;
-              DfaVariableValue accessorDfaVar =
-                getFactory().getVarFactory().createVariableValue(new GetterDescriptor(accessor), patternDfaVar);
+              PsiField field = PropertyUtil.getFieldOfGetter(accessor);
+              VariableDescriptor descriptor = field == null ? new GetterDescriptor(accessor) : new PlainDescriptor(field);
+              DfaVariableValue accessorDfaVar = getFactory().getVarFactory().createVariableValue(descriptor, patternDfaVar);
               addInstruction(new JvmPushInstruction(accessorDfaVar, null));
               processPattern(sourcePattern, patternComponent, substitutor.substitute(recordComponent.getType()), null, endPatternOffset);
             }
@@ -2504,11 +2505,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
     startElement(expression);
 
     final PsiExpression qualifierExpression = expression.getQualifierExpression();
-    if (qualifierExpression != null) {
-      if (!(expression.resolve() instanceof PsiMember member) || !member.hasModifierProperty(PsiModifier.STATIC)) {
-        qualifierExpression.accept(this);
-        addInstruction(new PopInstruction());
-      }
+    if (qualifierExpression != null && !(qualifierExpression instanceof PsiReferenceExpression ref && ref.resolve() instanceof PsiClass)) {
+      qualifierExpression.accept(this);
+      addInstruction(new PopInstruction());
     }
 
     // complex assignments (e.g. "|=") are both reading and writing
@@ -2738,7 +2737,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
   private static final CallInliner[] INLINERS = {
     new AssertJInliner(), new OptionalChainInliner(), new LambdaInliner(), new CollectionUpdateInliner(),
     new StreamChainInliner(), new MapUpdateInliner(), new AssumeInliner(), new ClassMethodsInliner(),
-    new AssertAllInliner(), new BoxingInliner(), new SimpleMethodInliner(),
+    new AssertAllInliner(), new BoxingInliner(), new SimpleMethodInliner(), new AccessorInliner(),
     new TransformInliner(), new EnumCompareInliner(), new IndexOfInliner(), new AssertInstanceOfInliner()
   };
 }

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java

@@ -165,6 +165,11 @@ public final class JavaDfaValueFactory {
         }
       }
     }
+    qualifierExpression = PsiUtil.skipParenthesizedExprDown(qualifierExpression);
+    if (qualifierExpression instanceof PsiTypeCastExpression castExpression &&
+        castExpression.getType() instanceof PsiClassType) {
+      qualifierExpression = castExpression.getOperand();
+    }
     return getQualifierValue(factory, qualifierExpression);
   }
 

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java

@@ -0,0 +1,66 @@
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+package com.intellij.codeInspection.dataFlow.java.inliner;
+
+import com.intellij.codeInsight.Nullability;
+import com.intellij.codeInsight.NullabilityAnnotationInfo;
+import com.intellij.codeInsight.NullableNotNullManager;
+import com.intellij.codeInspection.dataFlow.NullabilityProblemKind;
+import com.intellij.codeInspection.dataFlow.java.CFGBuilder;
+import com.intellij.codeInspection.dataFlow.java.JavaDfaValueFactory;
+import com.intellij.codeInspection.dataFlow.jvm.descriptors.GetterDescriptor;
+import com.intellij.codeInspection.dataFlow.jvm.descriptors.PlainDescriptor;
+import com.intellij.codeInspection.dataFlow.value.DfaValue;
+import com.intellij.codeInspection.util.OptionalUtil;
+import com.intellij.psi.*;
+import com.intellij.psi.util.PropertyUtil;
+import com.intellij.psi.util.PsiUtil;
+import com.intellij.psi.util.TypeConversionUtil;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Inlines accessors to read fields directly
+ */
+public final class AccessorInliner implements CallInliner {
+  @Override
+  public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) {
+    PsiMethod method = call.resolveMethod();
+    if (method == null) return false;
+    if (PsiUtil.canBeOverridden(method)) return false;
+    PsiClass containingClass = method.getContainingClass();
+    if (containingClass != null) {
+      String qualifiedName = containingClass.getQualifiedName();
+      // Methods Enum.name() and Enum.ordinal() are handled especially
+      if (CommonClassNames.JAVA_LANG_ENUM.equals(qualifiedName)) return false;
+      // Unboxing calls like Boolean.booleanValue() are handled especially
+      if (qualifiedName != null && TypeConversionUtil.isPrimitiveWrapper(qualifiedName)) return false;
+      // Avoid inlining OptionalInt.isPresent(), etc.
+      if (OptionalUtil.isJdkOptionalClassName(qualifiedName)) return false;
+      // Known stable methods (like methods from reflection) may read non-final fields,
+      // so inlining them breaks the stability
+      if (GetterDescriptor.isKnownStableMethod(method)) return false;
+    }
+    PsiField field = PropertyUtil.getFieldOfGetter(method);
+    if (field == null) return false;
+    DfaValue value = JavaDfaValueFactory.getQualifierOrThisValue(builder.getFactory(), call.getMethodExpression());
+    if (value == null) return false;
+    NullableNotNullManager manager = NullableNotNullManager.getInstance(method.getProject());
+    NullabilityAnnotationInfo methodNullability = manager.findEffectiveNullabilityInfo(method);
+    NullabilityAnnotationInfo fieldNullability = manager.findEffectiveNullabilityInfo(field);
+    if (methodNullability != null && methodNullability.getNullability() == Nullability.NULLABLE &&
+        (fieldNullability == null || fieldNullability.getNullability() != Nullability.NULLABLE)) {
+      // Avoid inlining if getter is marked as nullable, while the field is not.
+      // In this rare case, we cannot preserve the nullability warning on the callsite.
+      return false;
+    }
+    boolean nonNull = methodNullability != null && methodNullability.getNullability() == Nullability.NOT_NULL && !methodNullability.isInferred();
+    PsiExpression qualifier = call.getMethodExpression().getQualifierExpression();
+    if (qualifier != null && !(qualifier instanceof PsiReferenceExpression ref && ref.resolve() instanceof PsiClass)) {
+      builder.pushExpression(qualifier).pop();
+    }
+    builder.push(new PlainDescriptor(field).createValue(builder.getFactory(), value), call);
+    if (nonNull) {
+      builder.nullCheck(NullabilityProblemKind.assumeNotNull.problem(call, call));
+    }
+    return true;
+  }
+}

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java

@@ -45,7 +45,7 @@ public final class GetterDescriptor extends PsiVarDescriptor {
 
   public GetterDescriptor(@NotNull PsiMethod getter) {
     myGetter = getter;
-    if (STABLE_METHODS.methodMatches(getter) || getter instanceof LightRecordMethod) {
+    if (isKnownStableMethod(getter) || getter instanceof LightRecordMethod) {
       myStable = true;
     }
     else {
@@ -54,6 +54,10 @@ public final class GetterDescriptor extends PsiVarDescriptor {
     }
   }
 
+  public static boolean isKnownStableMethod(@NotNull PsiMethod getter) {
+    return STABLE_METHODS.methodMatches(getter);
+  }
+
   @NotNull
   @Override
   public String toString() {

java/java-analysis-impl/src/com/intellij/psi/util/PropertyUtil.java

@@ -6,6 +6,8 @@ import com.intellij.lang.java.beans.PropertyKind;
 import com.intellij.lang.jvm.JvmModifier;
 import com.intellij.psi.*;
 import com.intellij.psi.impl.JavaSimplePropertyGistKt;
+import com.intellij.psi.impl.compiled.ClsMethodImpl;
+import com.intellij.psi.impl.light.LightRecordMethod;
 import com.intellij.psi.impl.source.PsiMethodImpl;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
@@ -39,8 +41,12 @@ public final class PropertyUtil extends PropertyUtilBase {
   private static @Nullable PsiField getFieldImpl(@NotNull PsiMethod method,
                                                  @NotNull Supplier<? extends PsiExpression> returnExprSupplier,
                                                  boolean useIndex) {
+    if (method instanceof LightRecordMethod) {
+      PsiRecordComponent component = JavaPsiRecordUtil.getRecordComponentForAccessor(method);
+      return component == null ? null : JavaPsiRecordUtil.getFieldForComponent(component);
+    }
     if (useIndex) {
-      if (PsiUtil.preferCompiledElement(method) instanceof PsiMethod compiledMethod) {
+      if (PsiUtil.preferCompiledElement(method) instanceof ClsMethodImpl compiledMethod) {
         return ProjectBytecodeAnalysis.getInstance(method.getProject()).findFieldForGetter(compiledMethod);
       }
       if (method instanceof PsiMethodImpl && method.isPhysical()) {

java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccess.java

@@ -0,0 +1,84 @@
+import java.util.*;
+import org.jetbrains.annotations.*;
+
+class Test {
+  record MyRecord(int value) {}
+  
+  void testRecord(MyRecord record) {
+    if (record.value() == 0) {
+      if (<warning descr="Condition 'record.value == 0' is always 'true'">record.value == 0</warning>) {}
+    }
+  }
+  
+  void testPoint(Point p1, Point p2) {
+    if (p1.x == p2.x) {
+      if (<warning descr="Condition 'p1.getX() != p2.getX()' is always 'false'">p1.getX() != p2.getX()</warning>) {
+        
+      }
+      if (p1.getXBoxed() == null) {}
+      if (p1.getXBoxed() != p2.getXBoxed()) {
+        
+      }
+    }
+    p2.y = 5;
+    if (<warning descr="Condition 'p2.getY() > 0' is always 'true'">p2.getY() > 0</warning>) {}
+  }
+  
+  void testWithNullity(WithNullity w) {
+    if (<warning descr="Condition 'w.getS() == null' is always 'false'">w.getS() == null</warning>) {
+    }
+    // Not inlined, because method is declared as nullable, while field is not
+    System.out.println(w.getS2().<warning descr="Method invocation 'trim' may produce 'NullPointerException'">trim</warning>());
+    // Inlined, nullability is taken from the field (optional warning inside the method)
+    System.out.println(w.getS3().<warning descr="Method invocation 'trim' may produce 'NullPointerException'">trim</warning>());
+    // Inlined, not-null nullability is forced by method declaration (but we have a warning inside the method)
+    System.out.println(w.getS4().trim());
+  }
+  
+  static final class WithNullity {
+    String s;
+    String s2;
+    @Nullable String s3;
+    @Nullable String s4;
+    
+    @NotNull
+    String getS() {
+      return s;
+    }
+    
+    @Nullable
+    String getS2() {
+      return s2;
+    }
+    
+    String getS3() {
+      return <warning descr="Expression 's3' might evaluate to null but is returned by the method which is not declared as @Nullable">s3</warning>;
+    }
+
+    @NotNull
+    String getS4() {
+      return <warning descr="Expression 's4' might evaluate to null but is returned by the method declared as @NotNull">s4</warning>;
+    }
+  }
+  
+  static final class Point {
+    int x, y;
+    
+    Point(int x, int y) {
+      this.x = x;
+      this.y = y;
+    }
+    
+    int getX() {
+      return x;
+    }
+    
+    int getY() {
+      return y;
+    }
+    
+    Integer getXBoxed() { // Boxing should not be supported intentionally
+      return x;
+    }
+  }
+}

java/java-tests/testData/inspection/dataFlow/fixture/InstanceofFromObjectToPrimitive.java

@@ -111,6 +111,16 @@ public class InstanceofFromObjectToPrimitive {
         }
     }
 
+    private static void testDirectFieldAccess() {
+        LongRecord o = new LongRecord(1L);
+        if (o.o == null) {
+            return;
+        }
+        if (<warning descr="Condition 'o instanceof LongRecord(long a)' is always 'true'">o instanceof LongRecord(long a)</warning>) { //true
+            System.out.println("long");
+        }
+    }
+
     private static void testIntegerRecordNotNull() {
         IntegerRecord o = new IntegerRecord(1);
         if (o.o() == null) {

java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java

@@ -144,4 +144,5 @@ public class DataFlowInspection21Test extends DataFlowInspectionTestCase {
   public void testClassFileGetter() {
     doTest();
   }
+  public void testGetterVsDirectAccess() { doTest(); }
 }

java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java

@@ -22,6 +22,7 @@ import org.junit.platform.suite.api.Suite;
   DataFlowInspectionHeavyTest.class,
   DataFlowInspectionAncientTest.class,
   DataFlowInspectionCancellingTest.class,
+  DataFlowInspectionPrimitivesInPatternsTest.class,
   ContractCheckTest.class,
   HardcodedContractsTest.class,
   DataFlowRangeAnalysisTest.class,

plugins/lombok/testData/highlighting/LombokBasics.java

@@ -34,7 +34,7 @@ final class <warning descr="Class 'Foo' is never used">Foo</warning> {
 
   public void <warning descr="Method 'test()' is never used">test</warning>() {
     bar = null;
-    System.out.println(getBar().trim());
+    System.out.println(getBar().<warning descr="Method invocation 'trim' will produce 'NullPointerException'">trim</warning>());
   }
 }
 class <warning descr="Class 'Outer' is never used">Outer</warning> {