diff --git a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java index 28b5bf0f7..17986e9a3 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java +++ b/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariables.java @@ -171,7 +171,7 @@ public J.DeconstructionPattern visitDeconstructionPattern(J.DeconstructionPatter // skip if defined as a parameter to a lambda expression parent instanceof J.Lambda || // skip if the initializer may have a side effect - initializerMightSideEffect(variable) + mightSideEffect(variable.getInitializer()) ) { return variable; } @@ -181,6 +181,9 @@ public J.DeconstructionPattern visitDeconstructionPattern(J.DeconstructionPatter List assignmentReferences = VariableReferences.findLhsReferences(parentScope.getValue(), variable.getName()); for (Statement ref : assignmentReferences) { if (ref instanceof J.Assignment) { + if (mightSideEffect(((J.Assignment) ref).getAssignment())) { + return variable; + } doAfterVisit(new PruneAssignmentExpression((J.Assignment) ref)); } doAfterVisit(new DeleteStatement<>(ref)); @@ -216,18 +219,14 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return mv; } - private boolean initializerMightSideEffect(J.VariableDeclarations.NamedVariable variable) { - if (variable.getInitializer() == null) { - return false; - } - AtomicBoolean mightSideEffect = new AtomicBoolean(false); - new JavaIsoVisitor() { + private boolean mightSideEffect(Expression initializer) { + return initializer != null && new JavaIsoVisitor() { @Override public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, AtomicBoolean result) { if (SAFE_GETTER_METHODS.matches(methodInvocation)) { return methodInvocation; } - if (withSideEffects == null || Boolean.FALSE.equals(withSideEffects)) { + if (withSideEffects == null || !withSideEffects) { result.set(true); } return methodInvocation; @@ -244,8 +243,7 @@ public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean resul result.set(true); return assignment; } - }.visit(variable.getInitializer(), mightSideEffect); - return mightSideEffect.get(); + }.reduce(initializer, new AtomicBoolean(false)).get(); } }); } diff --git a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java index cbc4ed33c..d1924b241 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RemoveUnusedLocalVariablesTest.java @@ -704,7 +704,7 @@ static void method() { try { used = new Object(); assertEquals(used, null); - unused = new Object(); + unused = used; } catch (Exception e) { // do nothing } @@ -1192,6 +1192,29 @@ void method() { ); } + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/740") + @Test + void doNotRemoveVariableAssignmentWithPotentialSideEffects() { + rewriteRun( + //language=java + java( + """ + class Test { + private String baz() { + String foo; + try { + foo = String.valueOf(1); + } catch (RuntimeException e) { + return "error"; + } + return "ok"; + } + } + """ + ) + ); + } + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/760") @Test void doNotRemoveUnusedPatternMatchingVariables() {