openrewrite / rewrite-feature-flags

OpenRewrite recipes for LaunchDarkly.
Apache License 2.0
3 stars 3 forks source link

Remove `boolVariation` feature flag, and associated code branches #5

Closed shanman190 closed 12 months ago

shanman190 commented 1 year ago

This one is going to be a little tricky as we'll have to determine an effective way to pick which side of the evaluation should be lifted.

Case: true side should remain, false side should be removed Before:

import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.server.LDClient;

class Test {
    public void a() {
        LDClient client = new LDClient("sdk-key");
        LDContext context = LDContext.builder("context-key")
                .name("user")
                .build();
        boolean flagValue = client.boolVariation("flag-key-123abc", context, false);
        if (flagValue) {
            // Application code to show the feature
        } else {
            // The code to run if the feature is off
        }
    }
}

After:

import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.server.LDClient;

class Test {
    public void a() {
        LDClient client = new LDClient("sdk-key");
        LDContext context = LDContext.builder("context-key")
                .name("user")
                .build();
        // Application code to show the feature
    }
}

NOTE: There's an additional potential cleanup here if the LDContext or LDClient is no longer used within the current scope, those could be removed as well.

NOTE 2: inverse case should also be possible for a user. We won't be able to know without user input which side of the if to select.

timtebeek commented 1 year ago

We might be able to reuse Simplify constant if branch execution , by replacing the call to the client with a literal true or false, which would then get cleaned up as seen in this test.

timtebeek commented 1 year ago

We recently had some improvements to the existing recipe, such that we should be in an even better state to clean up dead code paths when we replace a flag with true or false.

timtebeek commented 1 year ago

Figured give this a quick first go in

timtebeek commented 12 months ago

This one is implemented as set out above; there might be others to implement still for non boolean feature flag, but I suggest we raise separate issues for those to make it clear what has, and has not been picked up.