search

refactoring-ai / predicting-refactoring-ml

Refactoring recommendation via ML
MIT License
28 stars 8 forks source link

Not finding the class in the previous commit #144

Closed mauricioaniche closed 4 years ago

mauricioaniche commented 4 years ago

In the last run, we got an IllegalArgumentException in the buildRefactoringCommitObject. That was caused because the file we were looking for did not exist in the previous commit.

Looking at one instance of the exception, in this commit, the class DataManagerCustomer can't be found. It does exist in the previous version. However, note that it was moved from one package to another, and thus, from one directory to another.

RMiner returned the new path: app/src/main/java/org/apache/fineract/data/datamanager/api/DataManagerCustomer.java.

There are two possibilities here:

  1. A bug in RMiner. It returned the new path instead of the old path.
  2. There might had been multiple refactorings in this file. For a second refactoring, RMiner points to the new file. @jan-gerling, can you double check which refactorings you see in commit 738da55d5b8ac986a63ceb5692ed04c15b1dc864?

If 2 is the case, we have two options:

  1. Ignore it and move on.
  2. Build a map of files, before and after. So that, whenever RMiner returns us a file (that can be pointing to the new one), we always get its old version. For that, we'd have to play with the files in the diff, and their old names. I wonder whether the change I did that removed a lot of code (https://github.com/refactoring-ai/predicting-refactoring-ml/commit/7deec411a0976a50e98ba17cebd393105052ba79) actually caused the bug. Before, we were always using the oldPath as provided by Git.

Log I used to observe this?

/data-collection_worker_1 2020-03-17 10:47:05 ERROR RefactoringAnalyzer:76 Failed to collect commit data for refactored commit: 738da55d5b8ac986a63ceb5692ed04c15b1dc864
java.lang.IllegalArgumentException: No path found in f9b01d3634f66dd34609ba671e38aa7b65bbe53a: app/src/main/java/org/apache/fineract/data/datamanager/api/DataManagerCustomer.java
    at refactoringml.util.JGitUtils.readFileFromGit(JGitUtils.java:46) ~[data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.util.JGitUtils.readFileFromGit(JGitUtils.java:56) ~[data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RefactoringAnalyzer.buildRefactoringCommitObject(RefactoringAnalyzer.java:90) ~[data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RefactoringAnalyzer.collectCommitData(RefactoringAnalyzer.java:60) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.App.processCommit(App.java:206) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.App.run(App.java:134) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RunQueue.processRepository(RunQueue.java:111) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RunQueue.processResponse(RunQueue.java:66) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RunQueue.run(RunQueue.java:57) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
    at refactoringml.RunQueue.main(RunQueue.java:44) [data-collection-0.0.1-SNAPSHOT-jar-with-dependencies.jar:?]
jan-gerling commented 4 years ago

2. There might had been multiple refactorings in this file. For a second refactoring, RMiner points to the new file. @jan-gerling, can you double check which refactorings you see in commit 738da55d5b8ac986a63ceb5692ed04c15b1dc864?

.fineract.ui.online.customers.customerlist.CustomersPresenter"

mauricioaniche commented 4 years ago

Ok, I could reproduce the bug. Indeed, it happens when a Move Class happens together with another refactoring. I'll try to isolate it in a toy repo and fix the bug.

jan-gerling commented 4 years ago

Close it, for now, wait for the results of the last stress test