extract contract checking to a separatate inspection

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

@@ -0,0 +1,97 @@
+/*
+ * Copyright 2000-2014 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.intellij.codeInspection.dataFlow;
+
+import com.intellij.codeInsight.AnnotationUtil;
+import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool;
+import com.intellij.codeInspection.ProblemsHolder;
+import com.intellij.openapi.util.text.StringUtil;
+import com.intellij.psi.*;
+import com.intellij.psi.util.PsiTreeUtil;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ * @author peter
+ */
+public class ContractInspection extends BaseJavaBatchLocalInspectionTool {
+
+  @Override
+  @NotNull
+  public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, final boolean isOnTheFly) {
+    return new JavaElementVisitor() {
+
+      @Override
+      public void visitMethod(PsiMethod method) {
+        for (MethodContract contract : ControlFlowAnalyzer.getMethodContracts(method)) {
+          Map<PsiElement, String> errors = ContractChecker.checkContractClause(method, contract, false, isOnTheFly);
+          for (Map.Entry<PsiElement, String> entry : errors.entrySet()) {
+            PsiElement element = entry.getKey();
+            holder.registerProblem(element, entry.getValue());
+          }
+        }
+      }
+
+      @Override
+      public void visitAnnotation(PsiAnnotation annotation) {
+        if (!ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotation.getQualifiedName())) return;
+
+        PsiMethod method = PsiTreeUtil.getParentOfType(annotation, PsiMethod.class);
+        if (method == null) return;
+
+        String text = AnnotationUtil.getStringAttributeValue(annotation, null);
+        if (StringUtil.isNotEmpty(text)) {
+          String error = checkContract(method, text);
+          if (error != null) {
+            PsiAnnotationMemberValue value = annotation.findAttributeValue(null);
+            assert value != null;
+            holder.registerProblem(value, error);
+            return;
+          }
+        }
+
+        if (Boolean.TRUE.equals(AnnotationUtil.getBooleanAttributeValue(annotation, "pure")) &&
+            PsiType.VOID.equals(method.getReturnType())) {
+          PsiAnnotationMemberValue value = annotation.findDeclaredAttributeValue("pure");
+          assert value != null;
+          holder.registerProblem(value, "Pure methods must return something, void is not allowed as a return type");
+        }
+      }
+    };
+  }
+
+  @Nullable
+  public static String checkContract(PsiMethod method, String text) {
+    List<MethodContract> contracts;
+    try {
+      contracts = MethodContract.parseContract(text);
+    }
+    catch (MethodContract.ParseException e) {
+      return e.getMessage();
+    }
+    int paramCount = method.getParameterList().getParametersCount();
+    for (int i = 0; i < contracts.size(); i++) {
+      MethodContract contract = contracts.get(i);
+      if (contract.arguments.length != paramCount) {
+        return "Method takes " + paramCount + " parameters, while contract clause number " + (i + 1) + " expects " + contract.arguments.length;
+      }
+    }
+    return null;
+  }
+}

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

@@ -95,13 +95,6 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
       @Override
       public void visitMethod(PsiMethod method) {
         analyzeCodeBlock(method.getBody(), holder, isOnTheFly);
-
-        for (MethodContract contract : ControlFlowAnalyzer.getMethodContracts(method)) {
-          Map<PsiElement, String> errors = ContractChecker.checkContractClause(method, contract, IGNORE_ASSERT_STATEMENTS, isOnTheFly);
-          for (Map.Entry<PsiElement, String> entry : errors.entrySet()) {
-            holder.registerProblem(entry.getKey(), entry.getValue());
-          }
-        }
       }
 
       @Override
@@ -119,53 +112,9 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool {
         }
       }
 
-      @Override
-      public void visitAnnotation(PsiAnnotation annotation) {
-        if (!ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotation.getQualifiedName())) return;
-
-        PsiMethod method = PsiTreeUtil.getParentOfType(annotation, PsiMethod.class);
-        if (method == null) return;
-
-        String text = AnnotationUtil.getStringAttributeValue(annotation, null);
-        if (StringUtil.isNotEmpty(text)) {
-          String error = checkContract(method, text);
-          if (error != null) {
-            PsiAnnotationMemberValue value = annotation.findAttributeValue(null);
-            assert value != null;
-            holder.registerProblem(value, error);
-            return;
-          }
-        }
-
-        if (Boolean.TRUE.equals(AnnotationUtil.getBooleanAttributeValue(annotation, "pure")) && 
-            PsiType.VOID.equals(method.getReturnType())) {
-          PsiAnnotationMemberValue value = annotation.findDeclaredAttributeValue("pure");
-          assert value != null;
-          holder.registerProblem(value, "Pure methods must return something, void is not allowed as a return type");
-        }
-      }
     };
   }
 
-  @Nullable
-  public static String checkContract(PsiMethod method, String text) {
-    List<MethodContract> contracts;
-    try {
-      contracts = MethodContract.parseContract(text);
-    }
-    catch (MethodContract.ParseException e) {
-      return e.getMessage();
-    }
-    int paramCount = method.getParameterList().getParametersCount();
-    for (int i = 0; i < contracts.size(); i++) {
-      MethodContract contract = contracts.get(i);
-      if (contract.arguments.length != paramCount) {
-        return "Method takes " + paramCount + " parameters, while contract clause number " + (i + 1) + " expects " + contract.arguments.length;
-      }
-    }
-    return null;
-  }
-
   private void analyzeCodeBlock(@Nullable final PsiElement scope, ProblemsHolder holder, final boolean onTheFly) {
     if (scope == null) return;
 

java/java-impl/src/com/intellij/codeInspection/dataFlow/EditContractIntention.java

@@ -83,7 +83,7 @@ public class EditContractIntention extends BaseIntentionAction {
       public String getErrorText(String inputString) {
         if (StringUtil.isEmpty(inputString)) return null;
 
-        return DataFlowInspectionBase.checkContract(method, inputString);
+        return ContractInspection.checkContract(method, inputString);
       }
 
       @Override

java/java-tests/testData/inspection/dataFlow/contractCheck/SignatureIssues.java

@@ -0,0 +1,14 @@
+import org.jetbrains.annotations.Contract;
+import org.jetbrains.annotations.Nullable;
+
+class Foo {
+  @Contract(<warning descr="A contract clause must be in form arg1, ..., argN -> return-value">"a"</warning>)
+  void malformedContract() {}
+
+  @Contract(<warning descr="Method takes 2 parameters, while contract clause number 1 expects 1">"null -> _"</warning>)
+  void wrongParameterCount(Object a, boolean b) {}
+
+  @Contract(pure=<warning descr="Pure methods must return something, void is not allowed as a return type">true</warning>)
+  void voidPureMethod() {}
+
+}

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

@@ -41,12 +41,4 @@ class AssertIsNotNull {
   
   Object call() {return new Object();}
   
-  @Contract(<warning descr="A contract clause must be in form arg1, ..., argN -> return-value">"a"</warning>)
-  void malformedContract() {}
-
-  @Contract(<warning descr="Method takes 2 parameters, while contract clause number 1 expects 1">"null -> _"</warning>)
-  void wrongParameterCount(Object a, boolean b) {}
-  
-  @Contract(pure=<warning descr="Pure methods must return something, void is not allowed as a return type">true</warning>)
-  void voidPureMethod() {}
 }

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

@@ -1,7 +1,7 @@
 package com.intellij.codeInspection;
 
 import com.intellij.JavaTestUtil;
-import com.intellij.codeInspection.dataFlow.DataFlowInspection;
+import com.intellij.codeInspection.dataFlow.ContractInspection;
 import com.intellij.testFramework.LightProjectDescriptor;
 import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
 import org.jetbrains.annotations.NotNull;
@@ -22,7 +22,7 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase {
   }
 
   private void doTest() {
-    myFixture.enableInspections(new DataFlowInspection());
+    myFixture.enableInspections(new ContractInspection());
     myFixture.testHighlighting(true, false, true, getTestName(false) + ".java");
   }
 
@@ -36,4 +36,6 @@ public class ContractCheckTest extends LightCodeInsightFixtureTestCase {
   public void testDelegationWithUnknownArgument() { doTest(); }
   public void testEqualsUnknownValue() { doTest(); }
   public void testMissingFail() { doTest(); }
+
+  public void testSignatureIssues() { doTest(); }
 }

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -45,6 +45,7 @@ inspection.annotate.method.quickfix.name=Annotate method as ''@{0}''
 
 #dataflow
 inspection.data.flow.display.name=Constant conditions \\& exceptions
+inspection.contract.display.name=Contract issues
 inspection.data.flow.nullable.quickfix.option=<html><body>Suggest @Nullable annotation for methods that may possibly return null and <br>report nullable values passed to non-annotated parameters</body></html>
 inspection.data.flow.true.asserts.option=<html><body>Don't report assertions with condition statically proven to be always <code>true</code></body></html>
 inspection.data.flow.redundant.instanceof.quickfix=Replace with != null

platform/util/src/com/intellij/openapi/diagnostic/Logger.java

@@ -143,7 +143,7 @@ public abstract class Logger {
       error(resultMessage, new Throwable());
     }
 
-    //noinspection ConstantConditions
+    //noinspection Contract
     return value;
   }
 

resources-en/src/inspectionDescriptions/Contract.html

@@ -0,0 +1,9 @@
+<html>
+<body>
+This inspection reports various method contract (@Contract annotation) well-formedness issues:
+<!-- tooltip end -->
+<li>Errors in contract syntax</li>
+<li>Contracts not conforming to the method signature (wrong parameter count)</li>
+<li>Method implementations that contradict the contract (e.g. returning "true" when the contract says "false")</li>
+</body>
+</html>

resources/src/META-INF/IdeaPlugin.xml

@@ -518,6 +518,9 @@
     <localInspection language="JAVA" shortName="ConstantConditions" bundle="messages.InspectionsBundle" key="inspection.data.flow.display.name"
                      groupName="Probable bugs" enabledByDefault="true" level="WARNING"
                      implementationClass="com.intellij.codeInspection.dataFlow.DataFlowInspection"/>
+    <localInspection language="JAVA" shortName="Contract" bundle="messages.InspectionsBundle" key="inspection.contract.display.name"
+                     groupName="Probable bugs" enabledByDefault="true" level="WARNING"
+                     implementationClass="com.intellij.codeInspection.dataFlow.ContractInspection"/>
     <localInspection language="JAVA" shortName="UnusedAssignment" displayName="Unused assignment" groupName="Probable bugs" enabledByDefault="true"
                      level="WARNING" implementationClass="com.intellij.codeInspection.defUse.DefUseInspection"/>
     <localInspection language="JAVA" shortName="NumericOverflow" displayName="Numeric overflow" groupName="Numeric issues" enabledByDefault="true"