gumtree-spoon-ast-diff icon indicating copy to clipboard operation
gumtree-spoon-ast-diff copied to clipboard

[Feature] add nodes corresponding to imports

Open martinezmatias opened this issue 6 years ago • 6 comments

Proposition of feature (easy to implement).

As we don't have nodes for imports we can have an option to create them. I imagine that It's quite easy to do it: navigate all the TypeReference and create for each of them a new node of type import and as value the Qualified of each TypeReference. (All these new node will we wild of a new Node called imports).

The goal of this feature is to remove the Qualified name from the nodes corresponding to TypeReference and replace it by the SimpleName.

Thus, combining the mentioned changes we will able to correctly detect renames of packages (changes in imports) and (most importantly) avoiding Updates of TypeReference nodes which are children of, for instance, Variables.

WDYT? It makes sense to implement all the mentioned changes?

martinezmatias avatar Aug 20 '19 11:08 martinezmatias

It makes sense, but I'm not sure it would give better results overall. In particular, we would loose the perfectly located changes of types/targets.

Maybe we can implement this as an option?

monperrus avatar Aug 21 '19 05:08 monperrus

This feature is particularly needed in coming/repairability-mode project. Without considering import changes, for example, this commit is only seen as a single update operation that can be predicted by many repair tools. However, in fact, the imported package is not easy for repair tools to predict at all. @martinezmatias @monperrus

khesoem avatar Oct 14 '19 14:10 khesoem

Hi @khaes-kth

this commit is only seen as a single update operation that can be predicted by many repair tools

Do you mean that there are repair tools from the Repairability mode that classify that commit as a patch synthesized by them? if it's the case, we would need to refine the existing repair tools, because that is a transformation that repair tools do not carry out.

martinezmatias avatar Oct 14 '19 14:10 martinezmatias

Yes. For example, the mentioned commit is considered to be an instance of Elixir:update_typre_ref. However, I think in order to be able to address all cases like this, we should also have changes in "import" commands.

khesoem avatar Oct 14 '19 15:10 khesoem

However, I think in order to be able to address all cases like this

Do you have a case that the current implementation is not able to detect?

martinezmatias avatar Oct 14 '19 15:10 martinezmatias

I think if an import command is included in the original (old) version, the imported class can be counted as an accessible repair ingredient in some cases (Can't they?). For example, suppose that in this commit in the new version null was cast to HttpParams (I do not know why they have cast null to String! :D). In such a case, if HttpParams had been imported in the original version, it could have been counted as a fix ingredient that repair tools could use but if it had not been imported, no repair tool could have made such a change because the fix ingredient would not have been accessible to repair tools. So the fact that whether a class is imported in the old version or not makes the case different regarding the fix ingredients that repair tools can use.

(I hope my description is understandable!)

khesoem avatar Oct 14 '19 16:10 khesoem