SpoonLabs / coming

A tool for mining commits from Git repositories and diffs to automatically extract code change pattern instances and features with ast analysis
https://hal.inria.fr/hal-00861883/file/paper-short.pdf
MIT License
92 stars 33 forks source link

[Feature] Considering inheritance relation between actions. #180

Open khaes-kth opened 5 years ago

khaes-kth commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

khaes-kth commented 5 years ago

This feature is added in #182

martinezmatias commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Hi @khaes-kth

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

It seems perfect! Thanks a lot. Matias