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.

Sign up for GitHub

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

Inline local variables if assigned from feature flag call #41

Merged
merged 1 commit into from
Dec 22, 2024
Merged

Inline local variables if assigned from feature flag call #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 45 additions & 11 deletions src/main/java/org/openrewrite/featureflags/RemoveBooleanFlag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
import org.openrewrite.*;
import org.openrewrite.analysis.constantfold.ConstantFold;
import org.openrewrite.analysis.util.CursorUtil;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.SemanticallyEqual;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
Expand Down Expand Up @@ -65,25 +67,57 @@ public String getDescription() {
public TreeVisitor<?, ExecutionContext> getVisitor() {
final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, true);
JavaVisitor<ExecutionContext> visitor = new JavaVisitor<ExecutionContext>() {
@Override
public @Nullable J visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
if (multiVariable.getVariables().size() == 1 && matches(multiVariable.getVariables().get(0).getInitializer())) {
// Remove the variable declaration and inline any references to the variable with the literal value
J.Identifier identifierToReplaceWithLiteral = multiVariable.getVariables().get(0).getName();
doAfterVisit(new JavaVisitor<ExecutionContext>() {
@Override
public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) {
if (SemanticallyEqual.areEqual(ident , identifierToReplaceWithLiteral)) {
return buildLiteral().withPrefix(ident.getPrefix());
}
return ident;
}
});
cleanUpAfterReplacements();
return null;
}
return super.visitVariableDeclarations(multiVariable, ctx);
}

@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
if (methodMatcher.matches(mi) && isFeatureKey(mi.getArguments().get(0))) {
doAfterVisit(new SimplifyConstantIfBranchExecution().getVisitor());
doAfterVisit(Repeat.repeatUntilStable(new RemoveUnusedLocalVariables(null, true).getVisitor(), 3));
doAfterVisit(new RemoveUnusedPrivateFields().getVisitor());
J.Literal literal = new J.Literal(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, replacementValue, String.valueOf(replacementValue), null, JavaType.Primitive.Boolean);
return literal.withPrefix(mi.getPrefix());
if (matches(mi)) {
cleanUpAfterReplacements();
return buildLiteral().withPrefix(mi.getPrefix());
}
return mi;
}

private boolean isFeatureKey(Expression firstArgument) {
return CursorUtil.findCursorForTree(getCursor(), firstArgument)
.bind(c -> ConstantFold.findConstantLiteralValue(c, String.class))
.map(featureKey::equals)
.orSome(false);
private boolean matches(@Nullable Expression mi) {
if (methodMatcher.matches(mi)) {
Expression firstArgument = ((J.MethodInvocation) mi).getArguments().get(0);
return CursorUtil.findCursorForTree(getCursor(), firstArgument)
.bind(c -> ConstantFold.findConstantLiteralValue(c, String.class))
.map(featureKey::equals)
.orSome(false);
}
return false;
}

private void cleanUpAfterReplacements() {
doAfterVisit(new SimplifyConstantIfBranchExecution().getVisitor());
doAfterVisit(Repeat.repeatUntilStable(new RemoveUnusedLocalVariables(null, true).getVisitor(), 3));
doAfterVisit(new RemoveUnusedPrivateFields().getVisitor());
}

private J.Literal buildLiteral() {
return new J.Literal(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, replacementValue, String.valueOf(replacementValue), null, JavaType.Primitive.Boolean);
}

};
return Preconditions.check(new UsesMethod<>(methodMatcher), visitor);
}
Expand Down
55 changes: 44 additions & 11 deletions src/main/java/org/openrewrite/featureflags/RemoveStringFlag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.jspecify.annotations.Nullable;
import org.openrewrite.*;
import org.openrewrite.analysis.constantfold.ConstantFold;
import org.openrewrite.analysis.util.CursorUtil;
import org.openrewrite.java.JavaVisitor;
import org.openrewrite.java.MethodMatcher;
import org.openrewrite.java.search.SemanticallyEqual;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.Expression;
import org.openrewrite.java.tree.J;
Expand Down Expand Up @@ -65,24 +67,55 @@ public String getDescription() {
public TreeVisitor<?, ExecutionContext> getVisitor() {
final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, true);
JavaVisitor<ExecutionContext> visitor = new JavaVisitor<ExecutionContext>() {
@Override
public @Nullable J visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) {
if (multiVariable.getVariables().size() == 1 && matches(multiVariable.getVariables().get(0).getInitializer())) {
// Remove the variable declaration and inline any references to the variable with the literal value
J.Identifier identifierToReplaceWithLiteral = multiVariable.getVariables().get(0).getName();
doAfterVisit(new JavaVisitor<ExecutionContext>() {
@Override
public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) {
if (SemanticallyEqual.areEqual(ident , identifierToReplaceWithLiteral)) {
return buildLiteral().withPrefix(ident.getPrefix());
}
return ident;
}
});
cleanUpAfterReplacements();
return null;
}
return super.visitVariableDeclarations(multiVariable, ctx);
}

@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
J.MethodInvocation mi = (J.MethodInvocation) super.visitMethodInvocation(method, ctx);
if (methodMatcher.matches(mi) && isFeatureKey(mi.getArguments().get(0))) {
doAfterVisit(new SimplifyConstantIfBranchExecution().getVisitor());
doAfterVisit(Repeat.repeatUntilStable(new RemoveUnusedLocalVariables(null, true).getVisitor(), 3));
doAfterVisit(new RemoveUnusedPrivateFields().getVisitor());
J.Literal literal = new J.Literal(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, replacementValue, '"' + replacementValue + '"', null, JavaType.Primitive.String);
return literal.withPrefix(mi.getPrefix());
if (matches(mi)) {
cleanUpAfterReplacements();
return buildLiteral().withPrefix(mi.getPrefix());
}
return mi;
}

private boolean isFeatureKey(Expression firstArgument) {
return CursorUtil.findCursorForTree(getCursor(), firstArgument)
.bind(c -> ConstantFold.findConstantLiteralValue(c, String.class))
.map(featureKey::equals)
.orSome(false);
private boolean matches(@Nullable Expression expression) {
if (methodMatcher.matches(expression)) {
Expression firstArgument = ((J.MethodInvocation) expression).getArguments().get(0);
return CursorUtil.findCursorForTree(getCursor(), firstArgument)
.bind(c -> ConstantFold.findConstantLiteralValue(c, String.class))
.map(featureKey::equals)
.orSome(false);
}
return false;
}

private void cleanUpAfterReplacements() {
doAfterVisit(new SimplifyConstantIfBranchExecution().getVisitor());
doAfterVisit(Repeat.repeatUntilStable(new RemoveUnusedLocalVariables(null, true).getVisitor(), 3));
doAfterVisit(new RemoveUnusedPrivateFields().getVisitor());
}

private J.Literal buildLiteral() {
return new J.Literal(Tree.randomId(), Space.SINGLE_SPACE, Markers.EMPTY, replacementValue, '"' + replacementValue + '"', null, JavaType.Primitive.String);
}
};
return Preconditions.check(new UsesMethod<>(methodMatcher), visitor);
Expand Down
13 changes: 7 additions & 6 deletions src/test/java/org/openrewrite/featureflags/RemoveBooleanFlagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void customMethodPatternForWrapper() {
java(
"""
package com.acme.bank;

public class CustomLaunchDarklyWrapper {
public boolean featureFlagEnabled(String key, boolean fallback) {
return fallback;
Expand All @@ -50,7 +50,8 @@ public boolean featureFlagEnabled(String key, boolean fallback) {
class Foo {
private CustomLaunchDarklyWrapper wrapper = new CustomLaunchDarklyWrapper();
void bar() {
if (wrapper.featureFlagEnabled("flag-key-123abc", false)) {
boolean enabled = wrapper.featureFlagEnabled("flag-key-123abc", false);
if (enabled) {
// Application code to show the feature
System.out.println("Feature is on");
}
Expand Down Expand Up @@ -84,7 +85,7 @@ void customMethodPatternNoConstants() {
package com.osd.util;
import java.util.Map;
import java.util.HashMap;

public class ToggleChecker {
public boolean isToggleEnabled(String toggleName, boolean fallback) {
Map<String,Boolean> toggleMap = new HashMap<>();
Expand Down Expand Up @@ -134,7 +135,7 @@ void removeWhenFeatureFlagIsAConstant() {
java(
"""
package com.acme.bank;

public class CustomLaunchDarklyWrapper {
public boolean featureFlagEnabled(String key, boolean fallback) {
return fallback;
Expand All @@ -149,7 +150,7 @@ public boolean featureFlagEnabled(String key, boolean fallback) {
import com.acme.bank.CustomLaunchDarklyWrapper;
class Foo {
private static final String FEATURE_TOGGLE = "flag-key-123abc";

private CustomLaunchDarklyWrapper wrapper = new CustomLaunchDarklyWrapper();
void bar() {
if (wrapper.featureFlagEnabled(FEATURE_TOGGLE, false)) {
Expand Down Expand Up @@ -183,7 +184,7 @@ void removeUnnecessaryTernary() {
java(
"""
package com.acme.bank;

public class CustomLaunchDarklyWrapper {
public boolean featureFlagEnabled(String key, boolean fallback) {
return fallback;
Expand Down
4 changes: 1 addition & 3 deletions src/test/java/org/openrewrite/featureflags/RemoveStringFlagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package org.openrewrite.featureflags;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.java.JavaParser;
Expand Down Expand Up @@ -62,8 +61,7 @@ void bar() {
"""
class Foo {
void bar() {
String topic = "topic-456";
System.out.println("Publishing to topic: " + topic);
System.out.println("Publishing to topic: " + "topic-456");
}
}
"""
Expand Down
14 changes: 6 additions & 8 deletions src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveBoolVariationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.jupiter.api.Test;
import org.openrewrite.DocumentExample;
import org.openrewrite.InMemoryExecutionContext;
import org.openrewrite.Issue;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;
Expand Down Expand Up @@ -77,10 +78,10 @@ void keyInConstant() {
"""
import com.launchdarkly.sdk.LDContext;
import com.launchdarkly.sdk.server.LDClient;

class Foo {
private static final String FEATURE_FLAG_123ABC = "flag-key-123abc";

private LDClient client = new LDClient("sdk-key-123abc");
void bar() {
LDContext context = null;
Expand Down Expand Up @@ -333,6 +334,7 @@ void bar() {
);
}

@Issue("https://github.com/openrewrite/rewrite-feature-flags/issues/40")
@Test
void localVariablesNotInlined() {
// language=java
Expand All @@ -357,12 +359,8 @@ void bar() {
"""
class Foo {
void bar() {
// Local variables not yet inlined
boolean flagEnabled = true;
if (flagEnabled) {
// Application code to show the feature
System.out.println("Feature is on");
}
// Application code to show the feature
System.out.println("Feature is on");
}
}
"""
Expand Down
3 changes: 1 addition & 2 deletions src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveStringVariationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ void bar() {
"""
class Foo {
void bar() {
String topic = "topic-456";
System.out.println("Publishing to topic: " + topic);
System.out.println("Publishing to topic: " + "topic-456");
}
}
"""
Expand Down
Loading