Issue checkstyle#6068: migrate to picocli command line parser from Co…

…mmons CLI
remkop · Sep 22, 2018 · 4ff0ad4 · 4ff0ad4
1 parent 27512b0
commit 4ff0ad4
Show file tree

Hide file tree

Showing 14 changed files with 633 additions and 620 deletions.
diff --git a/.ci/jsoref-spellchecker/whitelist.words b/.ci/jsoref-spellchecker/whitelist.words
@@ -56,6 +56,7 @@ areaoftest
 arget
 argn
 Arity
+arity
 Arquillian
 arraycopy
 arraylist
@@ -317,6 +318,7 @@ Dfoo
 Dforbiddenapis
 Dgpg
 Dgui
+dhj
 Diachenko
 dirname
 distelli
@@ -607,6 +609,7 @@ ith
 itr
 ivanov
 ivanovjr
+IVersion
 iws
 Izmailov
 jacoco
@@ -788,6 +791,7 @@ missingoverride
 missingswitchdefault
 missingtag
 Mixin
+mixin
 mkdir
 mkordas
 MLINKCHECK
@@ -973,6 +977,7 @@ pgjdbc
 Pgpg
 pguyot
 php
+picocli
 pid
 pitest
 pitesttyle
@@ -1045,6 +1050,7 @@ regexpsingleline
 regexpsinglelinejava
 relativized
 releasenotes
+remkop
 requirethis
 Rethrown
 returncount

diff --git a/README.md b/README.md
@@ -77,7 +77,7 @@ are in the file named "RIGHTS.antlr" in this directory.
 This product includes software developed by
 The Apache Software Foundation (http://www.apache.org/).
 
-The software uses the Cli, Logging and Beanutils packages from the
+The software uses the Logging and Beanutils packages from the
 Apache Commons project (http://commons.apache.org/). The license terms
 of these packages are in the file named "LICENSE.apache20" in this
 directory.
@@ -87,6 +87,10 @@ The software uses the Google Guava Libraries
 these packages are in the file named "LICENSE.apache20" in this
 directory.
 
+The software uses the Picocli Library
+(https://github.com/remkop/picocli/). Its license terms 
+are in the file named "LICENSE.apache20" in this directory.
+
 [travis]:https://travis-ci.org/checkstyle/checkstyle/builds
 [travis img]:https://travis-ci.org/checkstyle/checkstyle.svg
 

diff --git a/config/import-control.xml b/config/import-control.xml
@@ -29,7 +29,7 @@
   <allow class="java.security.NoSuchAlgorithmException" local-only="true"/>
   <allow pkg="com.puppycrawl.tools.checkstyle.utils" local-only="true"/>
   <allow pkg="com.puppycrawl.tools.checkstyle.grammar" local-only="true"/>
-  <allow pkg="org.apache.commons.cli" local-only="true"/>
+  <allow pkg="picocli" local-only="true"/>
   <allow pkg="org.antlr.v4.runtime" local-only="true"/>
   <allow class="com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser.+"
           local-only="true" regex="true"/>

diff --git a/config/intellij-idea-inspections.xml b/config/intellij-idea-inspections.xml
@@ -1828,7 +1828,10 @@
         <option name="m_ignoreInvisibleFields" value="true"/>
     </inspection_tool>
     <inspection_tool class="FieldMayBeFinal" enabled="true" level="ERROR"
-                     enabled_by_default="true"/>
+                     enabled_by_default="true">
+        <exclude classes="com.puppycrawl.tools.checkstyle.Main,
+                        ,com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator" />
+    </inspection_tool>
     <inspection_tool class="FieldMayBeStatic" enabled="true" level="WARNING"
                      enabled_by_default="true"/>
     <inspection_tool class="FieldNotUsedInToString" enabled="true" level="ERROR"
@@ -2678,6 +2681,8 @@
                      enabled_by_default="true"/>
     <inspection_tool class="JSMismatchedCollectionQueryUpdate" enabled="true" level="ERROR"
                      enabled_by_default="true">
+        <exclude classes="com.puppycrawl.tools.checkstyle.Main,
+                    ,com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator" />
         <option name="queries" value="trace,write,forEach"/>
         <option name="updates" value="pop,push,shift,splice,unshift"/>
     </inspection_tool>
@@ -2945,7 +2950,10 @@
     <!-- finds false positives and we already have our own FinalLocalVariableCheck
          for this purpose -->
     <inspection_tool class="LocalCanBeFinal" enabled="false" level="ERROR"
-                     enabled_by_default="false"/>
+                     enabled_by_default="false">
+        <exclude classes="com.puppycrawl.tools.checkstyle.Main,
+                    ,com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator" />
+    </inspection_tool>
     <inspection_tool class="LocalVariableHidingMemberVariable" enabled="true" level="WARNING"
                      enabled_by_default="true">
         <option name="m_ignoreInvisibleFields" value="true"/>
@@ -4970,7 +4978,10 @@
                      enabled_by_default="true">
         <option name="m_ignoreCatchBlocksWithComments" value="false"/>
     </inspection_tool>
-    <inspection_tool class="UnusedDefine" enabled="false" level="ERROR" enabled_by_default="false"/>
+    <inspection_tool class="UnusedDefine" enabled="false" level="ERROR" enabled_by_default="false">
+        <exclude classes="com.puppycrawl.tools.checkstyle.Main,
+                ,com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator" />
+    </inspection_tool>
     <inspection_tool class="UnusedEquals" enabled="false" level="WARNING"
                      enabled_by_default="false"/>
     <inspection_tool class="UnusedImport" enabled="true" level="WARNING" enabled_by_default="true"/>

diff --git a/config/pmd-main.xml b/config/pmd-main.xml
@@ -19,5 +19,16 @@
                                  //MethodDeclaration[@Name='main']"/>
         </properties>
     </rule>
-
+    <rule ref="category/java/design.xml/ImmutableField">
+        <property name="ignoredAnnotations"
+                  value="picocli.CommandLine.Option
+                        |picocli.CommandLine.Parameters
+                        |picocli.CommandLine.Spec"/>
+    </rule>
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateField">
+        <property name="ignoredAnnotations"
+                  value="picocli.CommandLine.Option
+                        |picocli.CommandLine.Parameters
+                        |picocli.CommandLine.Spec"/>
+    </rule>
 </ruleset>
diff --git a/config/pmd.xml b/config/pmd.xml
@@ -36,7 +36,9 @@
   <rule ref="category/java/bestpractices.xml/SystemPrintln">
     <properties>
       <!-- it is ok to use println in CLI class. -->
-      <property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='Main']"/>
+      <property name="violationSuppressXPath"
+                value="//ClassOrInterfaceDeclaration
+                       [@Image='Main' or @Image='JavadocPropertiesGenerator']"/>
     </properties>
   </rule>
 
@@ -310,8 +312,18 @@
   <rule ref="category/java/design.xml/TooManyFields">
     <properties>
       <!-- Unable to split fields out of the class. -->
+      <!-- Main has an annotated field for each command line option. This is by design. -->
+      <property name="violationSuppressXPath"
+                value="//ClassOrInterfaceDeclaration[@Image='Checker']
+    | //ClassOrInterfaceDeclaration[@Image='Main']"/>
+    </properties>
+  </rule>
+  <rule ref="category/java/design.xml/ImmutableField">
+    <properties>
+      <!-- Main has annotated fields whose value is injected by picocli.
+        These fields should not be marked final. -->
       <property name="violationSuppressXPath"
-                value="//ClassOrInterfaceDeclaration[@Image='Checker']"/>
+                value="//ClassOrInterfaceDeclaration[@Image='Main']"/>
     </properties>
   </rule>
   <rule ref="category/java/design.xml/TooManyMethods">

diff --git a/config/spotbugs-exclude.xml b/config/spotbugs-exclude.xml
@@ -107,6 +107,46 @@
         <Method name="loadProperties"/>
         <Bug pattern="NP_NULL_PARAM_DEREF"/>
     </Match>
+    <Match>
+        <!-- false-positive: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Bug pattern="NP_UNWRITTEN_FIELD"/>
+    </Match>
+    <Match>
+        <!-- false-positive: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Bug pattern="UWF_UNWRITTEN_FIELD"/>
+    </Match>
+    <Match>
+        <!-- false-positive: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Bug pattern="MS_SHOULD_BE_FINAL"/>
+    </Match>
+    <Match>
+        <!-- false-positive: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Bug pattern="MS_SHOULD_BE_REFACTORED_TO_BE_FINAL"/>
+    </Match>
+    <Match>
+        <!-- false-positive: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Bug pattern="SS_SHOULD_BE_STATIC"/>
+    </Match>
     <Match>
         <!-- false-positive. See details at https://github.com/checkstyle/checkstyle/pull/5403 -->
         <Class name="com.puppycrawl.tools.checkstyle.PackageNamesLoader"/>

diff --git a/pom.xml b/pom.xml
@@ -231,6 +231,11 @@
   </pluginRepositories>
 
   <dependencies>
+    <dependency>
+      <groupId>info.picocli</groupId>
+      <artifactId>picocli</artifactId>
+      <version>3.6.0</version>
+    </dependency>
     <dependency>
       <groupId>antlr</groupId>
       <artifactId>antlr</artifactId>
@@ -246,11 +251,6 @@
       <artifactId>commons-beanutils</artifactId>
       <version>1.9.3</version>
     </dependency>
-    <dependency>
-      <groupId>commons-cli</groupId>
-      <artifactId>commons-cli</artifactId>
-      <version>1.4</version>
-    </dependency>
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
@@ -826,6 +826,7 @@
                     <exclude>
                       com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocParser*
                     </exclude>
+                    <exclude>com.puppycrawl.tools.checkstyle.Main</exclude>
                     <exclude>com.puppycrawl.tools.checkstyle.grammar.javadoc.JavadocLexer</exclude>
                     <exclude>
                       com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser.JavadocParserErrorStrategy
@@ -956,6 +957,26 @@
                     </limit>
                   </limits>
                 </rule>
+                <rule>
+                  <element>CLASS</element>
+                  <includes>
+                    <include>
+                      com.puppycrawl.tools.checkstyle.Main
+                    </include>
+                  </includes>
+                  <limits>
+                    <limit>
+                      <counter>LINE</counter>
+                      <value>COVEREDRATIO</value>
+                      <minimum>0.99</minimum>
+                    </limit>
+                    <limit>
+                      <counter>BRANCH</counter>
+                      <value>COVEREDRATIO</value>
+                      <minimum>0.99</minimum>
+                    </limit>
+                  </limits>
+                </rule>
               </rules>
             </configuration>
           </execution>