Skip to content

Workaround for #543, test bounds when doing dodgy generic type inferencing #544

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

Closed
wants to merge 2 commits into from

Conversation

ahgittin
Copy link

No description provided.

@ahgittin
Copy link
Author

Workaround for #543 (and test case which breaks before the workaround but passes after).

Note this does not fix the underlying cause, but it does mean people (by which I mean me) are less impacted by the quick and dirty behaviour of SimpleType._narrow !

@cowtowncoder
Copy link
Member

Sounds reasonable. One of high-priority things that would help is to move to using ClassMate (https://github.com/cowtowncoder/java-classmate), which handles type resolution properly, and has no known problems, compared to Jackson's own (and older, predating ClassMate, which I wrote as kind of "jackson type resolution v2"). But unfortunately doing this while retaining backwards-compatibility is not an easy task.

Until then, this sounds like a reasonable improvement.

One practical thing first however: we need CLAs from all contributors (one is enough for any number of contributions over any Jackson project):

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

would it be possible to fill the one-pager, scan, send image file to info at "fasterxml.com"? I know it's hassle, but we have to do this to keep corporate users happy etc.

Big thank you in advance for reporting this, tracking the problem & figuring out a reasonable work-around.

@cowtowncoder
Copy link
Member

Actually, I think I can get the gist of the issue here and use the test, then do similar fix to work around the problem. Will do that manually, closing this PR, but end effect should be the same. Thanks!

@ahgittin
Copy link
Author

have submitted CLA @cowtowncoder - feel free to use the code if it's helpful

classmate looks cool. i shuddered to think about implementing the proper fix here after a few minutes thought! agree classmate is the right approach.

@cowtowncoder
Copy link
Member

@ahgittin Perfect, thanks! And yes, type resolution and handling are quite tricky areas -- classmate works better because it started from scratch, but after I had learnt a lot and was able to resolve things properly, with no simplifications or short-cuts. Ironically code itself ended to be probably more compact and maybe even simpler than what Jackson uses... live & learn. :-)

@ahgittin
Copy link
Author

yes i believe covariance is a mind-altering drug. i usually avoid it. but you're packaging it into a tasty little pill!

@cowtowncoder
Copy link
Member

Yes! Thanks for CLA, simplifies things and can quickly integrate patches in future. In this case I did manual merge & backported fix in 2.4 and 2.3 as well.

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

Successfully merging this pull request may close these issues.

2 participants