From 5379b771ca4b474714a028b1e2865f500bbff189 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 18 Jul 2014 15:12:01 +0200 Subject: [PATCH] turn off contract inference for overrideable methods; hopefully, not forever (IDEA-127518) --- .../InferredAnnotationsManagerImpl.java | 31 ++++++++++++------- .../ContractInferenceBewareOverriding.java | 21 +++++++++++++ .../fixture/UseInferredContracts.java | 2 +- .../ContractInferenceFromSourceTest.groovy | 2 +- .../DataFlowInspectionTest.java | 1 + 5 files changed, 44 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ContractInferenceBewareOverriding.java diff --git a/java/java-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java b/java/java-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java index e9536e43abbb..3cd036db6ed1 100644 --- a/java/java-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java @@ -17,17 +17,20 @@ package com.intellij.codeInsight; import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; import com.intellij.codeInspection.dataFlow.ContractInference; -import com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer; import com.intellij.codeInspection.dataFlow.MethodContract; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.PsiAnnotation; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiModifierListOwner; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.List; +import static com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT; + public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { @Nullable @Override @@ -37,7 +40,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { return fromBytecode; } - if (listOwner instanceof PsiMethod && ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN)) { + if (listOwner instanceof PsiMethod && ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN) && !PsiUtil.canBeOverriden((PsiMethod)listOwner)) { List contracts = ContractInference.inferContracts((PsiMethod)listOwner); if (!contracts.isEmpty()) { return ProjectBytecodeAnalysis.getInstance(listOwner.getProject()).createContractAnnotation("\"" + StringUtil.join(contracts, "; ") + "\""); @@ -50,19 +53,25 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { @NotNull @Override public PsiAnnotation[] findInferredAnnotations(@NotNull PsiModifierListOwner listOwner) { + List result = ContainerUtil.newArrayList(); PsiAnnotation[] fromBytecode = ProjectBytecodeAnalysis.getInstance(listOwner.getProject()).findInferredAnnotations(listOwner); - if (fromBytecode.length > 0) { - return fromBytecode; - } - - if (listOwner instanceof PsiMethod) { - List contracts = ContractInference.inferContracts((PsiMethod)listOwner); - if (!contracts.isEmpty()) { - return new PsiAnnotation[]{ProjectBytecodeAnalysis.getInstance(listOwner.getProject()).createContractAnnotation("\"" + StringUtil.join(contracts, "; ") + "\"")}; + for (PsiAnnotation annotation : fromBytecode) { + if (!ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotation.getQualifiedName()) || + !(listOwner instanceof PsiMethod) || + !PsiUtil.canBeOverriden((PsiMethod)listOwner)) { + result.add(annotation); } } - return PsiAnnotation.EMPTY_ARRAY; + if (listOwner instanceof PsiMethod && !PsiUtil.canBeOverriden((PsiMethod)listOwner)) { + List contracts = ContractInference.inferContracts((PsiMethod)listOwner); + if (!contracts.isEmpty()) { + result.add(ProjectBytecodeAnalysis.getInstance(listOwner.getProject()) + .createContractAnnotation("\"" + StringUtil.join(contracts, "; ") + "\"")); + } + } + + return result.isEmpty() ? PsiAnnotation.EMPTY_ARRAY : result.toArray(new PsiAnnotation[result.size()]); } @Override diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractInferenceBewareOverriding.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractInferenceBewareOverriding.java new file mode 100644 index 000000000000..a1d263f778f6 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractInferenceBewareOverriding.java @@ -0,0 +1,21 @@ +import org.jetbrains.annotations.Nullable; + +class Doo { + + boolean isMaybeNotNull(@Nullable Object o) { + return o != null; + } + + void foo(@Nullable String s) { + if (isMaybeNotNull(s)) { + System.out.println(s.length()); + } + } + +} + +class DooImpl extends Doo { + boolean isMaybeNotNull(@Nullable Object o) { + return hashCode() == 42; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/UseInferredContracts.java b/java/java-tests/testData/inspection/dataFlow/fixture/UseInferredContracts.java index 0c5d8a6e0436..34b9d708a4b2 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/UseInferredContracts.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/UseInferredContracts.java @@ -2,7 +2,7 @@ import org.jetbrains.annotations.Nullable; class Doo { - boolean isNotNull(@Nullable Object o) { + static boolean isNotNull(@Nullable Object o) { return o != null; } diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy index f4f1acda8684..8656101155a3 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ContractInferenceFromSourceTest.groovy @@ -277,7 +277,7 @@ class ContractInferenceFromSourceTest extends LightCodeInsightFixtureTestCase { } private List inferContracts(String method) { - def clazz = myFixture.addClass("class Foo { $method }") + def clazz = myFixture.addClass("final class Foo { $method }") return ContractInference.inferContracts(clazz.methods[0]).collect { it as String } } } diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 915bfa078f11..4cba2ab918a7 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -251,6 +251,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testRootThrowableCause() { doTest(); } public void testUseInferredContracts() { doTest(); } + public void testContractInferenceBewareOverriding() { doTest(); } public void testNumberComparisonsWhenValueIsKnown() { doTest(); }