Skip to content

Redeclare variables in new blocks when running MinimumSwitchCases #689

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Redeclare variables in new blocks when running MinimumSwitchCases #689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
baf7b11
Fixed the provided failing test
Jenson3210 Aug 5, 2025
55e7840
Update src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCase…
Jenson3210 Aug 5, 2025
ca643b8
Fixed the provided failing test
Jenson3210 Aug 5, 2025
3269a39
Review comments
Jenson3210 Aug 6, 2025
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
53 changes: 46 additions & 7 deletions src/main/java/org/openrewrite/staticanalysis/MinimumSwitchCases.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@
import org.openrewrite.marker.Markers;
import org.openrewrite.staticanalysis.csharp.CSharpFileChecker;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.*;

import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -192,6 +189,26 @@ public J visitSwitch(J.Switch switch_, ExecutionContext ctx) {
}
}

Map<String, J.VariableDeclarations> firstCaseDeclarations = new JavaIsoVisitor<Map<String, J.VariableDeclarations>>() {
@Override
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, Map<String, J.VariableDeclarations> variableDeclarations) {
for (J.VariableDeclarations.NamedVariable var_ : multiVariable.getVariables()) {
variableDeclarations.put(var_.getSimpleName(), multiVariable.withVariables(ListUtils.filter(multiVariable.getVariables(), v -> v == var_)));
}
return multiVariable;
}

@Override
public J.Block visitBlock(J.Block block, Map<String, J.VariableDeclarations> variableDeclarations) {
for (Statement statement : block.getStatements()) {
if (statement instanceof J.VariableDeclarations) {
visitVariableDeclarations((J.VariableDeclarations) statement, variableDeclarations);
}
}
return block;
}
}.reduce(cases[0], new HashMap<>());

// move first case to "if"
List<Statement> thenStatements = getStatements(cases[0]);

Expand All @@ -201,13 +218,36 @@ public J visitSwitch(J.Switch switch_, ExecutionContext ctx) {
// move second case to "else"
if (cases[1] != null) {
assert generatedIf.getElsePart() != null;
JavaVisitor<Map<String, J.VariableDeclarations>> firstAssignmentAsDeclarationVisitor = new JavaVisitor<Map<String, J.VariableDeclarations>>() {

@Override
public J visitStatement(Statement statement, Map<String, J.VariableDeclarations> stringVariableDeclarationsMap) {
if (statement instanceof J.Assignment) {
return visitAssignment((J.Assignment) statement, stringVariableDeclarationsMap);
}
return super.visitStatement(statement, stringVariableDeclarationsMap);
}

@Override
public J visitAssignment(J.Assignment assignment, Map<String, J.VariableDeclarations> firstCaseDeclarations) {
if (assignment.getVariable() instanceof J.Identifier) {
String varName = ((J.Identifier) assignment.getVariable()).getSimpleName();
if (firstCaseDeclarations.containsKey(varName)) {
J.VariableDeclarations originalDecl = firstCaseDeclarations.remove(varName);
return originalDecl.withVariables(ListUtils.map(originalDecl.getVariables(), v -> v.withInitializer(assignment.getAssignment())));
}
}
return super.visitAssignment(assignment, firstCaseDeclarations);
}
};

if (isDefault(cases[1])) {
generatedIf = generatedIf.withElsePart(generatedIf.getElsePart().withBody(((J.Block) generatedIf.getElsePart().getBody()).withStatements(ListUtils.map(getStatements(cases[1]),
s -> s instanceof J.Break ? null : s))));
s -> s instanceof J.Break || s == null ? null : (Statement) firstAssignmentAsDeclarationVisitor.visitStatement(s, firstCaseDeclarations)))));
} else {
J.If elseIf = (J.If) generatedIf.getElsePart().getBody();
generatedIf = generatedIf.withElsePart(generatedIf.getElsePart().withBody(elseIf.withThenPart(((J.Block) elseIf.getThenPart()).withStatements(ListUtils.map(getStatements(cases[1]),
s -> s instanceof J.Break ? null : s)))));
s -> s instanceof J.Break || s == null ? null : (Statement) firstAssignmentAsDeclarationVisitor.visitStatement(s, firstCaseDeclarations))))));
}
}

Expand Down Expand Up @@ -250,7 +290,6 @@ private boolean switchesOnEnum(J.Switch switch_) {
return selectorType instanceof JavaType.Class &&
((JavaType.Class) selectorType).getKind() == JavaType.Class.Kind.Enum;
}

});
}

Expand Down
104 changes: 104 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/MinimumSwitchCasesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,4 +887,108 @@ void doSomethingElse() {}
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/687")
@Test
void variableRedeclarationInNewBlocks() {
rewriteRun(
//language=java
java(
"""
class Test {
int someInt;
void test() {
switch (someInt) {
case 1:
String data = getSomeString();
doThingOneWith(data);
break;
case 2:
data = getSomeOtherString();
doThingTwoWith(data);
break;
}
}
String getSomeString() { return "one"; }
String getSomeOtherString() { return "two"; }
void doThingOneWith(String data) {}
void doThingTwoWith(String data) {}
}
""",
"""
class Test {
int someInt;
void test() {
if (someInt == 1) {
String data = getSomeString();
doThingOneWith(data);
} else if (someInt == 2) {
String data = getSomeOtherString();
doThingTwoWith(data);
}
}
String getSomeString() { return "one"; }
String getSomeOtherString() { return "two"; }
void doThingOneWith(String data) {}
void doThingTwoWith(String data) {}
}
"""
)
);
}

@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/687")
@Test
void MultipleVariableRedeclarationInNewBlocks() {
rewriteRun(
//language=java
java(
"""
class Test {
int someInt;
void test() {
switch (someInt) {
case 1:
String data = getSomeString(), otherString = getSomeString();
doThingOneWith(data);
doThingOneWith(otherString);
break;
case 2:
data = getSomeOtherString();
otherString = getSomeOtherString();
doThingTwoWith(data);
doThingTwoWith(otherString);
break;
}
}
String getSomeString() { return "one"; }
String getSomeOtherString() { return "two"; }
void doThingOneWith(String data) {}
void doThingTwoWith(String data) {}
}
""",
"""
class Test {
int someInt;
void test() {
if (someInt == 1) {
String data = getSomeString(), otherString = getSomeString();
doThingOneWith(data);
doThingOneWith(otherString);
} else if (someInt == 2) {
String data = getSomeOtherString();
String otherString = getSomeOtherString();
doThingTwoWith(data);
doThingTwoWith(otherString);
}
}
String getSomeString() { return "one"; }
String getSomeOtherString() { return "two"; }
void doThingOneWith(String data) {}
void doThingTwoWith(String data) {}
}
"""
)
);
}
}