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

For WildcardType, use Types of upper bounds (i.e. erase wildcard) #244

Merged
merged 2 commits into from Nov 3, 2015

Conversation

Projects
None yet
2 participants
@dbevacqua
Contributor

dbevacqua commented Nov 2, 2015

If I try to use JaVers to compare classes with upper-bounded wildcard types, I get an exception with message like the following:

beaconJaversException: GENERIC_TYPE_NOT_PARAMETRIZED JaVers runtime error - 
expected actual Class argument in type 'java.util.Set<? extends Foo>'. 
JaVers is strongly-typed and needs to know actual Class of elements stored in your collections. 

with an instruction to:

Try at least <Object>. Wildcards (e.g. <?>), unbounded type parameters (e.g. <T>) and raw types (e.g. List) are not supported.

For reasons, we cannot easily alter our model to narrow to Set<Foo> or Set<SpecificSubclassOfFoo>, and anyway we would prefer not to let our choice of this (admittedly wonderful) tool drive such things.

The error actually does not say that bounded parameters are not supported, so this pull request - treating wildcards with an upper bound as having the type of the upper bound - seems consistent with it. Certainly the tests still pass, and the behaviour is as we would expect.

In addition (and perhaps this should be the subject of another pull request), a more general change to include unbounded types (which seems like it's the same as "Try at least ") might be worth considering, though such a change would break your tests (but only the ones that expect the GENERIC_TYPE_NOT_PARAMETRIZED error).

We've only recently started using JaVers, and our usage may be fairly narrow, so it's quite possible we've missed some obvious reason why this is not a good idea, but would appreciate if you would consider it.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 2, 2015

You are right, upper-bounded wildcard should be treated as bound type, so your Pull goes in right direction. Thanks for contributing. Could you please add test case (in CollectionTypeTest) which will cover this feature?
like "should treat wildcards with an upper bound as the type of its upper bound"()

Considering your second issue about allowing unbounded wildcards, we could disqus it, but as you said, there should be separate issue/pull for this.

@dbevacqua

This comment has been minimized.

Contributor

dbevacqua commented Nov 3, 2015

Great, thanks. I've added a test to this effect.

@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 3, 2015

Excellent!

bartoszwalacik added a commit that referenced this pull request Nov 3, 2015

Merge pull request #244 from treatwell/use-wildcard-upper-bound
For WildcardType, use Types of upper bounds (i.e. erase wildcard)

@bartoszwalacik bartoszwalacik merged commit 3316080 into javers:master Nov 3, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bartoszwalacik

This comment has been minimized.

Member

bartoszwalacik commented Nov 3, 2015

going to release it tomorrow morning

@dbevacqua

This comment has been minimized.

Contributor

dbevacqua commented Nov 4, 2015

Great - thank you!

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