Issue checkstyle#12210: Add method to ignore unstable checker framewo…

…rk violations
Vyom-Yadav · Sep 18, 2022 · d499c0b · d499c0b
1 parent 6335dd8
commit d499c0b
Show file tree

Hide file tree

Showing 2 changed files with 154 additions and 30 deletions.
diff --git a/.ci/checker-framework-suppressions/checker-framework-methods-resource-fenum-suppressions.xml b/.ci/checker-framework-suppressions/checker-framework-methods-resource-fenum-suppressions.xml
@@ -11,7 +11,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/Main.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-10288 or any of its aliases.</message>
@@ -77,7 +78,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20912 or any of its aliases.</message>
@@ -88,7 +90,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20915 or any of its aliases.</message>
@@ -99,7 +102,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20917 or any of its aliases.</message>
@@ -110,7 +114,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20918 or any of its aliases.</message>
@@ -121,7 +126,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20923 or any of its aliases.</message>
@@ -132,7 +138,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/ant/CheckstyleAntTask.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20926 or any of its aliases.</message>
@@ -176,7 +183,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/meta/XmlMetaReader.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-23288 or any of its aliases.</message>
@@ -198,7 +206,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/DescendantIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20533 or any of its aliases.</message>
@@ -209,7 +218,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/DescendantIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20534 or any of its aliases.</message>
@@ -220,7 +230,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/DescendantIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20536 or any of its aliases.</message>
@@ -231,7 +242,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/FollowingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20414 or any of its aliases.</message>
@@ -242,7 +254,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/FollowingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20415 or any of its aliases.</message>
@@ -253,7 +266,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/FollowingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20418 or any of its aliases.</message>
@@ -264,7 +278,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/FollowingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-20420 or any of its aliases.</message>
@@ -275,7 +290,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/PrecedingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-10345 or any of its aliases.</message>
@@ -286,7 +302,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/PrecedingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-10346 or any of its aliases.</message>
@@ -297,7 +314,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/PrecedingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-10349 or any of its aliases.</message>
@@ -309,7 +327,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/PrecedingIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-10351 or any of its aliases.</message>
@@ -320,7 +339,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/ReverseDescendantIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-22037 or any of its aliases.</message>
@@ -331,7 +351,8 @@
     </details>
   </checkerFrameworkError>
 
-  <checkerFrameworkError unstable="false">
+  <!-- Error is considered unstable as temp-var changes with each run. -->
+  <checkerFrameworkError unstable="true">
     <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/xpath/iterators/ReverseDescendantIterator.java</fileName>
     <specifier>required.method.not.called</specifier>
     <message>@MustCall method close may not have been invoked on temp-var-22043 or any of its aliases.</message>

diff --git a/.ci/checker-framework.groovy b/.ci/checker-framework.groovy
@@ -1,4 +1,3 @@
-import groovy.transform.EqualsAndHashCode
 import groovy.transform.Field
 import groovy.transform.Immutable
 import groovy.util.slurpersupport.GPathResult
@@ -382,7 +381,6 @@ private static Set<CheckerFrameworkError> setDifference(final Set<CheckerFramewo
 /**
  * A class to represent the XML {@code checkerFrameworkError} node.
  */
-@EqualsAndHashCode(excludes = ["lineNumber", "unstable"])
 @Immutable
 class CheckerFrameworkError implements Comparable<CheckerFrameworkError> {
 
@@ -398,6 +396,21 @@ class CheckerFrameworkError implements Comparable<CheckerFrameworkError> {
     List<String> details
     String lineContent
     int lineNumber
+
+    /**
+     * Whether the error is unstable. Unstable errors in suppression list are not flagged as
+     * unnecessary suppressions. An error is considered to be unstable when:
+     * <ul>
+     *     <li>
+     *         Error message changes with each run. Some error messages contains strings like
+     *         {@code temp-var-1234}, the numerical part changes with each run so the error is
+     *         considered unstable. In such cases numerical values in {@code details} and
+     *         {@code message} are replaced with empty string while comparing and hashing errors.
+     *     </li>
+     *     <li>
+     *         The error is in general unstable, it sometimes appears and sometimes it does not.
+     *     </li>
+     */
     boolean unstable
 
     @Override
@@ -435,17 +448,107 @@ class CheckerFrameworkError implements Comparable<CheckerFrameworkError> {
             return i
         }
 
-        i = getMessage() <=> other.getMessage()
-        if (i != 0) {
-            return i
+        if (this.isUnstable() || other.isUnstable()) {
+            final String messageWithoutLineNumber = getMessage().replaceAll('\\d+', '')
+            final String thatMessageWithoutLineNumber = other.getMessage().replaceAll('\\d+', '')
+            i = messageWithoutLineNumber <=> thatMessageWithoutLineNumber
+            if (i != 0) {
+                return i
+            }
+
+            final List<String> detailsWithoutLineNumber = this.getDetails().collect {
+                it.replaceAll('\\d+', '')
+            }
+            final List<String> thatDetailsWithoutLineNumber = other.getDetails().collect {
+                it.replaceAll('\\d+', '')
+            }
+
+            i = detailsWithoutLineNumber.join('') <=> thatDetailsWithoutLineNumber.join('')
+            if (i != 0) {
+                return i
+            }
         }
+        else {
+            i = getMessage() <=> other.getMessage()
+            if (i != 0) {
+                return i
+            }
 
-        i = getLineContent() <=> other.getLineContent()
-        if (i != 0) {
-            return i
+            i = getDetails().join('') <=> other.getDetails().join('')
+            if (i != 0) {
+                return i
+            }
+        }
+
+        return getLineContent() <=> other.getLineContent()
+    }
+
+    boolean equals(o) {
+        if (this.is(o)) {
+            return true
+        }
+        if (!(o instanceof CheckerFrameworkError)) {
+            return false
         }
 
-        return this.getDetails().join('') <=> other.getDetails().join('')
+        CheckerFrameworkError that = (CheckerFrameworkError) o
+
+        if (this.unstable || that.unstable) {
+            final String messageWithoutLineNumber = this.message.replaceAll('\\d+', '')
+            final String thatMessageWithoutLineNumber = that.message.replaceAll('\\d+', '')
+            if (messageWithoutLineNumber != thatMessageWithoutLineNumber) {
+                return false
+            }
+
+            final List<String> detailsWithoutLineNumber = this.details.collect {
+                it.replaceAll('\\d+', '')
+            }
+            final List<String> thatDetailsWithoutLineNumber = that.details.collect {
+                it.replaceAll('\\d+', '')
+            }
+            if (!detailsWithoutLineNumber.equals(thatDetailsWithoutLineNumber)) {
+                return false
+            }
+        }
+        else {
+
+            if (!details.equals(that.details)) {
+                return false
+            }
+            if (message != that.message) {
+                return false
+            }
+        }
+
+        if (fileName != that.fileName) {
+            return false
+        }
+        if (lineContent != that.lineContent) {
+            return false
+        }
+        if (specifier != that.specifier) {
+            return false
+        }
+
+        return true
+    }
+
+    int hashCode() {
+        int result
+        if (unstable) {
+            result = (message != null ? message.replaceAll('\\d+', '').hashCode() : 0)
+            result = 31 * result + (details != null ? details.collect {
+                it.replaceAll('\\d+', '')
+            }.hashCode() : 0)
+        }
+        else {
+            result = (message != null ? message.hashCode() : 0)
+            result = 31 * result + (details != null ? details.hashCode() : 0)
+        }
+        result = 31 * result + (fileName != null ? fileName.hashCode() : 0)
+        result = 31 * result + (specifier != null ? specifier.hashCode() : 0)
+        result = 31 * result + (lineContent != null ? lineContent.hashCode() : 0)
+        return result
     }
 
     /**