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

[converter] Avoid losing generics information in nested types #129

Closed
wants to merge 2 commits into from

Conversation

timothyjward
Copy link
Contributor

The converter doesn't correctly handle conversion of nested generics as type information gets lost from the field/method the object is being converted into.

Signed-off-by: Tim Ward [email protected]

@rotty3000
Copy link
Contributor

@timothyjward I would merge this but apparently it breaks part of the build:

Apache Felix Schematizer Service ................... FAILURE [ 1.295 s]

I'm running mvn clean verify in the converter directory.

@timothyjward
Copy link
Contributor Author

That does appear to be caused by yet further missing type reification in the pipeline.

@rotty3000
Copy link
Contributor

note that without your change the build verifies fine.

@rotty3000
Copy link
Contributor

maybe the schematizer was making assumptions about the missing generic type info.

@timothyjward
Copy link
Contributor Author

maybe the schematizer was making assumptions about the missing generic type info.

All the tests put nested data in the “right form” e.g. a String where a String is expected so the naive conversion to Object is right. My change uses the correct generic type now, fixing some previously broken situations, but still breaks when the type is a variable. This was broken before, but more obviously broken now.

@timothyjward
Copy link
Contributor Author

The second commit goes a lot further and attempts to completely reify generics information for nested conversions. I have included further tests for nested variable usage and generic arrays. I think this is ready to merge now.

@bosschaert
Copy link
Contributor

@timothyjward Thanks! I can take a look into merging this.

@asfgit asfgit closed this in cdbb181 Jan 9, 2018
@bosschaert
Copy link
Contributor

Note that I missed some test classes in the mentioned commit. They were added in a subsequent commit ea17d18

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.

3 participants