Fix experimental cli option must be hidden (#1316)

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -49,6 +49,7 @@ import org.hyperledger.besu.cli.custom.JsonRPCAllowlistHostsProperty;
 import org.hyperledger.besu.cli.custom.RpcAuthFileValidator;
 import org.hyperledger.besu.cli.error.BesuExceptionHandler;
 import org.hyperledger.besu.cli.options.EthProtocolOptions;
+import org.hyperledger.besu.cli.options.EthstatsOptions;
 import org.hyperledger.besu.cli.options.MetricsCLIOptions;
 import org.hyperledger.besu.cli.options.NetworkingOptions;
 import org.hyperledger.besu.cli.options.SynchronizerOptions;
@@ -210,6 +211,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   final EthProtocolOptions ethProtocolOptions = EthProtocolOptions.create();
   final MetricsCLIOptions metricsCLIOptions = MetricsCLIOptions.create();
   final TransactionPoolOptions transactionPoolOptions = TransactionPoolOptions.create();
+  final EthstatsOptions ethstatsOptions = EthstatsOptions.create();
+
   private final RunnerBuilder runnerBuilder;
   private final BesuController.Builder controllerBuilderFactory;
   private final BesuPluginContextImpl besuPluginContext;
@@ -423,12 +426,14 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
 
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
   @Option(
+      hidden = true,
       names = {"--Xnat-kube-service-name"},
       description =
           "Specify the name of the service that will be used by the nat manager in Kubernetes. (default: ${DEFAULT-VALUE})")
   private String natManagerServiceName = DEFAULT_BESU_SERVICE_NAME_FILTER;
 
   @Option(
+      hidden = true,
       names = {"--Xnat-method-fallback-enabled"},
       description =
           "Enable fallback to NONE for the nat manager in case of failure. If False BESU will exit on failure. (default: ${DEFAULT-VALUE})",
@@ -750,12 +755,14 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   private final Integer stratumPort = 8008;
 
   @Option(
+      hidden = true,
       names = {"--Xminer-remote-sealers-limit"},
       description =
           "Limits the number of remote sealers that can submit their hashrates (default: ${DEFAULT-VALUE})")
   private final Integer remoteSealersLimit = DEFAULT_REMOTE_SEALERS_LIMIT;
 
   @Option(
+      hidden = true,
       names = {"--Xminer-remote-sealers-hashrate-ttl"},
       description =
           "Specifies the lifetime of each entry in the cache. An entry will be automatically deleted if no update has been received before the deadline (default: ${DEFAULT-VALUE} minutes)")
@@ -1040,21 +1047,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
       arity = "1")
   private final Long wsTimeoutSec = TimeoutOptions.defaultOptions().getTimeoutSeconds();
 
-  @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
-  @Option(
-      names = {"--Xethstats"},
-      paramLabel = "<nodename:secret@host:port>",
-      description = "Reporting URL of a ethstats server",
-      arity = "1")
-  private String ethstatsUrl = "";
-
-  @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
-  @Option(
-      names = {"--Xethstats-contact"},
-      description = "Contact address to send to ethstats server",
-      arity = "1")
-  private String ethstatsContact = "";
-
   private EthNetworkConfig ethNetworkConfig;
   private JsonRpcConfiguration jsonRpcConfiguration;
   private GraphQLConfiguration graphQLConfiguration;
@@ -1221,6 +1213,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
             .put("P2P Network", networkingOptions)
             .put("Synchronizer", synchronizerOptions)
             .put("TransactionPool", transactionPoolOptions)
+            .put("Ethstats", ethstatsOptions)
             .build();
 
     UnstableOptionsSubCommand.createUnstableOptions(commandLine, unstableOptions);
@@ -1385,7 +1378,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   }
 
   private void validateNetStatsParams() {
-    if (Strings.isNullOrEmpty(ethstatsUrl) && !ethstatsContact.isEmpty()) {
+    if (Strings.isNullOrEmpty(ethstatsOptions.getEthstatsUrl())
+        && !ethstatsOptions.getEthstatsContact().isEmpty()) {
       throw new ParameterException(
           this.commandLine,
           "The `--Xethstats-contact` requires ethstats server URL to be provided. Either remove --Xethstats-contact"
@@ -2073,8 +2067,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
             .identityString(identityString)
             .besuPluginContext(besuPluginContext)
             .autoLogBloomCaching(autoLogBloomCachingEnabled)
-            .ethstatsUrl(ethstatsUrl)
-            .ethstatsContact(ethstatsContact)
+            .ethstatsUrl(ethstatsOptions.getEthstatsUrl())
+            .ethstatsContact(ethstatsOptions.getEthstatsContact())
             .build();
 
     addShutdownHook(runner);

besu/src/main/java/org/hyperledger/besu/cli/options/EthstatsOptions.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.options;
+
+import org.hyperledger.besu.ethstats.util.NetstatsUrl;
+
+import java.util.Arrays;
+import java.util.List;
+
+import picocli.CommandLine;
+
+public class EthstatsOptions implements CLIOptions<NetstatsUrl> {
+
+  private static final String ETHSTATS = "--Xethstats";
+  private static final String ETHSTATS_CONTACT = "--Xethstats-contact";
+
+  @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
+  @CommandLine.Option(
+      hidden = true,
+      names = {ETHSTATS},
+      paramLabel = "<nodename:secret@host:port>",
+      description = "Reporting URL of a ethstats server",
+      arity = "1")
+  private String ethstatsUrl = "";
+
+  @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
+  @CommandLine.Option(
+      hidden = true,
+      names = {ETHSTATS_CONTACT},
+      description = "Contact address to send to ethstats server",
+      arity = "1")
+  private String ethstatsContact = "";
+
+  private EthstatsOptions() {}
+
+  public static EthstatsOptions create() {
+    return new EthstatsOptions();
+  }
+
+  @Override
+  public NetstatsUrl toDomainObject() {
+    return NetstatsUrl.fromParams(ethstatsUrl, ethstatsContact);
+  }
+
+  public String getEthstatsUrl() {
+    return ethstatsUrl;
+  }
+
+  public String getEthstatsContact() {
+    return ethstatsContact;
+  }
+
+  @Override
+  public List<String> getCLIOptions() {
+    return Arrays.asList(ETHSTATS + "=" + ethstatsUrl, ETHSTATS_CONTACT + "=" + ethstatsContact);
+  }
+}

errorprone-checks/build.gradle

@@ -30,6 +30,7 @@ dependencies {
   implementation 'com.google.auto.service:auto-service'
   implementation 'com.google.errorprone:error_prone_annotation'
   implementation 'com.google.errorprone:error_prone_core'
+  implementation 'info.picocli:picocli'
 
   testImplementation 'com.google.errorprone:error_prone_test_helpers'
   testImplementation 'junit:junit'

errorprone-checks/src/main/java/org/hyperledger/errorpronechecks/ExperimentalCliOptionMustBeHidden.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.errorpronechecks;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+
+import java.util.Map;
+import java.util.Optional;
+import javax.lang.model.element.AnnotationMirror;
+import javax.lang.model.element.AnnotationValue;
+import javax.lang.model.element.ExecutableElement;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.AnnotationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.AnnotationTree;
+
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "ExperimentalCliOptionMustBeHidden",
+    summary = "Experimental options must be hidden.",
+    severity = WARNING,
+    linkType = BugPattern.LinkType.NONE)
+public class ExperimentalCliOptionMustBeHidden extends BugChecker implements AnnotationTreeMatcher {
+
+  @Override
+  public Description matchAnnotation(AnnotationTree tree, VisitorState state) {
+    final AnnotationMirror annotationMirror = ASTHelpers.getAnnotationMirror(tree);
+    if (annotationMirror.getAnnotationType().toString().equals("picocli.CommandLine.Option")) {
+      final Optional<? extends AnnotationValue> names =
+          getAnnotationValue(annotationMirror, "names");
+      if (names.isPresent() && names.get().getValue().toString().contains("--X")) {
+        final Optional<? extends AnnotationValue> isHidden =
+            getAnnotationValue(annotationMirror, "hidden");
+        if (isHidden.isEmpty() || !((boolean) isHidden.get().getValue())) {
+          return describeMatch(tree);
+        }
+      }
+    }
+    return Description.NO_MATCH;
+  }
+
+  private Optional<? extends AnnotationValue> getAnnotationValue(
+      final AnnotationMirror annotationMirror, final String name) {
+    final Map<? extends ExecutableElement, ? extends AnnotationValue> elementValues =
+        annotationMirror.getElementValues();
+    final Optional<? extends AnnotationValue> retValue =
+        elementValues.keySet().stream()
+            .filter(k -> k.getSimpleName().toString().equals(name))
+            .map(elementValues::get)
+            .findAny();
+    return retValue;
+  }
+}

errorprone-checks/src/test/java/org/hyperledger/errorpronechecks/ExperimentalCliOptionMustBeHiddenTest.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.errorpronechecks;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ExperimentalCliOptionMustBeHiddenTest {
+
+  private CompilationTestHelper compilationHelper;
+
+  @Before
+  public void setup() {
+    compilationHelper =
+        CompilationTestHelper.newInstance(ExperimentalCliOptionMustBeHidden.class, getClass());
+  }
+
+  @Test
+  public void experimentalCliOptionMustBeHiddenPositiveCases() {
+    compilationHelper.addSourceFile("ExperimentalCliOptionNotHiddenPositiveCases.java").doTest();
+  }
+
+  @Test
+  public void experimentalCliOptionMustBeHiddenNegativeCases() {
+    compilationHelper.addSourceFile("ExperimentalCliOptionNotHiddenNegativeCases.java").doTest();
+  }
+
+  @Test
+  public void methodInputParametersMustBeFinalNegativeCases() {
+    compilationHelper.addSourceFile("MethodInputParametersMustBeFinalNegativeCases.java").doTest();
+  }
+
+  @Test
+  public void methodInputParametersMustBeFinalInterfaceNegativeCases() {
+    compilationHelper
+        .addSourceFile("MethodInputParametersMustBeFinalInterfaceNegativeCases.java")
+        .doTest();
+  }
+}

errorprone-checks/src/test/resources/org/hyperledger/errorpronechecks/ExperimentalCliOptionNotHiddenNegativeCases.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.hyperledger.errorpronechecks;
+
+import picocli.CommandLine;
+
+import java.util.Objects;
+
+public class ExperimentalCliOptionNotHiddenNegativeCases {
+
+  @CommandLine.Option(
+          hidden = true,
+          names = {"--Xexperimental"})
+  private String experimental = "";
+
+  @CommandLine.Option(
+          hidden = false,
+          names = {"--notExperimental"})
+  private String notExperimental = "";
+
+  @CommandLine.Option(
+          names = {"--notExperimental2"})
+  private String notExperimental2 = "";
+}

errorprone-checks/src/test/resources/org/hyperledger/errorpronechecks/ExperimentalCliOptionNotHiddenPositiveCases.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.hyperledger.errorpronechecks;
+
+import picocli.CommandLine;
+
+import java.util.Objects;
+
+public class ExperimentalCliOptionNotHiddenPositiveCases {
+
+  // BUG: Diagnostic contains:  Experimental options must be hidden.
+  @CommandLine.Option(
+          hidden = false,
+          names = {"--Xexperimental"})
+  private String experimental = "";
+
+  // BUG: Diagnostic contains:  Experimental options must be hidden.
+  @CommandLine.Option(
+          names = {"--Xexperimental2"})
+  private String experimental2 = "";
+}