havardh / javaflow

Java model to flowtype converter
22 stars 5 forks source link

fix: convert wildcard type params to "any" #70

Closed pascalgn closed 5 years ago

pascalgn commented 5 years ago

Please don't ask me why, but we do have fields like private List<?> list in our model and changing them was actually more effort than I thought :)

Not 100 % sure about the implementation, maybe we need another class Wildcards instead of adding the check directly in JavaFlowConverter, but I thought that might be too much.

havardh commented 5 years ago

Would it be too strict to say that this would be achievable by adding a custom type substitution?

?: any
<typename>.?: any

We could surely document this use case. I do think that if we would want to add this we would do some more work so that we would have an internal base implementation for other languages too.

pascalgn commented 5 years ago

The way I implemented it, this should still work, at least for specific packages.

So for example if someone knows they have List<?> but actually always use it as if it was List<String>, they would still be able to add com.example.?: string to their custom types.

However, as a general fallback I think ? -> any would still be a good idea, as not having it will produce field: List<?>, which is invalid

havardh commented 5 years ago

I need to consider this more in depth and come back to you 👍

havardh commented 5 years ago

I've added https://github.com/havardh/javaflow/pull/72 as my proposal to add this, it does not provide the built-in '?'. But I'm getting less confident about requiring that change. I'll let it be up to you to pick your favorite out of these implementations.

pascalgn commented 5 years ago

I mostly think this is a good idea. I'm not 100 % sure, because ? is actually nothing the user should care about -- almost every time, they would expect it to become any, I would assume. However, there are also some more types like BigDecimal, BigInteger, Date, etc. where we could provide defaults like number and string, so that might be a good place to also add ?: any then. In addition, your proposal also allows users to easily handle cases like List<? extends Serializable>