jcommander icon indicating copy to clipboard operation
jcommander copied to clipboard

[BUG] Invalid type inference for Sets of Enums

Open yeikel opened this issue 3 years ago • 3 comments

I have the following parameter :

@Parameter(names = { "--roles"}, description = "List of roles separated by comma")
private Set<Relationship.TypeEnum> roles = new HashSet<>(Arrays.asList(Relationship.TypeEnum.values()));

And when I pass a Role as a String, for example --roles "SUPP", it creates a Set<String> instead of a Set<Relationship.TypeEnum>

image

This behavior does not happen with ArrayLists

image

yeikel avatar Nov 26 '20 18:11 yeikel

I will work on this issue.

Harlan1997 avatar May 19 '21 18:05 Harlan1997

I wrote a test to reproduce this problem.

package com.beust.jcommander;

import org.testng.Assert;
import org.testng.annotations.Test;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

public class SetOfEnumTest {

    public enum Season{
        SPRING,
        SUMMER,
        AUTUMN,
        WINTER;
    }

    @Test
    public void testParse()
    {
        class Args {
            @Parameter(names = { "--season"}, description = "List of seasons separated by comma")
            private Set<Season> seasons = new HashSet<>();
        }
        Args args = new Args();
        JCommander.newBuilder()
                .addObject(args)
                .build()
                .parse("--season", "SPRING");
        Assert.assertEquals(Season.class, args.seasons.toArray()[0].getClass());
    }

    public static void main(String[] args) {
        new SetOfEnumTest().testParse();
    }
}

The result of this test is:

java.lang.AssertionError: expected [class java.lang.String] but found [class com.beust.jcommander.SetOfEnumTest$Season]
Expected :class java.lang.String
Actual   :class com.beust.jcommander.SetOfEnumTest$Season

Harlan1997 avatar May 19 '21 18:05 Harlan1997

The cause of this problem is that the current approach to convertValue() in JCommander only considers List conversion. To fix it, convertValue() should consider other collections like Set().

if (type.isAssignableFrom(List.class)) -> if (type.isAssignableFrom(List.class) || type.isAssignableFrom(Set.class))
if (type.isAssignableFrom(List.class) && converter == null) -> if ((type.isAssignableFrom(List.class) || type.isAssignableFrom(Set.class)) && converter == null)

Then it will pass the test.

Harlan1997 avatar May 19 '21 18:05 Harlan1997