OptionUtil: Use ImmutableList<String> instead of String[]

Change Id7911fb6d reduced the severity of ImmutableAnnotationChecker
to WARN because it was failing the build with errors like:

  error: [ImmutableAnnotationChecker] annotations should be immutable:
  'AutoAnnotation_OptionUtil_newOption' has field 'aliases' of type
  'java.lang.String[]', arrays are mutable

Fix this by using ImmutableList<String> instead of String[], and
change the severity of the check back to ERROR.

Change-Id: I72d6044a4d5422bf4e28c051b9a39168769a4d57
This commit is contained in:
David Pursehouse
2019-08-30 11:33:48 +09:00
parent e3eccbd6d8
commit 9ab0d639b0
4 changed files with 18 additions and 13 deletions

View File

@@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
import com.google.common.io.CharStreams; import com.google.common.io.CharStreams;
import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelType;
@@ -349,15 +350,15 @@ public class ReviewCommand extends SshCommand {
private static Option newApproveOption(LabelType type, String usage) { private static Option newApproveOption(LabelType type, String usage) {
return OptionUtil.newOption( return OptionUtil.newOption(
asOptionName(type), asOptionName(type),
new String[0], ImmutableList.of(),
usage, usage,
"N", "N",
false, false,
false, false,
false, false,
LabelHandler.class, LabelHandler.class,
new String[0], ImmutableList.of(),
new String[0]); ImmutableList.of());
} }
private static class LabelSetter implements Setter<Short> { private static class LabelSetter implements Setter<Short> {

View File

@@ -35,10 +35,12 @@
package com.google.gerrit.util.cli; package com.google.gerrit.util.cli;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.util.cli.Localizable.localizable; import static com.google.gerrit.util.cli.Localizable.localizable;
import static java.util.Objects.requireNonNull; import static java.util.Objects.requireNonNull;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap; import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger; import com.google.common.flogger.FluentLogger;
@@ -411,7 +413,8 @@ public class CmdLineParser {
private static Option newPrefixedOption(String prefix, Option o) { private static Option newPrefixedOption(String prefix, Option o) {
requireNonNull(prefix); requireNonNull(prefix);
checkArgument(o.name().startsWith("-"), "Option name must start with '-': %s", o); checkArgument(o.name().startsWith("-"), "Option name must start with '-': %s", o);
String[] aliases = Arrays.stream(o.aliases()).map(prefix::concat).toArray(String[]::new); ImmutableList<String> aliases =
Arrays.stream(o.aliases()).map(prefix::concat).collect(toImmutableList());
return OptionUtil.newOption( return OptionUtil.newOption(
prefix + o.name(), prefix + o.name(),
aliases, aliases,
@@ -421,8 +424,8 @@ public class CmdLineParser {
false, false,
o.hidden(), o.hidden(),
o.handler(), o.handler(),
o.depends(), ImmutableList.copyOf(o.depends()),
new String[0]); ImmutableList.of());
} }
public class MyParser extends org.kohsuke.args4j.CmdLineParser { public class MyParser extends org.kohsuke.args4j.CmdLineParser {
@@ -614,15 +617,15 @@ public class CmdLineParser {
private Option newHelpOption() { private Option newHelpOption() {
return OptionUtil.newOption( return OptionUtil.newOption(
"--help", "--help",
new String[] {"-h"}, ImmutableList.of("-h"),
"display this help text", "display this help text",
"", "",
false, false,
false, false,
false, false,
BooleanOptionHandler.class, BooleanOptionHandler.class,
new String[0], ImmutableList.of(),
new String[0]); ImmutableList.of());
} }
private boolean isHandlerSpecified(OptionDef option) { private boolean isHandlerSpecified(OptionDef option) {

View File

@@ -15,6 +15,7 @@
package com.google.gerrit.util.cli; package com.google.gerrit.util.cli;
import com.google.auto.value.AutoAnnotation; import com.google.auto.value.AutoAnnotation;
import com.google.common.collect.ImmutableList;
import org.kohsuke.args4j.Option; import org.kohsuke.args4j.Option;
import org.kohsuke.args4j.spi.OptionHandler; import org.kohsuke.args4j.spi.OptionHandler;
@@ -24,15 +25,15 @@ public class OptionUtil {
@SuppressWarnings("rawtypes") @SuppressWarnings("rawtypes")
public static Option newOption( public static Option newOption(
String name, String name,
String[] aliases, ImmutableList<String> aliases,
String usage, String usage,
String metaVar, String metaVar,
boolean required, boolean required,
boolean help, boolean help,
boolean hidden, boolean hidden,
Class<? extends OptionHandler> handler, Class<? extends OptionHandler> handler,
String[] depends, ImmutableList<String> depends,
String[] forbids) { ImmutableList<String> forbids) {
return new AutoAnnotation_OptionUtil_newOption( return new AutoAnnotation_OptionUtil_newOption(
name, aliases, usage, metaVar, required, help, hidden, handler, depends, forbids); name, aliases, usage, metaVar, required, help, hidden, handler, depends, forbids);
} }

View File

@@ -53,7 +53,7 @@ java_package_configuration(
"-Xep:FunctionalInterfaceClash:ERROR", "-Xep:FunctionalInterfaceClash:ERROR",
"-Xep:FutureReturnValueIgnored:ERROR", "-Xep:FutureReturnValueIgnored:ERROR",
"-Xep:GetClassOnEnum:ERROR", "-Xep:GetClassOnEnum:ERROR",
"-Xep:ImmutableAnnotationChecker:WARN", "-Xep:ImmutableAnnotationChecker:ERROR",
"-Xep:ImmutableEnumChecker:ERROR", "-Xep:ImmutableEnumChecker:ERROR",
"-Xep:IncompatibleModifiers:ERROR", "-Xep:IncompatibleModifiers:ERROR",
"-Xep:InjectOnConstructorOfAbstractClass:ERROR", "-Xep:InjectOnConstructorOfAbstractClass:ERROR",