cbeust/jcommander

[BUG] Invalid type inference for Sets of Enums

yeikel opened this issue · 4 comments

yeikel commented

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

Harlan1997 commented

I will work on this issue.

Harlan1997 commented

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 commented

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.