[PiranhaJava] Ability to detect and removed stale code for specific use-cases

[sivacoder]

In the past we have made PR's to Piranha to support additional usage patterns. Also, thanks to all the new functionalities added to Piranha Java last few months.

We are in the final stage of putting Piranha to pre-production for our java based micro services. We have hit this two blockers and need your support for this:

Case 1: Tests using the mocked frameworks (mockito) where the same feature flag is used
when(featureUtil.isFlagEnabled("stale_flag").thenReturn(true);

is converted to

when(true).thenReturn(true);

With this change, build is failing as Mockito is classified as "Unnecessary Mocking". Any way to get going on this?

Case 2: Removing Unused Imports (Static and Non-Static) after Piranha cleaning up:

Imagine a code like

import com.test.FlagUtil;
if (flagUtil.isFlagEnabled("stale_flag")) {
// Do some job
}

is converted to
import com.test.FlagUtil;
// Do some job

But can the unused imports also be removed (static and non-static) ?

[mkr-plse]

Case 1: Tests using the mocked frameworks (mockito) where the same feature flag is used

In an email, @pranavsb had proposed the following solution:

So a config field to specify strings like "accept" and "when" (usual starting methods for testcases, can use FQNs as well).
And then I'm guessing in "matchMethod" we check for this field and check that the subtree contains a treated or control XP API and delete that line.

To elaborate further on that, update the properties.json file to accept an additional configuration for test methods which will wrap an XP API, and then check for those methods in matchMethod and if those method contains XP API invocation, delete the entire invocation. This is a stop gap until we have multi pass analysis (see here and [here])(https://github.com/uber/piranha/milestone/1).

Case 2: Removing Unused Imports (Static and Non-Static) after Piranha cleaning up:

Yes. Enabling RemoveUnusedImports errorprone check should help here.

[sivacoder]

Thanks @mkr-plse for the inputs. On #1, we tried some options, but the implementation was getting complicated.

When we intercept matchMethod, we are getting the complete statement.
For example: org.mockito.Mockito.when(experimentation.isToggleDisabled(STALE_FLAG)).thenReturn(false)

From this, we are able to extract the methodName as org.mockito.Mockito.when, but checking if when arguments matches the stale_flag we had to do multiple iterations of getMethodSelect() getExpression() and so on and was becoming very specific to our use-case and not generic.

Any suggestion to take this forward?

[mkr-plse]

If you call evalExpr on the argument expression to when and if that evaluation either returns Value.TRUE or Value.FALSE, you can remove the entire method expression.

Specifically, expression representing experimentation.isToggleDisabled(STALE_FLAG) will be passed to evalExpr which internally will then evaluate it to a boolean constant.

[sivacoder]

Interesting, thanks @mkr-plse . Let me try evalExpr method. Also, is there a standard way to extract the outer method name?

Ie., in this example of org.mockito.Mockito.when(experimentation.isToggleDisabled(STALE_FLAG)).thenReturn(false),
to extract out org.mockito.Mockito.when, is there also a standard way to get the method name as FQN?

[mkr-plse]

Also, is there a standard way to extract the outer method name?

See getXPAPI.

[ketkarameya]

@sivacoder We have implemented the functionality discussed in this issue, and it is available for use in our latest release(0.1.6).

[ketkarameya]

Closing this issue. Please reopen if any problems arise.