Issue #12210: Add method to ignore unstable checker framework violations

checkstyle · Sep 19, 2022 · 701bd65 · 701bd65
1 parent e0f745a
commit 701bd65
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 31 deletions.
diff --git a/.ci/StarterRuleSet-AllRulesByCategory.groovy.txt b/.ci/StarterRuleSet-AllRulesByCategory.groovy.txt
@@ -120,7 +120,12 @@ ruleset {
     EmptyMethodInAbstractClass
     FinalClassWithProtectedMember
     ImplementationAsType
-    Instanceof
+
+    // Using instanceof in equals method is alright
+    Instanceof {
+        ignoreTypeNames = 'CheckerFrameworkError'
+    }
+
     LocaleSetDefault
     NestedForLoop
     PrivateFieldCouldBeFinal

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,17 @@ 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.
+     *
+     * <p>An error is considered to be unstable when 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.
+     */
     boolean unstable
 
     @Override
@@ -435,17 +444,68 @@ 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()*.replaceAll('\\d+', '')
+            final List<String> thatDetailsWithoutLineNumber =
+                    other.getDetails()*.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()
+    }
+
+    @Override
+    boolean equals(Object object) {
+        if (this.is(object)) {
+            return true
+        }
+        if (!(object instanceof CheckerFrameworkError)) {
+            return false
         }
 
-        return this.getDetails().join('') <=> other.getDetails().join('')
+        CheckerFrameworkError that = (CheckerFrameworkError) object
+
+        return (this <=> that) == 0
+    }
+
+    @Override
+    int hashCode() {
+        int result
+        if (unstable) {
+            result = (message != null ? message.replaceAll('\\d+', '').hashCode() : 0)
+            result = 31 * result + (details != null ? details*.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
     }
 
     /**