From e8a61b484599ff092bb056b0ad32c34f8b317785 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 24 Oct 2024 10:55:46 +0200 Subject: [PATCH] [java-analysis] IDEA-361307 Provide index to map getter to the field in compiled code GitOrigin-RevId: 8cf9a9628e922aae75c1366cde4150cf820d023f --- .../BytecodeAnalysisIndex.java | 9 ++++ .../bytecodeAnalysis/ClassDataIndexer.java | 43 +++++++++++++++++-- .../codeInspection/bytecodeAnalysis/Data.java | 12 +++++- .../bytecodeAnalysis/Direction.java | 3 +- .../ProjectBytecodeAnalysis.java | 17 ++++++++ .../com/intellij/psi/util/PropertyUtil.java | 19 ++++++-- .../dataFlow/fixture/ClassFileGetter.java | 18 ++++++++ .../DataFlowInspection21Test.java | 4 ++ .../BytecodeAccessorTest.java | 32 ++++++++++++++ .../BytecodeAnalysisTest.java | 2 +- 10 files changed, 149 insertions(+), 10 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ClassFileGetter.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAccessorTest.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java index fd2de7cb5bbf..fb93addade4d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/BytecodeAnalysisIndex.java @@ -186,6 +186,12 @@ public final class BytecodeAnalysisIndex extends ScalarIndexExtension { } writeDataValue(out, effects.returnValue); } + else if (rhs instanceof FieldAccess fieldAccess) { + out.writeUTF(fieldAccess.name()); + } + else { + throw new UnsupportedOperationException("Unsupported result: " + rhs + " in " + eqs); + } } } @@ -205,6 +211,9 @@ public final class BytecodeAnalysisIndex extends ScalarIndexExtension { DataValue returnValue = readDataValue(in); results.add(new DirectionResultPair(directionKey, new Effects(returnValue, Set.copyOf(effects)))); } + else if (direction == Direction.Access) { + results.add(new DirectionResultPair(directionKey, new FieldAccess(in.readUTF()))); + } else { boolean isFinal = in.readBoolean(); // flag if (isFinal) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java index 75de9e9f6e76..fb82b2b0411a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ClassDataIndexer.java @@ -22,7 +22,7 @@ import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.org.objectweb.asm.*; -import org.jetbrains.org.objectweb.asm.tree.MethodNode; +import org.jetbrains.org.objectweb.asm.tree.*; import org.jetbrains.org.objectweb.asm.tree.analysis.AnalyzerException; import java.io.DataOutputStream; @@ -53,7 +53,7 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator MERGER = (eq1, eq2) -> eq1.equals(eq2) ? eq1 : new Equations(Collections.emptyList(), false); - private static final int VERSION = 16; // change when inference algorithm changes + private static final int VERSION = 17; // change when inference algorithm changes private static final int VERSION_MODIFIER = HardCodedPurity.AGGRESSIVE_HARDCODED_PURITY ? 1 : 0; private static final int FINAL_VERSION = VERSION * 2 + VERSION_MODIFIER; private static final VirtualFileGist> ourGist = GistManager.getInstance().newVirtualFileGist( @@ -199,7 +199,7 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator processClass(final ClassReader classReader, final String presentableUrl) { + static Map processClass(final ClassReader classReader, final String presentableUrl) { // It is OK to share pending states, actions and results for analyses. // Analyses are designed in such a way that they first write to states/actions/results and then read only those portion @@ -641,6 +641,9 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator result) throws AnalyzerException { Set fieldsToTrack = method.methodName.equals("") ? myStaticFinalFields : Collections.emptySet(); + if (argumentTypes.length == 0 && !Type.VOID_TYPE.equals(returnType)) { + ContainerUtil.addIfNotNull(result, getterEquation(method, graph, stable)); + } CombinedAnalysis analyzer = new CombinedAnalysis(method, graph, fieldsToTrack); analyzer.analyze(); ContainerUtil.addIfNotNull(result, analyzer.outContractEquation(stable)); @@ -667,6 +670,40 @@ public class ClassDataIndexer implements VirtualFileGist.GistCalculator GETSTATIC + xRETURN + // !isStatic -> ALOAD_0 + GETFIELD + xRETURN + if (!isStatic) { + if (!(instructions.get(0) instanceof VarInsnNode varAccess) || + varAccess.getOpcode() != Opcodes.ALOAD || + varAccess.var != 0) { + return null; + } + } + if (!(instructions.get(shift) instanceof FieldInsnNode fieldAccess) || + fieldAccess.getOpcode() != (isStatic ? Opcodes.GETSTATIC : Opcodes.GETFIELD) || + !fieldAccess.owner.equals(className)) { + return null; + } + String name = fieldAccess.name; + if (!isReturn(instructions.get(1 + shift))) return null; + return new Equation(new EKey(method, Access, stable), new FieldAccess(name)); + } + + private static boolean isReturn(AbstractInsnNode insn) { + int opcode = insn.getOpcode(); + return opcode == Opcodes.ARETURN || opcode == Opcodes.DRETURN || + opcode == Opcodes.FRETURN || opcode == Opcodes.IRETURN || + opcode == Opcodes.LRETURN; + } + private void storeStaticFieldEquations(CombinedAnalysis analyzer) { for (Equation equation : analyzer.staticFieldEquations()) { myEquations.put(equation.key, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Data.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Data.java index a89331bd0ddc..1c1f53f679bc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Data.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Data.java @@ -129,7 +129,7 @@ class Equations { } } -class DirectionResultPair { +final class DirectionResultPair { final int directionKey; @NotNull final Result result; @@ -159,7 +159,7 @@ class DirectionResultPair { } } -interface Result { +sealed interface Result permits Effects, FieldAccess, Pending, Value { /** * @return a stream of keys which should be solved to make this result final */ @@ -171,6 +171,14 @@ interface Result { } } +/** + * A result for the {@link Direction#Access} direction: + * for setter/constructor parameter: unconditional field set; + * for method: unconditional field return + * @param name name of the field + */ +record FieldAccess(String name) implements Result {} + final class Pending implements Result { final Component @NotNull [] delta; // sum diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Direction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Direction.java index 2e1e2ef1ab85..91f175d70dd4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Direction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/Direction.java @@ -10,10 +10,11 @@ public abstract class Direction { public static final Direction Out = explicitDirection("Out"); public static final Direction NullableOut = explicitDirection("NullableOut"); public static final Direction Pure = explicitDirection("Pure"); + public static final Direction Access = explicitDirection("Access"); public static final Direction Throw = explicitDirection("Throw"); public static final Direction Volatile = explicitDirection("Volatile"); - private static final List ourConcreteDirections = Arrays.asList(Out, NullableOut, Pure, Throw, Volatile); + private static final List ourConcreteDirections = Arrays.asList(Out, NullableOut, Pure, Access, Throw, Volatile); private static final int CONCRETE_DIRECTIONS_OFFSET = ourConcreteDirections.size(); private static final int IN_OUT_OFFSET = 2; // nullity mask is 0/1 private static final int IN_THROW_OFFSET = 2 + Value.values().length; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java index 7ed6035d008b..756b135d72d4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/bytecodeAnalysis/ProjectBytecodeAnalysis.java @@ -69,6 +69,23 @@ public class ProjectBytecodeAnalysis { nullableMethodTransitivity = Registry.is(NULLABLE_METHOD_TRANSITIVITY); } + /** + * @param getter getter method + * @return field that this method reads and returns; null if the method is not identified as a getter + */ + public @Nullable PsiField findFieldForGetter(@NotNull PsiMethod getter) { + EKey eKey = getKey(getter); + if (eKey == null) return null; + EKey accessKey = myEquationProvider.adaptKey(eKey.withDirection(Access)); + for (Equations equation : myEquationProvider.getEquations(accessKey.member)) { + if (equation.find(Access).orElse(null) instanceof FieldAccess access) { + PsiClass containingClass = getter.getContainingClass(); + return containingClass != null ? containingClass.findFieldByName(access.name(), false) : null; + } + } + return null; + } + @Nullable public PsiAnnotation findInferredAnnotation(@NotNull PsiModifierListOwner listOwner, @NotNull String annotationFQN) { if (!(listOwner instanceof PsiCompiledElement)) { 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 b5697aa0c23f..9ebd8dfa1bae 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 @@ -1,6 +1,7 @@ // 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.psi.util; +import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; import com.intellij.lang.java.beans.PropertyKind; import com.intellij.lang.jvm.JvmModifier; import com.intellij.psi.*; @@ -29,14 +30,26 @@ public final class PropertyUtil extends PropertyUtilBase { @Nullable public static PsiField getFieldOfGetter(PsiMethod method, Supplier returnExprSupplier, boolean useIndex) { - PsiField field = useIndex && method instanceof PsiMethodImpl && method.isPhysical() - ? JavaSimplePropertyGistKt.getFieldOfGetter(method) - : getSimplyReturnedField(returnExprSupplier.get()); + PsiField field = getFieldImpl(method, returnExprSupplier, useIndex); if (field == null || !checkFieldLocation(method, field)) return null; final PsiType returnType = method.getReturnType(); return returnType != null && field.getType().equals(returnType) ? field : null; } + private static @Nullable PsiField getFieldImpl(@NotNull PsiMethod method, + @NotNull Supplier returnExprSupplier, + boolean useIndex) { + if (useIndex) { + if (PsiUtil.preferCompiledElement(method) instanceof PsiMethod compiledMethod) { + return ProjectBytecodeAnalysis.getInstance(method.getProject()).findFieldForGetter(compiledMethod); + } + if (method instanceof PsiMethodImpl && method.isPhysical()) { + return JavaSimplePropertyGistKt.getFieldOfGetter(method); + } + } + return getSimplyReturnedField(returnExprSupplier.get()); + } + public static boolean isSimpleGetter(@Nullable PsiMethod method) { //noinspection TestOnlyProblems return isSimpleGetter(method, true); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ClassFileGetter.java b/java/java-tests/testData/inspection/dataFlow/fixture/ClassFileGetter.java new file mode 100644 index 000000000000..9ce2d53cc656 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ClassFileGetter.java @@ -0,0 +1,18 @@ +import java.util.*; + +class Test { + void testVersion() { + Runtime runtime = Runtime.getRuntime(); + Runtime.Version version = runtime.version(); + if (version.build().isPresent()) { + unknown(); + if (version.build().isPresent()) { + } + if (runtime == Runtime.getRuntime()) { + + } + } + } + + native void unknown(); +} \ No newline at end of file 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 d3d134ea0249..caf21d2cf2cc 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java @@ -140,4 +140,8 @@ public class DataFlowInspection21Test extends DataFlowInspectionTestCase { addJetBrainsNotNullByDefault(myFixture); doTest(); } + + public void testClassFileGetter() { + doTest(); + } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAccessorTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAccessorTest.java new file mode 100644 index 000000000000..5a7e3f41ffab --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAccessorTest.java @@ -0,0 +1,32 @@ +// 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.java.codeInspection.bytecodeAnalysis; + +import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; +import com.intellij.psi.*; +import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; +import org.jetbrains.annotations.NotNull; + +public final class BytecodeAccessorTest extends LightJavaCodeInsightFixtureTestCase { + public void testAccessors() { + PsiClass psiClass = + JavaPsiFacade.getInstance(getProject()).findClass("java.util.OptionalInt", GlobalSearchScope.allScope(getProject())); + assertNotNull(psiClass); + assertInstanceOf(psiClass, PsiCompiledElement.class); + PsiMethod isPresentMethod = psiClass.findMethodsByName("isPresent", false)[0]; + assertFalse(isPresentMethod.hasModifierProperty(PsiModifier.STATIC)); + PsiMethod emptyMethod = psiClass.findMethodsByName("empty", false)[0]; + assertTrue(emptyMethod.hasModifierProperty(PsiModifier.STATIC)); + ProjectBytecodeAnalysis analysis = ProjectBytecodeAnalysis.getInstance(getProject()); + PsiField isPresentField = analysis.findFieldForGetter(isPresentMethod); + assertNotNull(isPresentField); + PsiField emptyField = analysis.findFieldForGetter(emptyMethod); + assertNotNull(emptyField); + } + + @Override + protected @NotNull LightProjectDescriptor getProjectDescriptor() { + return JAVA_21; + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAnalysisTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAnalysisTest.java index 52f76901c603..414bc92fcb69 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAnalysisTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/bytecodeAnalysis/BytecodeAnalysisTest.java @@ -169,7 +169,7 @@ public class BytecodeAnalysisTest extends LightJavaCodeInsightFixtureTestCase { fail(message + ": @NotNull inferred, but not expected"); } } - + private void checkCompoundIds(String className) throws IOException { GlobalSearchScope scope = GlobalSearchScope.moduleWithLibrariesScope(getModule()); PsiClass psiClass = JavaPsiFacade.getInstance(getProject()).findClass(PACKAGE_NAME + '.' + className, scope);