Issue #12164: Enable Checker Framework by Vyom-Yadav · Pull Request #12194 · checkstyle/checkstyle

Vyom-Yadav · 2022-09-12T09:08:53Z

Resolves #12164

nrmancuso · 2022-09-12T13:29:37Z

Please apply:

➜  checkstyle git:(issue-12164-1) ✗ git diff | cat
diff --git a/pom.xml b/pom.xml
index 858d95227..1db2387f3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -257,6 +257,12 @@
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
       <version>31.1-jre</version>
+      <exclusions>
+        <exclusion>
+          <groupId>org.checkerframework</groupId>
+          <artifactId>checker-qual</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>org.apache.ant</groupId>

For issue in hibernate, we will need to send a PR to do something similar. We can exclude enforcer run temporarily for them in our CI.

mvn clean verify passes for hibernate with:

➜  hibernate-search git:(main) ✗ git diff | cat
diff --git a/pom.xml b/pom.xml
index 651f9e8334..7ec032b53a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -405,7 +405,7 @@
         <version.scripting.plugin>3.0.0</version.scripting.plugin>
         <version.gpg.plugin>3.0.1</version.gpg.plugin>
         <version.org.codehaus.groovy-jsr223>3.0.12</version.org.codehaus.groovy-jsr223>
-        <version.com.puppycrawl.tools.checkstyle>10.3.3</version.com.puppycrawl.tools.checkstyle>
+        <version.com.puppycrawl.tools.checkstyle>10.3.4-SNAPSHOT</version.com.puppycrawl.tools.checkstyle>
         <version.versions.plugin>2.12.0</version.versions.plugin>
 
         <!-- Asciidoctor -->
@@ -1326,6 +1326,12 @@
                 <artifactId>guava</artifactId>
                 <version>${version.org.google.guava}</version>
                 <scope>compile</scope>
+                <exclusions>
+                    <exclusion>
+                        <groupId>org.checkerframework</groupId>
+                        <artifactId>checker-qual</artifactId>
+                    </exclusion>
+                </exclusions>
             </dependency>
             <dependency>
                 <groupId>org.jberet</groupId>

Vyom-Yadav · 2022-09-15T13:12:57Z

Error from https://github.com/checkstyle/checkstyle/actions/runs/3047499328/jobs/4911550442

Error: /home/runner/work/checkstyle/checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractCheck.java:[40,16] error: [type.checking.not.run] OptionalChecker did not run because of a previous error issued by javac

Running checker framework without -Awarns issues a lots of warnings like these.
From SO Answer (By creator of Checker Framework)

The Checker Framework runs as a plugin to javac. When javac issues an error in one class (including any Checker Framework error), javac may or may not process other classes. Unfortunately, there is no good way for a user to predict how far javac will get.

Creating a different profile for each checker will also not work. If a checker fails then new warnings from that checker can also be like did not run because of a previous error issued by javac.

Using -Awarns will simply convert errors to warnings.

romani · 2022-09-16T15:30:33Z

Usage of warns might help only to not let maven execution fail with error code. So it will help us to analyze results and do failure based on some other reasons.

Do we have problems to get stable output from Checker execution ? I try to understand what blocks it us to merge this PR

Vyom-Yadav · 2022-09-16T16:55:49Z

Usage of warns might help only to not let maven execution fail with error code.

That is not the main purpose of using -Awarns, not failing on error can be done by
<failOnError>false</failOnError>.

We are using -Awarns because many errors are there due to the checker not running due to
a previous error issued by javac. Using -Awarns only shows the error we need, without the errors of type did not run because of a previous error issued by javac.

romani · 2022-09-16T20:20:27Z

Good to know.

Can you briefly answer my other question ?

Vyom-Yadav · 2022-09-17T09:36:46Z

Do we have problems to get stable output from Checker execution ? I try to understand what blocks it us to merge this PR

I executed it locally a couple of times, and new violations pop up in the CI, trying once more.

Update: It is stable now.

Vyom-Yadav · 2022-09-17T10:44:43Z

-    SystemExit {
-        doNotApplyToFileNames = 'pitest-survival-check-xml.groovy,error-prone-check.groovy'
-    }


We suppress every groovy script, removal of this is better, we will most probably use groovy only for writing scripts.

romani · 2022-09-17T14:13:35Z

@Vyom-Yadav , please put in commit proper issue number - #12164

please fix codenarc

Vyom-Yadav · 2022-09-17T14:22:52Z

+  <!-- Unstable as 'capture#' keeps changing in different runs. -->
+  <checkerFrameworkError unstable="true">
+    <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/HandlerFactory.java</fileName>
+    <specifier>argument</specifier>
+    <message>incompatible argument for parameter constructor of invokeConstructor.</message>
+    <lineContent>handlerCtor, indentCheck, ast, parent);</lineContent>
+    <details>
+      found   : @Initialized @Nullable Constructor&lt;capture#788 extends @Initialized @Nullable Object&gt;
+      required: @Initialized @NonNull Constructor&lt;capture#788 extends @Initialized @Nullable Object&gt;
+    </details>
+  </checkerFrameworkError>


Unstable violation

Vyom-Yadav · 2022-09-17T14:23:25Z

+  <!-- Unstable as 'capture#' keeps changing in different runs. -->
+  <checkerFrameworkError unstable="true">
+    <fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/HandlerFactory.java</fileName>
+    <specifier>type.argument</specifier>
+    <message>incompatible type argument for type parameter T extends Object of invokeConstructor.</message>
+    <lineContent>resultHandler = (AbstractExpressionHandler) CommonUtil.invokeConstructor(</lineContent>
+    <details>
+      found   : capture#788[ extends @UnknownKeyFor Object super @KeyForBottom Void]
+      required: [extends @UnknownKeyFor Object super @UnknownKeyFor NullType]
+    </details>
+  </checkerFrameworkError>


Unstable violation.

romani

Ok let's try this Checker in read day to day coding

nrmancuso

There are a lot of nuances to understand with each checker, let’s merge this and live with it in order to learn. @Vyom-Yadav please create an issue to resolve checker suppressions, and in it please explain details of unstable suppressions.

Vyom-Yadav force-pushed the enableCheckerFramework branch 2 times, most recently from 422f9de to 8e5c196 Compare September 12, 2022 11:09

nrmancuso reviewed Sep 12, 2022

View reviewed changes

Comment thread pom.xml

Vyom-Yadav force-pushed the enableCheckerFramework branch 2 times, most recently from 86bfbfc to aced8ac Compare September 12, 2022 11:28

nrmancuso mentioned this pull request Sep 12, 2022

Issue #12164: Enable Checker Framework #12192

Closed

Vyom-Yadav force-pushed the enableCheckerFramework branch 2 times, most recently from 327a395 to 2bb2df4 Compare September 13, 2022 18:33

Vyom-Yadav force-pushed the enableCheckerFramework branch 2 times, most recently from 4b681b0 to 83bbf94 Compare September 15, 2022 13:19

Vyom-Yadav closed this Sep 15, 2022

Vyom-Yadav force-pushed the enableCheckerFramework branch from 83bbf94 to 0167a94 Compare September 15, 2022 13:23

Vyom-Yadav reopened this Sep 15, 2022

Vyom-Yadav force-pushed the enableCheckerFramework branch from 972583d to 25d7657 Compare September 15, 2022 13:32

Vyom-Yadav mentioned this pull request Sep 15, 2022

Issue #12164: Enable Optional Checker for detecting errors in using the Optional type #12184

Closed

Vyom-Yadav force-pushed the enableCheckerFramework branch from 25d7657 to 8ad9305 Compare September 16, 2022 11:21

Vyom-Yadav force-pushed the enableCheckerFramework branch 4 times, most recently from 03106b0 to 0da7afe Compare September 17, 2022 09:35

Vyom-Yadav force-pushed the enableCheckerFramework branch 3 times, most recently from ea5a8cd to e34f1a6 Compare September 17, 2022 10:43

Vyom-Yadav commented Sep 17, 2022

View reviewed changes

Vyom-Yadav force-pushed the enableCheckerFramework branch 4 times, most recently from fc18676 to 6f66617 Compare September 17, 2022 13:36

Issue checkstyle#12164: Enable Checker Framework

ca2a030

Vyom-Yadav force-pushed the enableCheckerFramework branch from 6f66617 to ca2a030 Compare September 17, 2022 14:19

Vyom-Yadav commented Sep 17, 2022

View reviewed changes

romani approved these changes Sep 17, 2022

View reviewed changes

romani assigned nrmancuso Sep 17, 2022

nrmancuso approved these changes Sep 17, 2022

View reviewed changes

nrmancuso merged commit 6335dd8 into checkstyle:master Sep 17, 2022

Uh oh!

Conversation

Vyom-Yadav commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nrmancuso commented Sep 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vyom-Yadav commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vyom-Yadav commented Sep 16, 2022

Uh oh!

romani commented Sep 16, 2022

Uh oh!

Vyom-Yadav commented Sep 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vyom-Yadav Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

romani commented Sep 17, 2022

Uh oh!

Vyom-Yadav Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

Vyom-Yadav Sep 17, 2022

Choose a reason for hiding this comment

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

nrmancuso left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Vyom-Yadav commented Sep 12, 2022 •

edited

Loading

nrmancuso commented Sep 12, 2022 •

edited

Loading

Vyom-Yadav commented Sep 15, 2022 •

edited

Loading

romani commented Sep 16, 2022 •

edited

Loading

Vyom-Yadav commented Sep 17, 2022 •

edited

Loading

nrmancuso left a comment •

edited

Loading