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

…mmons CLI

.ci/jsoref-spellchecker/whitelist.words

@@ -55,7 +55,7 @@ APRV
 areaoftest
 arget
 argn
-Arity
+arity
 Arquillian
 arraycopy
 arraylist
@@ -315,8 +315,10 @@ Dexec
 dfn
 Dfoo
 Dforbiddenapis
+dghj
 Dgpg
 Dgui
+dhj
 Diachenko
 dirname
 distelli
@@ -607,6 +609,7 @@ ith
 itr
 ivanov
 ivanovjr
+IVersion
 iws
 Izmailov
 jacoco
@@ -787,7 +790,7 @@ missingdeprecated
 missingoverride
 missingswitchdefault
 missingtag
-Mixin
+mixin
 mkdir
 mkordas
 MLINKCHECK
@@ -973,6 +976,7 @@ pgjdbc
 Pgpg
 pguyot
 php
+picocli
 pid
 pitest
 pitesttyle
@@ -1045,6 +1049,7 @@ regexpsingleline
 regexpsinglelinejava
 relativized
 releasenotes
+remkop
 requirethis
 Rethrown
 returncount

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
 

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"/>

config/pmd-main.xml

@@ -19,5 +19,25 @@
                                  //MethodDeclaration[@Name='main']"/>
         </properties>
     </rule>
-
+    <rule ref="category/java/design.xml/ImmutableField">
+        <properties>
+            <!-- Fields with these annotations will have their value injected by picocli.
+              These fields should not be marked final. -->
+            <property name="ignoredAnnotations"
+                      value="picocli.CommandLine.Option
+                            |picocli.CommandLine.Parameters
+                            |picocli.CommandLine.Spec"/>
+        </properties>
+    </rule>
+    <rule ref="category/java/bestpractices.xml/UnusedPrivateField">
+        <properties>
+            <!-- Some fields with picocli annotations will be only used by
+              the picocli and not by the application.
+              They may appear unused by static code analysis tools. -->
+            <property name="ignoredAnnotations"
+                      value="picocli.CommandLine.Option
+                            |picocli.CommandLine.Parameters
+                            |picocli.CommandLine.Spec"/>
+        </properties>
+    </rule>
 </ruleset>

config/pmd.xml

@@ -36,7 +36,10 @@
   <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='Main$CliOptions'
+                       or @Image='JavadocPropertiesGenerator']"/>
     </properties>
   </rule>
 
@@ -310,8 +313,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">

config/spotbugs-exclude.xml

@@ -107,6 +107,20 @@
         <Method name="loadProperties"/>
         <Bug pattern="NP_NULL_PARAM_DEREF"/>
     </Match>
+    <Match>
+        <!-- won't fix: field values are injected by picocli -->
+        <Or>
+            <Class name="com.puppycrawl.tools.checkstyle.Main"/>
+            <Class name="com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator"/>
+        </Or>
+        <Or>
+            <Bug pattern="NP_UNWRITTEN_FIELD"/>
+            <Bug pattern="UWF_UNWRITTEN_FIELD"/>
+            <Bug pattern="MS_SHOULD_BE_FINAL"/>
+            <Bug pattern="MS_SHOULD_BE_REFACTORED_TO_BE_FINAL"/>
+            <Bug pattern="SS_SHOULD_BE_STATIC"/>
+        </Or>
+    </Match>
     <Match>
         <!-- false-positive. See details at https://github.com/checkstyle/checkstyle/pull/5403 -->
         <Class name="com.puppycrawl.tools.checkstyle.PackageNamesLoader"/>

pom.xml

@@ -231,6 +231,11 @@
   </pluginRepositories>
 
   <dependencies>
+    <dependency>
+      <groupId>info.picocli</groupId>
+      <artifactId>picocli</artifactId>
+      <version>3.7.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>
@@ -1346,6 +1346,7 @@
         <excludes>
           <!-- system-out is ok there, that is CLI -->
           <exclude>**/Main.class</exclude>
+          <exclude>**/Main$CliOptions.class</exclude>
           <exclude>**/JavadocPropertiesGenerator.class</exclude>
           <!-- generated classes, unfortunately use problematic api -->
           <exclude>**/GeneratedJavaLexer.class</exclude>

src/main/java/com/puppycrawl/tools/checkstyle/JavadocPropertiesGenerator.java

@@ -27,19 +27,18 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import org.apache.commons.cli.CommandLine;
-import org.apache.commons.cli.CommandLineParser;
-import org.apache.commons.cli.DefaultParser;
-import org.apache.commons.cli.HelpFormatter;
-import org.apache.commons.cli.Options;
-import org.apache.commons.cli.ParseException;
-
 import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.DetailNode;
 import com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 import com.puppycrawl.tools.checkstyle.utils.JavadocUtil;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.ParameterException;
+import picocli.CommandLine.Parameters;
+import picocli.CommandLine.ParseResult;
 
 /**
  * This class is used internally in the build process to write a property file
@@ -48,26 +47,20 @@
  * For IDE plugins (like the eclipse plugin) it would be useful to have
  * a programmatic access to the first sentence of the TokenType constants,
  * so they can use them in their configuration gui.
+ * @noinspection UseOfSystemOutOrSystemErr, unused, ClassIndependentOfModule
  */
 public final class JavadocPropertiesGenerator {
 
-    /**
-     * The command line option to specify the output file.
-     */
-    private static final String OPTION_DEST_FILE = "destfile";
-
-    /**
-     * The width of the CLI help option.
-     */
-    private static final int HELP_WIDTH = 100;
-
     /**
      * This regexp is used to extract the first sentence from the text.
      * The end of the sentence is determined by the symbol "period", "exclamation mark" or
      * "question mark", followed by a space or the end of the text.
      */
     private static final Pattern END_OF_SENTENCE_PATTERN = Pattern.compile("(.*?[.?!])(\\s|$)");
 
+    /** Max width of the usage help message for this command. */
+    private static final int USAGE_HELP_WIDTH = 100;
+
     /**
      * Don't create instance of this class, use the {@link #main(String[])} method instead.
      */
@@ -78,39 +71,43 @@ private JavadocPropertiesGenerator() {
      * TokenTypes.properties generator entry point.
      * @param args the command line arguments
      * @throws CheckstyleException if parser or lexer failed or if there is an IO problem
-     * @throws ParseException if the command line can not be passed
      **/
-    public static void main(String... args)
-            throws CheckstyleException, ParseException {
-        final CommandLine commandLine = parseCli(args);
-        if (commandLine.getArgList().size() == 1) {
-            final File inputFile = new File(commandLine.getArgList().get(0));
-            final File outputFile = new File(commandLine.getOptionValue(OPTION_DEST_FILE));
-            writePropertiesFile(inputFile, outputFile);
+    public static void main(String... args) throws CheckstyleException {
+        final CliOptions cliOptions = new CliOptions();
+        final CommandLine cmd = new CommandLine(cliOptions).setUsageHelpWidth(USAGE_HELP_WIDTH);
+        try {
+            final ParseResult parseResult = cmd.parseArgs(args);
+            if (parseResult.isUsageHelpRequested()) {
+                cmd.usage(System.out);
+            }
+            else {
+                writePropertiesFile(cliOptions);
+            }
         }
-        else {
-            printUsage();
+        catch (ParameterException ex) {
+            System.err.println(ex.getMessage());
+            ex.getCommandLine().usage(System.err);
         }
     }
 
     /**
      * Creates the .properties file from a .java file.
-     * @param inputFile .java file
-     * @param outputFile .properties file
+     * @param options the user-specified options
      * @throws CheckstyleException if a javadoc comment can not be parsed
      */
-    private static void writePropertiesFile(File inputFile, File outputFile)
-            throws CheckstyleException {
-        try (PrintWriter writer = new PrintWriter(outputFile, StandardCharsets.UTF_8.name())) {
-            final DetailAST top = JavaParser.parseFile(inputFile, JavaParser.Options.WITH_COMMENTS);
+    private static void writePropertiesFile(CliOptions options) throws CheckstyleException {
+        try (PrintWriter writer = new PrintWriter(options.outputFile,
+                StandardCharsets.UTF_8.name())) {
+            final DetailAST top = JavaParser.parseFile(options.inputFile,
+                    JavaParser.Options.WITH_COMMENTS);
             final DetailAST objBlock = getClassBody(top);
             if (objBlock != null) {
                 iteratePublicStaticIntFields(objBlock, writer::println);
             }
         }
         catch (IOException ex) {
-            throw new CheckstyleException("Failed to write javadoc properties of '" + inputFile
-                + "' to '" + outputFile + "'", ex);
+            throw new CheckstyleException("Failed to write javadoc properties of '"
+                    + options.inputFile + "' to '" + options.outputFile + "'", ex);
         }
     }
 
@@ -304,35 +301,22 @@ private static void formatHtmlElement(StringBuilder builder, DetailNode node) {
     }
 
     /**
-     *  Prints the usage information.
+     * Helper class encapsulating the command line options and positional parameters.
      */
-    private static void printUsage() {
-        final HelpFormatter formatter = new HelpFormatter();
-        formatter.setWidth(HELP_WIDTH);
-        formatter.printHelp(String.format("java %s [options] <input file>.",
-            JavadocPropertiesGenerator.class.getName()), buildOptions());
-    }
+    @Command(name = "java com.puppycrawl.tools.checkstyle.JavadocPropertiesGenerator",
+            mixinStandardHelpOptions = true)
+    private static class CliOptions {
 
-    /**
-     * Parses command line based on passed arguments.
-     * @param args command line arguments
-     * @return parsed information about passed arguments
-     * @throws ParseException when passed arguments are not valid
-     */
-    private static CommandLine parseCli(String... args)
-            throws ParseException {
-        final CommandLineParser clp = new DefaultParser();
-        return clp.parse(buildOptions(), args);
-    }
+        /**
+         * The command line option to specify the output file.
+         */
+        @Option(names = "--destfile", required = true, description = "The output file.")
+        private File outputFile;
 
-    /**
-     * Builds and returns the list of supported parameters.
-     * @return available options
-     */
-    private static Options buildOptions() {
-        final Options options = new Options();
-        options.addRequiredOption(null, OPTION_DEST_FILE, true, "The output file.");
-        return options;
+        /**
+         * The command line positional parameter to specify the input file.
+         */
+        @Parameters(index = "0", description = "The input file.")
+        private File inputFile;
     }
-
 }