Mosallamy / refdiff-python

Python refactoring extraction tool for GitHub commit history
6 stars 4 forks source link

fix(parser): exclude empty tokens and cstRoot bug #11

Closed rodrigo-brito closed 3 years ago

rodrigo-brito commented 3 years ago

One test failed: image

For the example in the test-data/parser/python/example.py, the parent of the function sum and multi must be the class test and not example.py, right?

rodrigo-brito commented 3 years ago

Now, the plugin detects MOVE and RENAME image

Mosallamy commented 3 years ago

Hi @rodrigo-brito , thanks a lot for your fixes

As for your question, yes test should be the parent of both (sum and multi) and not example.py

But I have a question, in this part we added this condition because the root node should contain only the file.

image

In the example bellow, the file contains a class and the class contains a function, the root should have one node inside it which is the file and the class will be one of the nodes inside the file and the function will be one of the nodes inside the class image

rodrigo-brito commented 3 years ago

If you look in unit tests for Java Plugin, you will see that CST Root should contain all nodes, it is a kind of index of nodes: https://github.com/aserg-ufmg/RefDiff/blob/889b0bfbf2c18726d44f077371966606232cca0b/refdiff-java/src/test/java/refdiff/parsers/java/TestJavaParser.java#L33

The hierarchy of nodes is defined by the parent field, which specifies the parent if it is a child node, and for the root node, we do not have a parent.

The second option. We can define all children's node. For example

Mosallamy commented 3 years ago

The CST below is generated for the code example in your repo: https://github.com/rodrigo-brito/refactoring-python-example

and it works as you described, the function printIsEven is a children node for the main.py

Screen Shot 2021-04-03 at 2 22 13 PM

rodrigo-brito commented 3 years ago

Yes, it looks fine. I will revert this change.

rodrigo-brito commented 3 years ago

Running the tests. Apparently, it includes only the last node, the node three (sum function is missing): image

Mosallamy commented 3 years ago

Oh! I'will check it out and get back to you, maybe the problem is in the parser

rodrigo-brito commented 3 years ago

I will open a pull request with the revert and some fix in unit tests and try to find the bug too.

QassemNa commented 3 years ago

Hi everyone, I have made a pull request to solve the issue for only the last node appering