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

fixed class hierarchy resolution #91

Closed
wants to merge 1 commit into from
Closed

fixed class hierarchy resolution #91

wants to merge 1 commit into from

Conversation

svenackermann
Copy link

Hi,
I encountered a problem with the resolution of type hierarchies, where a base class in a quite deep hierarchy from a framework was not resolved correctly.
I debugged this a lot and think this change should fix it. It seems quite obvious to me that this is wrong, so I decided to just provide a minimal pull request. It's obvious because resolving only the interfaces for a superclass does not add its own superclass-hierarchy to the context. So, it can happen that not the whole hierarchy is resolved.
I would also provide a test case for that, but unfortunately I'm not able to git push at work. I did this change with github's online editor. I can also not build/test this here, so please excuse that I did not completely follow the contribution guidelines. Maybe you can just see this as a bug report with a solution suggestion.

Hi,
I encountered a problem with the resolution of type hierarchies, where a base class in a quite deep hierarchy from a framework was not resolved correctly. 
I debugged this a lot and think this change should fix it. It seems quite obvious to me that this is wrong, so I decided to just provide a minimal pull request. It's obvious because resolving only the interfaces for a superclass does not add its own superclass-hierarchy to the context. So, it can happen that not the whole hierarchy is resolved.
I would also provide a test case for that, but unfortunately I'm not able to git push at work. I did this change with github's online editor.
codecholeric added a commit that referenced this pull request Jul 20, 2018
…rt would not be resolved correctly.

Issue: #91
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
codecholeric added a commit that referenced this pull request Jul 20, 2018
…rt would not be resolved correctly.

Issue: #91
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
@codecholeric
Copy link
Collaborator

codecholeric commented Jul 20, 2018

Thank you so much for debugging and reporting this!! This is indeed an error, I guess it went unnoticed for so long, because it only shows if you have deeply nested super classes missing from your import. I've taken your fix over in 76dab49, added a test for this and could reproduce it (with sufficiently many levels of hierarchy and the classes missing from the original import 😉)
I've fixed this here: https://github.com/TNG/ArchUnit/releases/tag/v0.8.3
As soon as the sync with Maven Central is through (prolly 1 or 2 hours), you can pull in the new version 0.8.3.
So thank you again for providing such a precise bug report 😃

@codecholeric
Copy link
Collaborator

BTW: A quick "it's fixed with 0.8.3" would be appreciated, I'll keep this PR open until then 😉

@codecholeric
Copy link
Collaborator

@codecholeric codecholeric added this to the 0.8.3 milestone Jul 20, 2018
@svenackermann
Copy link
Author

Thanks! Version 0.8.3 is working for me.

@svenackermann svenackermann deleted the patch-1 branch February 22, 2019 09:02
codecholeric added a commit that referenced this pull request Feb 21, 2021
…rt would not be resolved correctly.

Issue: #91
Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants