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

fix bug in isSubtypeOf (broken contract X.isSubtypeOf(X)==true) #1157

Merged
merged 6 commits into from
Feb 2, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

There are two implementations of CtTypeInformation#isSubtypeOf(CtTypeReference), which has following comment:

Returns true if the referenced type is a sub-type of the given type.

and two implementations:

  1. CtTypeReferenceImpl#isSubtypeOf
  2. CtTypeImpl#isSubtypeOf

There is bug that each implementation behaves different. See the failing test of this PR.

//contract: x.isSubtypeOf(x)==true
//using CtTypeReference implementation - test pass
assertTrue(xCtType.getReference().isSubtypeOf(xCtType.getReference()));
//using CtType implementation - test fails
assertTrue(xCtType.isSubtypeOf(xCtType.getReference()));

Which behavior is correct? I personally thought that x.isSubtypeOf(x)==true is correct. But I have seen code which expected opposite...

Should we fix it? How?

@monperrus
Copy link
Collaborator

Thanks for the report. They should have the same behavior and x.isSubtypeOf(x)==true is correct. Do you fix it?

@pvojtechovsky
Copy link
Collaborator Author

May be I can fix it this evening. We will see if I will have time.
But if you have time for that, then I can do something else :-)

@pvojtechovsky
Copy link
Collaborator Author

@surli wrote: However we should discuss the implementation of isSubTypeOf for TypeParameters: it wasn't supported until now.

I personally have nothing for this topic. I have not touched that problem/code/need yet. I suggest to discuss it in different PR/Issue, so this PR can be finished fast and I can fix other related problems in FieldReferenceFunction, which is actually broken... I will provide better tests and fixes for FieldReferenceFunction soon.

I commented the test for isSubTypeOf for TypeParameters, so this PR pass all other tests well and can be merged.

@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-20170131.234706-21

New API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-SNAPSHOT

Detected changes: 0.

@monperrus monperrus changed the title test of buggy contract X.isSubtypeOf(X)==true fix bug in isSubtypeOf (broken contract X.isSubtypeOf(X)==true) Feb 1, 2017
@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-20170131.234706-21

New API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-SNAPSHOT

Detected changes: 0.

@surli
Copy link
Collaborator

surli commented Feb 2, 2017

I suggest to discuss it in different PR/Issue, so this PR can be finished fast and I can fix other related problems in FieldReferenceFunction, which is actually broken... I will provide better tests and fixes for FieldReferenceFunction soon.
I commented the test for isSubTypeOf for TypeParameters, so this PR pass all other tests well and can be merged.

Great, indeed it should be done in another PR. I open an issue.

@spoon-bot
Copy link
Collaborator

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-20170201.234549-22

New API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0-SNAPSHOT

Detected changes: 0.

@pvojtechovsky pvojtechovsky deleted the bugIsSubtypeOf branch February 2, 2017 19:29
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.

4 participants