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

CK not being able to link class/methods with RMiner data #10

Open
mauricioaniche opened this issue Mar 16, 2020 · 10 comments
Open

CK not being able to link class/methods with RMiner data #10

mauricioaniche opened this issue Mar 16, 2020 · 10 comments
Assignees

Comments

@mauricioaniche
Copy link
Contributor

Sometimes, our tool can't link the method that was refactored (coming from RMiner) and the metrics that CK extracts.

Below, we'll show some examples for debugging.

@mauricioaniche mauricioaniche self-assigned this Mar 16, 2020
@jan-gerling
Copy link
Contributor

jan-gerling commented Mar 16, 2020

Examples

https://github.com/fossasia/open-event-droidgen

2020-03-14 01:10:01 ERROR RefactoringAnalyzer:195 CK did not find the refactored method: onLocationChanged/1[Location] for the refactoring type: Rename Parameter on commit 722a2a94adeff14eb44b70806fb8c5395f6e86ce on class org.fossasia.openevent.fragments.MapsFragment
All methods found by CK: , onCreate/1[Bundle]
2020-03-14 01:10:01 ERROR RefactoringAnalyzer:195 CK did not find the refactored method: onViewCreated/2[View,Bundle] for the refactoring type: Extract Method on commit 722a2a94adeff14eb44b70806fb8c5395f6e86ce on class org.fossasia.openevent.fragments.MapsFragment
All methods found by CK: , onCreate/1[Bundle]

https://github.com/lucko/LuckPerms

2020-03-14 01:28:06 ERROR RefactoringAnalyzer:195 CK did not find the refactored method: contains/1[@NonNull Entry] for the refactoring type: Change Parameter Type on commit bafada4f174f14379c76f317b621790224074294 on class me.lucko.luckperms.api.context.ContextSet
All methods found by CK: , mutableCopy/0, isEmpty/0, immutableCopy/0, iterator/0, getValues/1[String], getAnyValue/1[String], size/0, isImmutable/0, toMap/0, isSatisfiedBy/1[ContextSet], contains/1[Entry], toSet/0, toFlattenedMap/0, containsKey/1[String], contains/2[String,String]
2020-03-14 01:28:06 ERROR RefactoringAnalyzer:195 CK did not find the refactored method: add/1[@NonNull Entry] for the refactoring type: Change Parameter Type on commit bafada4f174f14379c76f317b621790224074294 on class me.lucko.luckperms.api.context.ImmutableContextSet.Builder
All methods found by CK: , add/1[Entry], add/2[String,String], build/0, addAll/1[Iterable], addAll/1[ContextSet]
2020-03-14 01:28:06 ERROR RefactoringAnalyzer:195 CK did not find the refactored method: add/1[@NonNull Entry] for the refactoring type: Change Parameter Type on commit bafada4f174f14379c76f317b621790224074294 on class me.lucko.luckperms.api.context.MutableContextSet
All methods found by CK: , fromSet/1[ContextSet], add/1[Entry], addAll/1[ContextSet], remove/2[String,String], clear/0, of/4[String,String,String,String], fromEntries/1[Iterable], removeAll/1[String], create/0, addAll/1[Iterable], of/2[String,String], add/2[String,String]

mauricioaniche referenced this issue in refactoring-ai/predicting-refactoring-ml Mar 17, 2020
@mauricioaniche
Copy link
Contributor Author

  • In commit 722a2a94adeff14eb44b70806fb8c5395f6e86ce, the code doesn't really compile, note the >>>> and ==== indicating that it was a bad merge.

  • In the LuckPerms project, I learned that, when we get the method name from RMiner, it might return annotations. Note how all the failures are related to annotations in the parameters, e.g., contains/1[@NonNull Entry]. We have to remove this annotations, as CK doesn't return them. I just implemented it in refactoring-ai/predicting-refactoring-ml@33e1b00.

mauricioaniche referenced this issue in refactoring-ai/predicting-refactoring-ml Mar 17, 2020
@jan-gerling
Copy link
Contributor

jan-gerling commented Mar 18, 2020

Examples from stress test 05:

/data-collection_worker_3 2020-03-18 02:33:13 ERROR RefactoringAnalyzer:163 CK did not find the refactored method: drawNode/2[Canvas,BinarySearchTree] for the refactoring type: Rename Variable on commit 2de1313a031155fc5c01e5af479eb3c29a7808d2 on class com.toly1994.ds4android.view.BinarySearchView
All methods found by CK: , removeMin/0, onDraw/1[Canvas], removeMax/0, order/1[OrderType], dataView/1[Canvas], init/0, helpView/1[Canvas], remove/1[E], addData/1[E], BinarySearchView/2[Context,AttributeSet], ctrlView/1[Canvas], drawNode/2[Canvas,Node], getMax/0, setOnBSTClickListener/1[OnBSTClickListener], onTouchEvent/1[MotionEvent], getMin/0, BinarySearchView/1[Context], contains/1[E]
/data-collection_worker_2 2020-03-17 11:44:04 ERROR RefactoringAnalyzer:163 CK did not find the refactored method: onLocationChanged/1[Location] for the refactoring type: Rename Parameter on commit 722a2a94adeff14eb44b70806fb8c5395f6e86ce on class org.fossasia.openevent.fragments.MapsFragment
All methods found by CK: , onCreate/1[Bundle]
/data-collection_worker_2 2020-03-17 11:44:04 ERROR RefactoringAnalyzer:163 CK did not find the refactored method: onViewCreated/2[View,Bundle] for the refactoring type: Extract Method on commit 722a2a94adeff14eb44b70806fb8c5395f6e86ce on class org.fossasia.openevent.fragments.MapsFragment
All methods found by CK: , onCreate/1[Bundle]

@mauricioaniche
Copy link
Contributor Author

mauricioaniche commented Mar 19, 2020

For case 1:

If you see the commit, drawNode is defined as
drawNode(Canvas canvas, BinarySearchTree<TreeNode<E>>.Node node). RMiner is returning BinarySearchTree as the type for the node parameter, but it's actually Node. See the output from RMiner: Rename Variable mLineLen : int to lineLen : int in method private drawNode(canvas Canvas, node BinarySearchTree<TreeNode<E>>) : void in class com.toly1994.ds4android.view.BinarySearchView.

(I informed RMiner folks: tsantalis/RefactoringMiner#91)

For case 2:

Code does not compile. Similar to a previous example we had, where you see that it was a bad merge. See commit.

mauricioaniche referenced this issue in refactoring-ai/predicting-refactoring-ml Mar 19, 2020
@mauricioaniche
Copy link
Contributor Author

Example from the stress test 6 (thanks, @jan-gerling):

/data-collection_worker_3 2020-03-19 14:00:34 ERROR RefactoringAnalyzer:198 CK did not find the refactored method: testGetTask/0 for the refactoring type: Change Variable Type on commit 24d7d29d5526749728544c5ba6bb937760988092 on class de.azapps.mirakel.model.query_builder.QueryBuilderTest/null
All methods found by CK: 
/data-collection_worker_3 2020-03-19 14:00:34 ERROR RefactoringAnalyzer:249 We did not find class de.azapps.mirakel.model.query_builder.QueryBuilderTest/null in CK's output 

I'll debug it.

@mauricioaniche
Copy link
Contributor Author

First example that I just debugged: we couldn't find the class, because the refactoring is inside an anonymous class. Linking an anonymous class retuned by CK and by RMiner is tricky. See issue in CK: mauricioaniche/ck#54.

mauricioaniche referenced this issue in refactoring-ai/predicting-refactoring-ml Mar 20, 2020
@mauricioaniche
Copy link
Contributor Author

Same thing for this other example. Anonymous class.

@mauricioaniche
Copy link
Contributor Author

mauricioaniche commented Mar 20, 2020

And now, even from the log, I can see the problem directly from the logs. For example, I see me.prettyprint.cassandra.service.KeyspaceImpl.multigetSlice.getCount in the same project. multigetSlice.getCount is clearly an anonymous class inside these methods.

Maybe this is a feature I should implement right now in CK.

@jan-gerling
Copy link
Contributor

And now, even from the log, I can see the problem directly from the logs. For example, I see me.prettyprint.cassandra.service.KeyspaceImpl.multigetSlice.getCount in the same project. multigetSlice.getCount is clearly an anonymous class inside these methods.

Maybe this is a feature I should implement right now in CK.

That's an important edge case, nice if it is handled.

@mauricioaniche
Copy link
Contributor Author

Shall we close this one? Last one seemed to be about anonymous classes, which we are ignoring now, if I recall correctly, right?

@jan-gerling jan-gerling transferred this issue from refactoring-ai/predicting-refactoring-ml Aug 4, 2020
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

2 participants