diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/CFGBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/CFGBuilder.java index a0cafc29f873..1c88cccf17a3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/CFGBuilder.java +++ b/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. *

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java index a2e77b1ec866..9686f53526c8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java +++ b/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() }; } \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java index f099647f61cc..0a21157e50d6 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java +++ b/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); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java new file mode 100644 index 000000000000..7ec47285363e --- /dev/null +++ b/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; + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java index e70cd43b8334..7e4db560a32c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java +++ b/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() { diff --git a/java/java-analysis-impl/src/com/intellij/psi/util/PropertyUtil.java b/java/java-analysis-impl/src/com/intellij/psi/util/PropertyUtil.java index 9ebd8dfa1bae..ddd1453ea6e7 100644 --- a/java/java-analysis-impl/src/com/intellij/psi/util/PropertyUtil.java +++ b/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 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()) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccess.java b/java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccess.java new file mode 100644 index 000000000000..a6d2f5126363 --- /dev/null +++ b/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 (record.value == 0) {} + } + } + + void testPoint(Point p1, Point p2) { + if (p1.x == p2.x) { + if (p1.getX() != p2.getX()) { + + } + if (p1.getXBoxed() == null) {} + if (p1.getXBoxed() != p2.getXBoxed()) { + + } + } + p2.y = 5; + if (p2.getY() > 0) {} + } + + void testWithNullity(WithNullity w) { + if (w.getS() == null) { + } + // Not inlined, because method is declared as nullable, while field is not + System.out.println(w.getS2().trim()); + // Inlined, nullability is taken from the field (optional warning inside the method) + System.out.println(w.getS3().trim()); + // 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 s3; + } + + @NotNull + String getS4() { + return s4; + } + } + + 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; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceofFromObjectToPrimitive.java b/java/java-tests/testData/inspection/dataFlow/fixture/InstanceofFromObjectToPrimitive.java index 0d2a88e9a5bd..a1abdaedd79a 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceofFromObjectToPrimitive.java +++ b/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 (o instanceof LongRecord(long a)) { //true + System.out.println("long"); + } + } + private static void testIntegerRecordNotNull() { IntegerRecord o = new IntegerRecord(1); if (o.o() == null) { diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java index caf21d2cf2cc..853c40c86a22 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java +++ b/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(); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java index ef3ff4e2a434..050c6a92af7d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java +++ b/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, diff --git a/plugins/lombok/testData/highlighting/LombokBasics.java b/plugins/lombok/testData/highlighting/LombokBasics.java index 242859ad4d8e..c82beebd8a3c 100644 --- a/plugins/lombok/testData/highlighting/LombokBasics.java +++ b/plugins/lombok/testData/highlighting/LombokBasics.java @@ -34,7 +34,7 @@ final class Foo { public void test() { bar = null; - System.out.println(getBar().trim()); + System.out.println(getBar().trim()); } } class Outer {