diff --git a/src/main/java/org/openrewrite/featureflags/RemoveBooleanFlag.java b/src/main/java/org/openrewrite/featureflags/RemoveBooleanFlag.java index 16a92f4..8c98c60 100644 --- a/src/main/java/org/openrewrite/featureflags/RemoveBooleanFlag.java +++ b/src/main/java/org/openrewrite/featureflags/RemoveBooleanFlag.java @@ -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; @@ -65,25 +67,57 @@ public String getDescription() { public TreeVisitor getVisitor() { final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, true); JavaVisitor visitor = new JavaVisitor() { + @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() { + @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); } diff --git a/src/main/java/org/openrewrite/featureflags/RemoveStringFlag.java b/src/main/java/org/openrewrite/featureflags/RemoveStringFlag.java index 83fcedb..7cf74a3 100644 --- a/src/main/java/org/openrewrite/featureflags/RemoveStringFlag.java +++ b/src/main/java/org/openrewrite/featureflags/RemoveStringFlag.java @@ -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; @@ -65,24 +67,55 @@ public String getDescription() { public TreeVisitor getVisitor() { final MethodMatcher methodMatcher = new MethodMatcher(methodPattern, true); JavaVisitor visitor = new JavaVisitor() { + @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() { + @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); diff --git a/src/test/java/org/openrewrite/featureflags/RemoveBooleanFlagTest.java b/src/test/java/org/openrewrite/featureflags/RemoveBooleanFlagTest.java index 044fbf3..c087726 100644 --- a/src/test/java/org/openrewrite/featureflags/RemoveBooleanFlagTest.java +++ b/src/test/java/org/openrewrite/featureflags/RemoveBooleanFlagTest.java @@ -34,7 +34,7 @@ void customMethodPatternForWrapper() { java( """ package com.acme.bank; - + public class CustomLaunchDarklyWrapper { public boolean featureFlagEnabled(String key, boolean fallback) { return fallback; @@ -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"); } @@ -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 toggleMap = new HashMap<>(); @@ -134,7 +135,7 @@ void removeWhenFeatureFlagIsAConstant() { java( """ package com.acme.bank; - + public class CustomLaunchDarklyWrapper { public boolean featureFlagEnabled(String key, boolean fallback) { return fallback; @@ -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)) { @@ -183,7 +184,7 @@ void removeUnnecessaryTernary() { java( """ package com.acme.bank; - + public class CustomLaunchDarklyWrapper { public boolean featureFlagEnabled(String key, boolean fallback) { return fallback; diff --git a/src/test/java/org/openrewrite/featureflags/RemoveStringFlagTest.java b/src/test/java/org/openrewrite/featureflags/RemoveStringFlagTest.java index 991591f..0dcb5a6 100644 --- a/src/test/java/org/openrewrite/featureflags/RemoveStringFlagTest.java +++ b/src/test/java/org/openrewrite/featureflags/RemoveStringFlagTest.java @@ -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; @@ -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"); } } """ diff --git a/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveBoolVariationTest.java b/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveBoolVariationTest.java index 1b9566d..c2b2448 100644 --- a/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveBoolVariationTest.java +++ b/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveBoolVariationTest.java @@ -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; @@ -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; @@ -333,6 +334,7 @@ void bar() { ); } + @Issue("https://github.com/openrewrite/rewrite-feature-flags/issues/40") @Test void localVariablesNotInlined() { // language=java @@ -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"); } } """ diff --git a/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveStringVariationTest.java b/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveStringVariationTest.java index 532a40b..9f182a3 100644 --- a/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveStringVariationTest.java +++ b/src/test/java/org/openrewrite/featureflags/launchdarkly/RemoveStringVariationTest.java @@ -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"); } } """