[enhancement] : Implement a multi-pass analysis

[swayamraina]

while working on #57 and #35 i noticed we do not clean up empty methods.

Ideally we should delete methods which result in no-code and also delete their references in parent methods

[mkr-plse]

This is part of a broader problem. For PiranhaJava, multi-pass analysis is not yet implemented.

1. So, if a refactoring results in a method becoming empty, or it returning a constant true or false, the callers have to be refactored appropriately.
2. Similarly, if the return value of a feature flag API is stored in a variable and re-used elsewhere as part of some other conditional expression, that also needs to be refactored appropriately. For example, see code given in point 4 in Extensions for PiranhaJava #28.

I am increasing the scope of this issue to address these two issues.

[romaninozemtsev]

@mkr-plse @swayamraina is there high level explanation on what needs to be done for a multi pass analysis? may be that could help with contributions.

[mkr-plse]

In the current single pass analysis (in PiranhaJava), the code associated with the feature flags based on the configured APIs is removed. However, the code that becomes dead code due to the deletions in this pass are not deleted by the current implemenation of PiranhaJava. The goal of the multi pass analysis is to be able to identify such dead code and delete them. This has to be a multi-pass analysis (as opposed to a two pass analysis) because deletions in the second pass can potentially result in more dead code, and enable further deletions. A few scenarios:

1. Deletion of the code in the first pass can result in unused functions that used to be called from within the deleted code.
2. Same case as above, but unused variables.
3. Wrapper functions that return a boolean constant due to the updates in the first pass. This requires the call sites to be updated, along with deletion of the wrapper functions.
4. Same case as above, except when variables are assigned with a boolean constant value. This requires the usages of the variables to be refactored.

For a stale flag STALE_FLAG, assume it is disabled. Then the refactorings should be as follows:

Case 1:

...
if(isToggleEnabled(STALE_FLAG)) {
    fooOnlyCalledFromHere(); 
} else {
    bar(); 
}
...

void fooOnlyCalledFromHere() { 
...
}

should be refactored to

...
bar()
...

Currently, it is refactored as

...
bar(); 
...

void fooOnlyCalledFromHere() { 
...
}

Case 2:

private boolean willBecomeUnused = false;
...
if(isToggleEnabled(STALE_FLAG)) {
    willBecomeUnused = ... 
} else {
    bar();
}
...

should be refactored to

...
bar()
...

Currently, it is refactored as

private boolean willBecomeUnused = false;
...
bar(); 
...

Case 3:

...
if(shouldBeRefactored()) {
    somecall(); 
} else {
    bar(); 
}
...

boolean shouldBeRefactored() { 
    return isToggleEnabled(STALE_FLAG); 
}

should be refactored to

...
bar()
...

Currently, it is refactored as

...
if(shouldBeRefactored()) {
    somecall(); 
} else {
    bar(); 
}
...

boolean shouldBeRefactored() { 
    return false;
}

The above are a few examples that we see more often in the refactorings that requires additional manual changes and motivates the need for a multi-pass analysis.

@romaninozemtsev Please let me know if that clarifies the problem.

[himanshuguptan]

@mkr-plse @romaninozemtsev Do we have any updates on this milestone for allowing multi-pass refactoring for PiranhaJava?

[mkr-plse]

@himanshuguptan We are working on making this much easier with a newer implementation of Piranha. See here. Please check it out as we expect it to be very customizable and adaptable for various use cases. cc @ketkarameya

[ketkarameya]

@swayamraina @himanshuguptan we have developed the initial version of polyglot piranha. Let us know, if it is performing the "deep" cleans that you want.
If it is not, please feel free to open a new issue describing the expected deep clean. We could potentially add new rules to polyglot piranha to perform the desired deep cleanup.