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

review1: refactor(role): add CtRole.TYPE_MEMBER #1625

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

pvojtechovsky
Copy link
Collaborator

Adds CtRole.TYPE_MEMBER, which will be needed by #1614 and which makes sense for CtPath clients too.

@tdurieux does the sniper mode need to distinguish between CONSTRUCTOR, FIELD, ... Would not it be simpler to fire change listener even using CtRole.TYPE_MEMBER ???

If it would be simpler, then implementation of CtTypeImpl should fire CtRole.TYPE_MEMBER instead of CONSTRUCTOR, FIELD, METHOD, ... roles.

@monperrus
Copy link
Collaborator

OK to add the role now, the intercession event can be modified later on. One solution is to attach more than one role to an intercession event.

@pvojtechovsky
Copy link
Collaborator Author

Another solution would be to express inheritance between roles.

assertSame(CtRole.TYPE_MEMBER, CtRole.FIELD.getSuperRole())
assertNull(CtRole.TYPE_MEMBER.getSuperRole())

Then we would fire event with CtRole.FIELD - as now, and client might use getSuperRole() to access more generic role if needed.

May be the opposite would be interesting too CtRole CtRole.getSubRole(Class)
CtField field = ...
assertSame(CtRole.FIELD, CtRole.TYPE_MEMBER.getSubRole(field.getClass())

WDYT?

@pvojtechovsky pvojtechovsky changed the title refactor(role): add CtRole.TYPE_MEMBER review1: refactor(role): add CtRole.TYPE_MEMBER Oct 20, 2017
@monperrus
Copy link
Collaborator

That's also an interesting solution, yes. The one with multiple roles by event seems slightly easier (no need to commit to a predefined hierarchy).

@monperrus monperrus merged commit 0156dce into INRIA:master Oct 20, 2017
@pvojtechovsky pvojtechovsky deleted the refAddRoleTYPE_MEMBER branch October 28, 2017 19:50
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