Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add @SafeVarargs annotation to Cli.java #54

Closed
wants to merge 1 commit into from

Conversation

jesboat
Copy link
Contributor

@jesboat jesboat commented Aug 17, 2017

Summary:

Cli.CliBuilder and Cli.GroupBuilder have

public CliBuilder<C> withCommands(Class<? extends C> command, Class<? extends C>... moreCommands);

public GroupBuilder<C> withCommands(Class<? extends C> command, Class<? extends C>... moreCommands)

Both of them use the moreCommands array in a typesafe way, which means
they could be annotated with @SafeVarargs. They were not, though, which
generates unchecked warnings at call sites.

Note that the methods must be made final for SafeVarargs to be valid.

A description of SafeVarargs is
https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.7

Test plan: mvn clean verify passes on 8u141

Summary:

Cli.CliBuilder and Cli.GroupBuilder have

    public CliBuilder<C> withCommands(Class<? extends C> command, Class<? extends C>... moreCommands);

    public GroupBuilder<C> withCommands(Class<? extends C> command, Class<? extends C>... moreCommands)

Both of them use the `moreCommands` array in a typesafe way, which means
they could be annotated with @SafeVarargs. They were not, though, which
generates unchecked warnings at call sites.

Note that the methods must be made final for SafeVarargs to be valid.

A description of SafeVarargs is
https://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.6.4.7

Test plan: `mvn clean verify` passes on 8u141
@jesboat
Copy link
Contributor Author

jesboat commented Aug 18, 2017

@dain @electrum @martint

@electrum
Copy link
Member

Merged, thanks!

@electrum electrum closed this Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants