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

[Feature] Considering inheritance relation between actions. #180

Open
khaes-kth opened this issue Oct 14, 2019 · 9 comments
Open

[Feature] Considering inheritance relation between actions. #180

khaes-kth opened this issue Oct 14, 2019 · 9 comments

Comments

@khaes-kth
Copy link
Contributor

khaes-kth commented Oct 14, 2019

Hi,

Currently, a change is an instance of a pattern only if the mentioned entity type in the pattern specification is exactly the same as the diff entity type. See HERE. However, it would be interesting if we could define a pattern by mentioning an entity type and retrieve all changes with that type of entity or any type of entity that extends the mentioned type.

For example, this change includes an invocation update (minRatioPositions.add->Precision.equals). However, it is also an instance of expression update because invocation extends expression.
It would be interesting to define a pattern as follows and detect all instances of expression updates (including invocation updates such as the mentioned one):

<pattern name="expression_replacement">
    <entity id="1" type="Expression" />
    <action entityId="1" type="UPD"/>
</pattern>

Currently, this pattern specification does not recognize this change as a detected instance.

@martinezmatias
Copy link
Collaborator

Hi @khaes-kth

it would be interesting if we could define a pattern by mentioning an entity type and retrieve all changes with that type of entity or any type of entity that extends the mentioned type.

Yes, I completely agree that we need that functionality.
One simple option that came to my mind is to create that hierarchy statically (in a Json File and load each time the tool starts, or even in the hardcoded). So that structure could be, for each element, the list of parent in the hierarchy.

@khaes-kth
Copy link
Contributor Author

Yes, this is an option. But what about the following dynamic option?
1- Use reflection to find the Gumtree class (such as CtExpression) corresponding to the pattern entity type.
2- Check if the found class is an instanceof the type of the diff entity. Here, parentEntity.getEntityType() gives the pattern entity type and currentNodeFromAction is the diff entity.

@monperrus
Copy link
Contributor

monperrus commented Oct 17, 2019

per today's discussion, @khaes-kth will implement a first version

@khaes-kth
Copy link
Contributor Author

This feature is added in #182

@martinezmatias
Copy link
Collaborator

Hi @khaes-kth

I reopen the issue, until PR be merged.
BTW: which solution have you implemented? (The PR has many of commits for review)

@khaes-kth
Copy link
Contributor Author

Hi @martinezmatias ,

I reopen the issue, until PR be merged.

OK, Good.

BTW: which solution have you implemented? (The PR has many of commits for review)

As you suggested, the static solution is implemented. The relations are stored here and used here and here.

@martinezmatias
Copy link
Collaborator

Hi @khaes-kth
It seems perfect.
One minor refactor I propose is to rename the entity GumtreeHelper . IMHO, including Gumtree in the name is confusing because Gumtree is the algorithm to compute ast diff and this helper focuses on determining relations between the types of an AST.
I would propose something similar to EntityTypeRelationshipResolver.
WDYT?

@khaes-kth
Copy link
Contributor Author

You are right. It is confusing. However, I had chosen that name because that class not only retrieved entity type relations, but it also had several other functions like this and this.
I just renamed the GumtreeHelper class to EntityTypesInfoResolver and moved the getOperationStats function to the class where it is used.
Do you think it should have been refactored in a different way?

@martinezmatias
Copy link
Collaborator

Hi @khaes-kth

Do you think it should have been refactored in a different way?

It seems perfect!
Thanks a lot.
Matias

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

No branches or pull requests

3 participants