From 19e2484d2a53d6009e61dcbbba8a9f112fbd5098 Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 13:25:45 +0200 Subject: [PATCH 1/3] UnwrapElseAfterReturn: only unwrap when else declarations do not collide Statements hoisted out of an else block move into the enclosing block, where the names they declare stay in scope to the end of that block. The recipe hoisted them unconditionally, so an else block declaring a name that a later statement declares again produced two declarations of the same name in one block, which Java forbids for method locals and local classes, and the output no longer compiled. A hoisted name could also capture a later unqualified use that had resolved to a field or a statically imported member, silently changing semantics. Both flattening branches, the plain else and the innermost else of an else-if chain, now inspect the statements that follow the if and leave the else in place when a hoisted name would collide with or capture one of them. The names considered are declared variables and local types plus every instanceof pattern variable anywhere inside a hoisted statement, because flow scoping (JLS 6.3.2) can carry a pattern variable past its own statement once that statement sits directly in the enclosing block. Not every pattern variable escapes that way, so the check errs toward keeping the else block and gives up a few rewrites that would have been safe. Names declared in a nested scope inside the else, a for loop variable for example, are not hoisted and still permit unwrapping. No existing test expectation changed. --- .../staticanalysis/UnwrapElseAfterReturn.java | 108 +++- .../UnwrapElseAfterReturnTest.java | 519 +++++++++++++++++- 2 files changed, 622 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java index 322819ab7..eebb56b65 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java @@ -22,6 +22,7 @@ import org.openrewrite.Repeat; import org.openrewrite.TreeVisitor; import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.JavaVisitor; import org.openrewrite.java.tree.Comment; import org.openrewrite.java.tree.J; @@ -30,9 +31,14 @@ import java.time.Duration; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import static java.util.Collections.singletonList; + public class UnwrapElseAfterReturn extends Recipe { @Getter @@ -52,10 +58,11 @@ public TreeVisitor getVisitor() { public J.Block visitBlock(J.Block block, ExecutionContext ctx) { J.Block b = visitAndCast(block, ctx, super::visitBlock); AtomicReference<@Nullable Space> endWhitespace = new AtomicReference<>(null); - J.Block alteredBlock = b.withStatements(ListUtils.flatMap(b.getStatements(), statement -> { + J.Block alteredBlock = b.withStatements(ListUtils.flatMap(b.getStatements(), (index, statement) -> { if (statement instanceof J.If) { J.If ifStatement = (J.If) statement; if (ifStatement.getElsePart() != null && endsWithReturnOrThrow(ifStatement.getThenPart())) { + List laterStatements = b.getStatements().subList(index + 1, b.getStatements().size()); Statement elsePart = ifStatement.getElsePart().getBody(); if (elsePart instanceof J.If) { // Else-if chain: find and unwrap the innermost else @@ -65,11 +72,13 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) { endsWithReturnOrThrow(innermost.getThenPart()) && !(innermost.getElsePart().getBody() instanceof J.If)) { // Unwrap the innermost else - J.If modifiedChain = removeInnermostElse(ifStatement); Statement innermostElseBody = innermost.getElsePart().getBody(); - return flatten(innermost, innermostElseBody, endWhitespace, modifiedChain); + if (!collidesWithLaterScope(innermostElseBody, laterStatements)) { + J.If modifiedChain = removeInnermostElse(ifStatement); + return flatten(innermost, innermostElseBody, endWhitespace, modifiedChain); + } } - } else { + } else if (!collidesWithLaterScope(elsePart, laterStatements)) { // Plain else block: unwrap directly J.If newIf = ifStatement.withElsePart(null); return flatten(ifStatement, elsePart, endWhitespace, newIf); @@ -105,6 +114,97 @@ private List flatten(J.If tailIf, Statement tailElse, AtomicReference return Arrays.asList(ifWithoutElse, tailElse.withPrefix(tailIf.getElsePart().getPrefix())); } + /** + * Statements hoisted out of the else block move into the enclosing block, where the names they + * declare stay in scope until the end of that block. Unwrapping is therefore skipped when that + * larger scope could change how a name in the statements after the {@code if} resolves: + *
    + *
  • A hoisted name that is declared again in a later statement, at any nesting depth, would + * usually no longer compile, since Java does not allow local variables or local classes of a + * method to shadow each other; that covers later locals, loop variables, catch parameters, + * resources, lambda parameters, pattern variables and local types.
    • + *
  • A later unqualified use of a hoisted name currently resolves to something else, such as + * a field or a statically imported member, and would be captured by the hoisted declaration, + * silently changing semantics or breaking compilation. Uses whose resolution cannot be + * affected by a local variable or local class coming into scope, such as method invocation + * names, qualified field accesses and labels, are exempt.
    • + *
+ * The names a hoisted statement introduces are its declared variables and local types, plus + * every {@code instanceof} pattern variable anywhere inside it: flow scoping (JLS 6.3.2) can + * extend a pattern variable past its statement once that statement sits directly in the + * enclosing block, e.g. {@code if (!(o instanceof String s)) return;} leaves {@code s} in + * scope for the rest of the block. Not every pattern variable escapes its statement, so this + * errs on the side of keeping the else block. + */ + private boolean collidesWithLaterScope(Statement elseBody, List laterStatements) { + if (laterStatements.isEmpty()) { + return false; + } + Set hoistedNames = new HashSet<>(); + JavaIsoVisitor> patternVariableCollector = new JavaIsoVisitor>() { + @Override + public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Set names) { + if (instanceOf.getPattern() instanceof J.Identifier) { + names.add(((J.Identifier) instanceOf.getPattern()).getSimpleName()); + } + return super.visitInstanceOf(instanceOf, names); + } + + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, Set names) { + // The bindings of a record deconstruction pattern are variable declarations nested inside the pattern + if (getCursor().firstEnclosing(J.DeconstructionPattern.class) != null) { + names.add(variable.getSimpleName()); + } + return super.visitVariable(variable, names); + } + }; + List hoistedStatements = elseBody instanceof J.Block ? ((J.Block) elseBody).getStatements() : singletonList(elseBody); + for (Statement hoisted : hoistedStatements) { + if (hoisted instanceof J.VariableDeclarations) { + for (J.VariableDeclarations.NamedVariable variable : ((J.VariableDeclarations) hoisted).getVariables()) { + hoistedNames.add(variable.getSimpleName()); + } + } else if (hoisted instanceof J.ClassDeclaration) { + hoistedNames.add(((J.ClassDeclaration) hoisted).getSimpleName()); + } + patternVariableCollector.visit(hoisted, hoistedNames); + } + if (hoistedNames.isEmpty()) { + return false; + } + + AtomicBoolean collides = new AtomicBoolean(false); + JavaIsoVisitor nameScanner = new JavaIsoVisitor() { + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean found) { + if (hoistedNames.contains(identifier.getSimpleName())) { + // Both declarations and unqualified uses appear as identifiers; only identifiers + // that resolve in another namespace or through a qualifier are unaffected + Object parent = getCursor().getParentTreeCursor().getValue(); + boolean unaffected = parent instanceof J.MethodInvocation && identifier == ((J.MethodInvocation) parent).getName() || + parent instanceof J.FieldAccess && identifier == ((J.FieldAccess) parent).getName() || + parent instanceof J.MemberReference && identifier == ((J.MemberReference) parent).getReference() || + parent instanceof J.MethodDeclaration && identifier == ((J.MethodDeclaration) parent).getName() || + parent instanceof J.Label || + parent instanceof J.Break || + parent instanceof J.Continue; + if (!unaffected) { + found.set(true); + } + } + return super.visitIdentifier(identifier, found); + } + }; + for (Statement laterStatement : laterStatements) { + nameScanner.visit(laterStatement, collides); + if (collides.get()) { + return true; + } + } + return false; + } + private J.@Nullable If findInnermostIfWithElse(J.If ifStatement) { if (ifStatement.getElsePart() == null) { return null; diff --git a/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java b/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java index 909d75139..8c51cb678 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java @@ -16,13 +16,16 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledForJreRange; +import org.junit.jupiter.api.condition.JRE; import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; import static org.openrewrite.java.Assertions.java; +import static org.openrewrite.java.Assertions.version; -@SuppressWarnings("ConstantConditions") +@SuppressWarnings({"ConstantConditions", "unused"}) class UnwrapElseAfterReturnTest implements RewriteTest { @Override @@ -650,6 +653,520 @@ int foo(boolean condition) { ); } + @Test + void doNotUnwrapWhenElseDeclarationCollidesWithLaterLocalVariable() { + rewriteRun( + //language=java + java( + """ + class Test { + void plain(boolean stop) { + if (stop) { + return; + } else { + int value = 1; + System.out.println(value); + } + int value = 2; + System.out.println(value); + } + + void chain(boolean first, boolean second) { + if (first) { + return; + } else if (second) { + return; + } else { + int value = 1; + System.out.println(value); + } + int value = 2; + System.out.println(value); + } + + void afterThrow(boolean stop) { + if (stop) { + throw new IllegalStateException(); + } else { + String value = "1"; + System.out.println(value); + } + long value = 2; + System.out.println(value); + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapWhenElseDeclarationCollidesWithNestedScope() { + rewriteRun( + //language=java + java( + """ + import java.io.IOException; + import java.io.StringReader; + import java.util.List; + + class Test { + void catchVariable(boolean stop) { + if (stop) { + return; + } else { + int value = 1; + System.out.println(value); + } + try { + System.out.println("try"); + } catch (RuntimeException value) { + System.out.println(value); + } + } + + void resourceVariable(boolean stop) throws IOException { + if (stop) { + return; + } else { + int reader = 1; + System.out.println(reader); + } + try (StringReader reader = new StringReader("")) { + System.out.println(reader.read()); + } + } + + void lambdaParameter(boolean stop, List values) { + if (stop) { + return; + } else { + int element = 1; + System.out.println(element); + } + values.forEach(element -> System.out.println(element)); + } + + void loopVariable(boolean stop) { + if (stop) { + return; + } else { + int index = 1; + System.out.println(index); + } + for (int index = 0; index < 2; index++) { + System.out.println(index); + } + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapWhenElseLocalClassCollidesWithLaterLocalClass() { + rewriteRun( + //language=java + java( + """ + class Test { + void foo(boolean stop) { + if (stop) { + return; + } else { + class Helper { + } + System.out.println(new Helper()); + } + class Helper { + } + System.out.println(new Helper()); + } + } + """ + ) + ); + } + + @Test + void doNotUnwrapWhenElseDeclarationCollidesWithLaterPatternVariable() { + rewriteRun( + version( + //language=java + java( + """ + class Test { + void foo(boolean stop, Object o) { + if (stop) { + return; + } else { + String text = "1"; + System.out.println(text); + } + if (o instanceof String text) { + System.out.println(text); + } + } + } + """ + ), 17 + ) + ); + } + + @Test + void doNotUnwrapWhenEscapedPatternVariableCollidesWithLaterDeclaration() { + rewriteRun( + version( + //language=java + java( + """ + class Test { + void plain(Object o, boolean stop) { + if (stop) { + return; + } else { + if (!(o instanceof String s)) { + return; + } + System.out.println(s); + } + String s = "later"; + System.out.println(s); + } + + void chain(Object o, boolean first, boolean second) { + if (first) { + return; + } else if (second) { + return; + } else { + if (!(o instanceof String s)) { + return; + } + System.out.println(s); + } + String s = "later"; + System.out.println(s); + } + + void singleStatementElse(Object o, boolean stop) { + if (stop) { + return; + } else + while (!(o instanceof String s)) + o = o.toString(); + String s = "later"; + System.out.println(s); + } + } + """ + ), 17 + ) + ); + } + + @Test + void doNotUnwrapWhenElseDeclarationShadowsNameUsedLater() { + rewriteRun( + //language=java + java( + """ + class Test { + int value; + + void plain(boolean stop) { + if (stop) { + return; + } else { + String value = "1"; + System.out.println(value); + } + int doubled = value * 2; + System.out.println(doubled); + } + + void chain(boolean first, boolean second) { + if (first) { + return; + } else if (second) { + return; + } else { + String value = "1"; + System.out.println(value); + } + int doubled = value * 2; + System.out.println(doubled); + } + + int sameType(boolean stop) { + if (stop) { + return -1; + } else { + int value = 1; + System.out.println(value); + } + return value; + } + } + """ + ) + ); + } + + @Test + void unwrapWhenLaterUsesAreNotCapturedByHoistedNames() { + rewriteRun( + //language=java + java( + """ + class Test { + int value; + + int value() { + return 42; + } + + void qualifiedFieldUse(boolean stop) { + if (stop) { + return; + } else { + String value = "1"; + System.out.println(value); + } + System.out.println(this.value); + } + + void methodNameUse(boolean stop) { + if (stop) { + return; + } else { + String value = "1"; + System.out.println(value); + } + System.out.println(value()); + } + } + """, + """ + class Test { + int value; + + int value() { + return 42; + } + + void qualifiedFieldUse(boolean stop) { + if (stop) { + return; + } + String value = "1"; + System.out.println(value); + System.out.println(this.value); + } + + void methodNameUse(boolean stop) { + if (stop) { + return; + } + String value = "1"; + System.out.println(value); + System.out.println(value()); + } + } + """ + ) + ); + } + + @Test + void unwrapWhenEscapedPatternVariableDoesNotCollide() { + rewriteRun( + version( + //language=java + java( + """ + class Test { + void foo(Object o, boolean stop) { + if (stop) { + return; + } else { + if (!(o instanceof String s)) { + return; + } + System.out.println(s); + } + System.out.println("done"); + } + } + """, + """ + class Test { + void foo(Object o, boolean stop) { + if (stop) { + return; + } + if (!(o instanceof String s)) { + return; + } + System.out.println(s); + System.out.println("done"); + } + } + """ + ), 17 + ) + ); + } + + @EnabledForJreRange(min = JRE.JAVA_21) + @Test + void unwrapOnlyWhenDeconstructionPatternBindingsDoNotCollide() { + rewriteRun( + version( + //language=java + java( + """ + class Test { + record Point(int x, int y) {} + + void collides(Object o, boolean stop) { + if (stop) { + return; + } else { + if (!(o instanceof Point(int x, int y))) { + return; + } + System.out.println(x + y); + } + int x = 5; + System.out.println(x); + } + + void doesNotCollide(Object o, boolean stop) { + if (stop) { + return; + } else { + if (!(o instanceof Point(int x, int y))) { + return; + } + System.out.println(x + y); + } + Point p = new Point(1, 2); + System.out.println(p); + } + } + """, + """ + class Test { + record Point(int x, int y) {} + + void collides(Object o, boolean stop) { + if (stop) { + return; + } else { + if (!(o instanceof Point(int x, int y))) { + return; + } + System.out.println(x + y); + } + int x = 5; + System.out.println(x); + } + + void doesNotCollide(Object o, boolean stop) { + if (stop) { + return; + } + if (!(o instanceof Point(int x, int y))) { + return; + } + System.out.println(x + y); + Point p = new Point(1, 2); + System.out.println(p); + } + } + """ + ), 21 + ) + ); + } + + @Test + void unwrapWhenElseDeclarationsDoNotCollide() { + rewriteRun( + //language=java + java( + """ + class Test { + void foo(boolean stop) { + if (stop) { + return; + } else { + int value = 1; + System.out.println(value); + } + int other = 2; + System.out.println(other); + } + } + """, + """ + class Test { + void foo(boolean stop) { + if (stop) { + return; + } + int value = 1; + System.out.println(value); + int other = 2; + System.out.println(other); + } + } + """ + ) + ); + } + + @Test + void unwrapWhenCollidingDeclarationRemainsNestedInTheElseBlock() { + rewriteRun( + //language=java + java( + """ + class Test { + void foo(boolean stop) { + if (stop) { + return; + } else { + for (int value = 0; value < 2; value++) { + System.out.println(value); + } + } + int value = 2; + System.out.println(value); + } + } + """, + """ + class Test { + void foo(boolean stop) { + if (stop) { + return; + } + for (int value = 0; value < 2; value++) { + System.out.println(value); + } + int value = 2; + System.out.println(value); + } + } + """ + ) + ); + } + @Test void commentsOnlyInBlocksWithNewLine() { rewriteRun( From 96450e88756eec2dff206aff2a86d4427faf673c Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 11 Aug 2026 10:26:38 +0200 Subject: [PATCH 2/3] Review fixes: lock in the label-namespace exemption with a test --- .../staticanalysis/UnwrapElseAfterReturn.java | 36 ++++++++----------- .../UnwrapElseAfterReturnTest.java | 31 ++++++++++++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java index eebb56b65..8456b1e4c 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java @@ -115,26 +115,18 @@ private List flatten(J.If tailIf, Statement tailElse, AtomicReference } /** - * Statements hoisted out of the else block move into the enclosing block, where the names they - * declare stay in scope until the end of that block. Unwrapping is therefore skipped when that - * larger scope could change how a name in the statements after the {@code if} resolves: - *
    - *
  • A hoisted name that is declared again in a later statement, at any nesting depth, would - * usually no longer compile, since Java does not allow local variables or local classes of a - * method to shadow each other; that covers later locals, loop variables, catch parameters, - * resources, lambda parameters, pattern variables and local types.
    • - *
  • A later unqualified use of a hoisted name currently resolves to something else, such as - * a field or a statically imported member, and would be captured by the hoisted declaration, - * silently changing semantics or breaking compilation. Uses whose resolution cannot be - * affected by a local variable or local class coming into scope, such as method invocation - * names, qualified field accesses and labels, are exempt.
    • - *
- * The names a hoisted statement introduces are its declared variables and local types, plus - * every {@code instanceof} pattern variable anywhere inside it: flow scoping (JLS 6.3.2) can - * extend a pattern variable past its statement once that statement sits directly in the - * enclosing block, e.g. {@code if (!(o instanceof String s)) return;} leaves {@code s} in - * scope for the rest of the block. Not every pattern variable escapes its statement, so this - * errs on the side of keeping the else block. + * Statements hoisted out of the else block move into the enclosing block, where the names they declare + * stay in scope until the end of that block. Unwrapping is skipped when that larger scope could change + * how a name in the statements after the {@code if} resolves: a later redeclaration no longer compiles + * (JLS 6.4 forbids local variables and local classes of a method shadowing each other), and a later + * unqualified use that resolves to a field or a statically imported member would be captured by the + * hoisted declaration instead. Uses a local cannot capture, such as method invocation names, qualified + * field accesses and labels, are exempt. + *

+ * Every {@code instanceof} pattern variable inside a hoisted statement counts as a hoisted name, because + * flow scoping (JLS 6.3.2) can extend one past its statement once that statement sits directly in the + * enclosing block, as in {@code if (!(o instanceof String s)) return;}. Not every pattern variable + * escapes, so this errs on the side of keeping the else block. */ private boolean collidesWithLaterScope(Statement elseBody, List laterStatements) { if (laterStatements.isEmpty()) { @@ -179,8 +171,8 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations @Override public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean found) { if (hoistedNames.contains(identifier.getSimpleName())) { - // Both declarations and unqualified uses appear as identifiers; only identifiers - // that resolve in another namespace or through a qualifier are unaffected + // Declarations and unqualified uses both appear as identifiers; only those resolving in + // another namespace or through a qualifier are unaffected Object parent = getCursor().getParentTreeCursor().getValue(); boolean unaffected = parent instanceof J.MethodInvocation && identifier == ((J.MethodInvocation) parent).getName() || parent instanceof J.FieldAccess && identifier == ((J.FieldAccess) parent).getName() || diff --git a/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java b/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java index 8c51cb678..dc5c85aa4 100644 --- a/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturnTest.java @@ -947,6 +947,22 @@ void methodNameUse(boolean stop) { } System.out.println(value()); } + + void labelUse(boolean stop) { + if (stop) { + return; + } else { + String value = "1"; + System.out.println(value); + } + value: + for (int i = 0; i < 2; i++) { + if (i == 1) { + break value; + } + continue value; + } + } } """, """ @@ -974,6 +990,21 @@ void methodNameUse(boolean stop) { System.out.println(value); System.out.println(value()); } + + void labelUse(boolean stop) { + if (stop) { + return; + } + String value = "1"; + System.out.println(value); + value: + for (int i = 0; i < 2; i++) { + if (i == 1) { + break value; + } + continue value; + } + } } """ ) From d3303f3cafba01352e1a1f185a608f2b4540f68e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:22:02 +0200 Subject: [PATCH 3/3] Trim commentary --- .../staticanalysis/UnwrapElseAfterReturn.java | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java index 8456b1e4c..b1f807ff7 100644 --- a/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java +++ b/src/main/java/org/openrewrite/staticanalysis/UnwrapElseAfterReturn.java @@ -115,18 +115,12 @@ private List flatten(J.If tailIf, Statement tailElse, AtomicReference } /** - * Statements hoisted out of the else block move into the enclosing block, where the names they declare - * stay in scope until the end of that block. Unwrapping is skipped when that larger scope could change - * how a name in the statements after the {@code if} resolves: a later redeclaration no longer compiles - * (JLS 6.4 forbids local variables and local classes of a method shadowing each other), and a later - * unqualified use that resolves to a field or a statically imported member would be captured by the - * hoisted declaration instead. Uses a local cannot capture, such as method invocation names, qualified - * field accesses and labels, are exempt. - *

- * Every {@code instanceof} pattern variable inside a hoisted statement counts as a hoisted name, because - * flow scoping (JLS 6.3.2) can extend one past its statement once that statement sits directly in the - * enclosing block, as in {@code if (!(o instanceof String s)) return;}. Not every pattern variable - * escapes, so this errs on the side of keeping the else block. + * Hoisting the else block widens the scope of the names it declares to the end of the enclosing block, + * so unwrapping is skipped where that could change how a later name resolves: a later redeclaration no + * longer compiles (JLS 6.4), and a later unqualified use of a field or statically imported member would + * be captured instead. Names a local cannot capture, such as invocation names and labels, are exempt. + * Every {@code instanceof} pattern variable counts as hoisted, since flow scoping (JLS 6.3.2) can carry + * one past its own statement; not all of them do, so this errs towards keeping the else block. */ private boolean collidesWithLaterScope(Statement elseBody, List laterStatements) { if (laterStatements.isEmpty()) { @@ -144,7 +138,7 @@ public J.InstanceOf visitInstanceOf(J.InstanceOf instanceOf, Set names) @Override public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, Set names) { - // The bindings of a record deconstruction pattern are variable declarations nested inside the pattern + // A record deconstruction pattern nests its bindings as variable declarations if (getCursor().firstEnclosing(J.DeconstructionPattern.class) != null) { names.add(variable.getSimpleName()); } @@ -171,8 +165,8 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations @Override public J.Identifier visitIdentifier(J.Identifier identifier, AtomicBoolean found) { if (hoistedNames.contains(identifier.getSimpleName())) { - // Declarations and unqualified uses both appear as identifiers; only those resolving in - // another namespace or through a qualifier are unaffected + // Declarations and unqualified uses are both identifiers; only another namespace or a + // qualifier makes one safe Object parent = getCursor().getParentTreeCursor().getValue(); boolean unaffected = parent instanceof J.MethodInvocation && identifier == ((J.MethodInvocation) parent).getName() || parent instanceof J.FieldAccess && identifier == ((J.FieldAccess) parent).getName() ||