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

Use generic types in containers (e.g. List, Map) #1396

Closed
mgorniew opened this issue Jul 29, 2021 · 5 comments · Fixed by #1401
Closed

Use generic types in containers (e.g. List, Map) #1396

mgorniew opened this issue Jul 29, 2021 · 5 comments · Fixed by #1401

Comments

@mgorniew
Copy link
Contributor

Hi,

I would like to be able to parse arguments which will map to list for generic types, e.g.

    static final class GenericValue<T> {
        private final T value;
    }

    class App {
        @Parameters(arity = "0..*", converter = GenericValueConverter.class)
        List<GenericValue<?>> values;
    }

This is now not possible. Is this limitation introduced to ensure better type safety? I've modified code to support such use case, but my solution sacrifices types safety by removing information about generic types:

mgorniew@6a6f773

Is such solution acceptable or this approach was considered before and rejected?

Thanks
Michał

@remkop
Copy link
Owner

remkop commented Jul 30, 2021

Hi @mgorniew, the logic in picocli for types only looks up to 2 levels: the first level is for containers like List or Map, and the second level is for the type itself (the type to which the input String values will be converted).

The logic for getting type information is in CommandLine.Model.RuntimeTypeInfo.inferTypes and extractTypeParameters.

It may be quite tricky to change picocli to handle generic types; picocli's type model currently models types in a ITypeInfo data structure that has a "type" and an auxiliary type; and the auxiliary type is used for the converter. For collections, the "type" is the collection and the aux type is the element type, for simple types, both the "type" and the aux type are the simple type. So this does not take into account situations where the aux type is some generic type T that should be ignored by the type converter...

@mgorniew
Copy link
Contributor Author

Simple idea here is to cut any additional generics information from aux types. So in case from my example this means that DciReward<?> will just be treated as DciReward. Such change (as in commit) seems to be backward compatible - all tests (beside ModelTypedMemberTest.testInferTypes ofc) still passes. But this change moves burden of using correct converter on users - e.g. if you will register global convert for DciReward then it will be used for both List<DciReward<Integer>> and List<DciReward<String>>. So local converters (defined in @Option and @Parameters) would be preferred for such types.

@remkop
Copy link
Owner

remkop commented Jul 30, 2021

That works for multi-value options like:

@Parameters(arity = "0..*", converter = GenericValueConverter.class)
 List<GenericValue<?>> values;

Would it also work for single-value options like:

@Parameters(converter = GenericValueConverter.class)
 GenericValue<?> value;

@mgorniew
Copy link
Contributor Author

mgorniew commented Jul 30, 2021

I didn't dig to much into code, but I've added tests (new commit) and it seems to work fine. Should I create PR? We could discuss details there.

@remkop
Copy link
Owner

remkop commented Jul 31, 2021

Okay please create a PR.

(Note I have very little time to work on picocli recently, you may need to be patient.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants