tsantalis / RefactoringMiner

MIT License
351 stars 135 forks source link

Problematic Inline Method refactorings #490

Open pedramnoori opened 1 year ago

pedramnoori commented 1 year ago

Problem

Mapping and replacement inaccuracy within around 10 Inline Method refactoring cases

Commit

https://github.com/TeamAmaze/AmazeFileManager/commit/185b497c12692a2ff17020b7ab65c01f7afe6b59

Description

There are seven instances of the Inline Method refactoring, all of which share the same description: "Inline Method public getString(c Context, a int): String inlined to public openWith(f File, c Context): void in class com.amaze.filemanager.utils.Futils"

None of these instances have a mapped statement 482 in the child version ("String[] items=new String[] {..}"), which is clearly a problem.

If you go through the Inline Method refactoring cases, within the very first instance of the above mentioned cases, the reported replacement for this: "return c.getResources().getString(a); ---> a.title(c.getResources().getString(R.string.openas));" mapping is as follows:

"c.getResources().getString(R.string.openas) -> c.getResources().getString(R.string.openas)" (with the type of argument replaced with return)

Which is perfect considering the sub-expression mapping happened. But in the other six cases, a different (seemingly wrong) replacement has been reported, which causes the PurityChecker to fail to report these cases as pure.

Worth mentioning that same problems goes for the other Inline Method refactorings, with the description of: "Inline Method public getString(c Context, a int) : String inlined to public showProps(hFile BaseFile, perm String, c Main, root boolean) : void in class com.amaze.filemanager.utils.Futils"

pedramnoori commented 1 year ago

As @tsantalis suggested, there are 7 calls within method openWith(), so the mapping algorithm between the inlined method and openWith() runs 7 times, but in all cases it attempts to match with statement: a.title(c.getResources().getString(R.string.openas));

However, in each case the method call is different, and thus the parameterToArgumentMap changes.

For example, in the second call R.string.text corresponds to parameter a

As a result, it matches statement a.title(c.getResources().getString(R.string.openas)); by replacing the entire method call.

To fix this case, we should improve the scope of the method calls to go within a smaller part of a statement.