From bced1b80fdceedf5a7c3bc8436513135ed4b5c9e Mon Sep 17 00:00:00 2001 From: Tom Tourwe Date: Mon, 26 Oct 2026 14:49:22 +0100 Subject: [PATCH 1/2] A case label with several patterns keeps all of them Java allows several patterns in one label when none binds a variable (JEP 456): case VarcharType _, CharType _ ->. SwitchEntry held one pattern, and the two front ends lost the rest: maddi-java-openjdk kept the first and dropped the others, maddi-java-parser set each in turn and kept the last. So the type of every other pattern was missing from the model. Found on trino 817f7034667, PathEvaluationVisitor: typesReferenced did not report CharType, and a dead-code removal took the file's import of CharType from under two live uses. SwitchEntry.otherPatterns() holds the patterns after the first (a default method returning an empty list, so no implementor or caller has to change), and both front ends fill it. SwitchEntryImpl takes them into typesReferenced, the Java printer (case A _ alone compiles and no longer matches B), translate, rewire, its own visit and its variable streams; the walks of SwitchExpressionImpl and SwitchStatementNewStyleImpl visit them too. The old-style switch already kept every pattern, one SwitchLabel each. TestSwitchExpression.test3 (types referenced and printing) and test4, TestSwitchNewStyle.testWalkReachesEveryPattern (the walk): each failed before the change. Signed-off-by: Tom Tourwe --- .../maddi/cst/api/statement/SwitchEntry.java | 13 +++++ .../impl/expression/SwitchExpressionImpl.java | 2 + .../cst/impl/statement/SwitchEntryImpl.java | 57 ++++++++++++++++--- .../SwitchStatementNewStyleImpl.java | 2 + .../java/openjdk/ScanCompilationUnit.java | 20 ++++--- .../expression/TestSwitchExpression.java | 45 +++++++++++++++ .../openjdk/statement/TestSwitchNewStyle.java | 34 +++++++++++ .../maddi/parser/java/CommonParse.java | 11 +++- 8 files changed, 167 insertions(+), 17 deletions(-) diff --git a/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/statement/SwitchEntry.java b/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/statement/SwitchEntry.java index be44fcf1a..4539a9505 100644 --- a/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/statement/SwitchEntry.java +++ b/maddi-cst-api/src/main/java/io/codelaser/maddi/cst/api/statement/SwitchEntry.java @@ -43,6 +43,15 @@ public interface SwitchEntry extends Comparable, Element { */ RecordPattern patternVariable(); + /** + * @return the patterns after the first one of a label that carries several (JEP 456: {@code case A _, B _ ->}), + * in source order; empty otherwise. {@link #patternVariable()} is the first. Java allows several patterns in one + * label only when none of them binds a variable, so these never introduce one. + */ + default List otherPatterns() { + return List.of(); + } + /** * @return the arm's code as a block (wrapping {@link #statement()} when it is a single statement). */ @@ -76,6 +85,10 @@ interface Builder extends Element.Builder { @Fluent Builder setPatternVariable(RecordPattern patternVariable); + /** A pattern after the first one of the label, see {@link SwitchEntry#otherPatterns()}. */ + @Fluent + Builder addOtherPattern(RecordPattern pattern); + @Fluent Builder setWhenExpression(Expression whenExpression); diff --git a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/expression/SwitchExpressionImpl.java b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/expression/SwitchExpressionImpl.java index cd244196b..5da9b4e71 100644 --- a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/expression/SwitchExpressionImpl.java +++ b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/expression/SwitchExpressionImpl.java @@ -159,6 +159,7 @@ public void visit(Predicate predicate) { for (SwitchEntry entry : entries) { entry.conditions().forEach(e -> e.visit(predicate)); if (entry.patternVariable() != null) entry.patternVariable().visit(predicate); + entry.otherPatterns().forEach(o -> o.visit(predicate)); // case A _, B _ -> (JEP 456) entry.whenExpression().visit(predicate); entry.statement().visit(predicate); i++; @@ -173,6 +174,7 @@ public void visit(Visitor visitor) { for (SwitchEntry entry : entries) { entry.conditions().forEach(e -> e.visit(visitor)); if (entry.patternVariable() != null) entry.patternVariable().visit(visitor); + entry.otherPatterns().forEach(o -> o.visit(visitor)); // case A _, B _ -> (JEP 456) entry.whenExpression().visit(visitor); visitor.startSubBlock(i); entry.statement().visit(visitor); diff --git a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchEntryImpl.java b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchEntryImpl.java index 57b487802..ae8899f53 100644 --- a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchEntryImpl.java +++ b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchEntryImpl.java @@ -41,6 +41,7 @@ public class SwitchEntryImpl implements SwitchEntry { private final List conditions; private final RecordPattern patternVariable; + private final List otherPatterns; private final Expression whenExpression; private final Statement statement; private final Source source; @@ -52,8 +53,19 @@ public SwitchEntryImpl(List comments, RecordPattern patternVariable, Expression whenExpression, Statement statement) { + this(comments, source, conditions, patternVariable, List.of(), whenExpression, statement); + } + + public SwitchEntryImpl(List comments, + Source source, + List conditions, + RecordPattern patternVariable, + List otherPatterns, + Expression whenExpression, + Statement statement) { this.conditions = conditions; this.patternVariable = patternVariable; + this.otherPatterns = List.copyOf(otherPatterns); this.whenExpression = whenExpression; this.statement = statement; this.comments = comments == null ? List.of() : List.copyOf(comments); @@ -67,12 +79,13 @@ public int compareTo(SwitchEntry o) { @Override public SwitchEntry withStatement(Statement statement) { - return new SwitchEntryImpl(comments, source, conditions, patternVariable, whenExpression, statement); + return new SwitchEntryImpl(comments, source, conditions, patternVariable, otherPatterns, whenExpression, + statement); } @Override public int complexity() { - return (patternVariable != null ? 1 : 0) + return (patternVariable != null ? 1 : 0) + otherPatterns.size() + conditions.stream().mapToInt(Expression::complexity).sum() + whenExpression.complexity() + statement().complexity(); } @@ -92,6 +105,11 @@ public RecordPattern patternVariable() { return patternVariable; } + @Override + public List otherPatterns() { + return otherPatterns; + } + @Override public Block statementAsBlock() { if (statement instanceof Block b) return b; @@ -157,6 +175,11 @@ public OutputBuilder print(Qualification qualification) { outputBuilder.add(SymbolEnum.COMMA); } outputBuilder.add(patternVariable.print(qualification)); + // 'case A _, B _ ->' (JEP 456): printing the first pattern only turned it into 'case A _ ->', which + // compiles and no longer matches B + for (RecordPattern other : otherPatterns) { + outputBuilder.add(SymbolEnum.COMMA).add(other.print(qualification)); + } if (whenExpression != null && !whenExpression.isEmpty()) { outputBuilder.add(SpaceEnum.ONE).add(KeywordImpl.WHEN).add(SpaceEnum.ONE) .add(whenExpression.print(qualification)); @@ -173,15 +196,18 @@ public SwitchEntry translate(TranslationMap translationMap) { .collect(translationMap.toList(conditions)); RecordPattern tPattern = patternVariable == null ? null : patternVariable.translate(translationMap); + List tOthers = otherPatterns.stream().map(o -> o.translate(translationMap)) + .collect(translationMap.toList(otherPatterns)); Expression tWhen = whenExpression == null ? null : whenExpression.translate(translationMap); List tStatements = statement.translate(translationMap); Statement tStatement = tStatements.isEmpty() ? null : tStatements.getFirst(); - if (tConditions == conditions && tPattern == patternVariable && tWhen == whenExpression - && tStatement == statement) { + if (tConditions == conditions && tPattern == patternVariable && tOthers == otherPatterns + && tWhen == whenExpression && tStatement == statement) { return this; } if (tStatement == null) return null; // a way for the entry to disappear - SwitchEntry result = new SwitchEntryImpl(comments, source, tConditions, tPattern, tWhen, tStatement); + SwitchEntry result = new SwitchEntryImpl(comments, source, tConditions, tPattern, tOthers, tWhen, + tStatement); return translationMap.postTranslationHandler(this, result); } @@ -190,6 +216,7 @@ public SwitchEntry rewire(InfoMapView infoMap) { return new SwitchEntryImpl(comments, source, conditions.stream().map(e -> e.rewire(infoMap)).toList(), patternVariable == null ? null : (RecordPattern) patternVariable.rewire(infoMap), + otherPatterns.stream().map(o -> (RecordPattern) o.rewire(infoMap)).toList(), whenExpression.rewire(infoMap), statement.rewire(infoMap)); } @@ -202,6 +229,7 @@ public Source source() { public void visit(Predicate predicate) { if (predicate.test(this)) { if (patternVariable != null) patternVariable.visit(predicate); + otherPatterns.forEach(o -> o.visit(predicate)); whenExpression.visit(predicate); } } @@ -228,7 +256,9 @@ public Stream variableStreamDescend() { // the case labels read variables too (#66): a Java enum label is a static field read, and K2 lowers a // subject-less Kotlin `when` to a switch over `true` whose labels are arbitrary boolean expressions Stream s0 = conditions.stream().flatMap(Expression::variableStreamDescend); - Stream s1 = patternVariable == null ? Stream.of() : patternVariable.variableStreamDescend(); + Stream s1 = Stream.concat( + patternVariable == null ? Stream.of() : patternVariable.variableStreamDescend(), + otherPatterns.stream().flatMap(RecordPattern::variableStreamDescend)); return Stream.concat(s0, Stream.concat(s1, Stream.concat(whenExpression.variableStreamDescend(), statement.variableStreamDescend()))); } @@ -239,7 +269,11 @@ public Stream typesReferenced(Predicate predicat // the case labels ('case Kind.CONST ->') reference types too; without them, a type used only in a // label is invisible to import maintenance Stream s0 = conditions.stream().flatMap(c -> c.typesReferenced(predicate)); - Stream s1 = patternVariable == null ? Stream.of() : patternVariable.typesReferenced(predicate); + // every pattern of 'case A _, B _ ->', not only the first: B used to be invisible here, and trino's + // 'case VarcharType _, CharType _' lost its import of CharType to a dead-code removal + Stream s1 = Stream.concat( + patternVariable == null ? Stream.of() : patternVariable.typesReferenced(predicate), + otherPatterns.stream().flatMap(o -> o.typesReferenced(predicate))); return Stream.concat(s0, Stream.concat(s1, Stream.concat(whenExpression.typesReferenced(predicate), statement.typesReferenced(predicate)))); } @@ -247,6 +281,7 @@ public Stream typesReferenced(Predicate predicat public static class EntryBuilderImpl extends ElementImpl.Builder implements Builder { private final List conditions = new ArrayList<>(); private RecordPattern patternVariable; + private final List otherPatterns = new ArrayList<>(); private Expression whenExpression; private Statement statement; @@ -268,6 +303,12 @@ public Builder setPatternVariable(RecordPattern patternVariable) { return this; } + @Override + public Builder addOtherPattern(RecordPattern pattern) { + this.otherPatterns.add(pattern); + return this; + } + @Override public Builder setWhenExpression(Expression whenExpression) { this.whenExpression = whenExpression; @@ -277,7 +318,7 @@ public Builder setWhenExpression(Expression whenExpression) { @Override public SwitchEntry build() { return new SwitchEntryImpl(List.copyOf(comments), source, - List.copyOf(conditions), patternVariable, whenExpression, statement); + List.copyOf(conditions), patternVariable, otherPatterns, whenExpression, statement); } } } diff --git a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchStatementNewStyleImpl.java b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchStatementNewStyleImpl.java index 28e1a0eb6..0706c6718 100644 --- a/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchStatementNewStyleImpl.java +++ b/maddi-cst-impl/src/main/java/io/codelaser/maddi/cst/impl/statement/SwitchStatementNewStyleImpl.java @@ -89,6 +89,7 @@ public void visit(Predicate predicate) { for (SwitchEntry entry : entries) { entry.conditions().forEach(e -> e.visit(predicate)); if (entry.patternVariable() != null) entry.patternVariable().visit(predicate); + entry.otherPatterns().forEach(o -> o.visit(predicate)); // case A _, B _ -> (JEP 456) entry.whenExpression().visit(predicate); entry.statement().visit(predicate); } @@ -102,6 +103,7 @@ public void visit(Visitor visitor) { for (SwitchEntry entry : entries) { entry.conditions().forEach(e -> e.visit(visitor)); if (entry.patternVariable() != null) entry.patternVariable().visit(visitor); + entry.otherPatterns().forEach(o -> o.visit(visitor)); // case A _, B _ -> (JEP 456) entry.whenExpression().visit(visitor); visitor.startSubBlock(i); entry.statement().visit(visitor); diff --git a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java index f0787fc5e..260b988fa 100644 --- a/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java +++ b/maddi-java-openjdk/src/main/java/io/codelaser/maddi/java/openjdk/ScanCompilationUnit.java @@ -3659,6 +3659,7 @@ public Void visitSwitchExpression(SwitchExpressionTree node, Void unused) { for (JCTree.JCCase jcCase : cases) { List conditions = new ArrayList<>(); RecordPatternResult recordPatternResult = null; + List otherPatterns = new ArrayList<>(); for (JCTree.JCCaseLabel caseLabel : jcCase.getLabels()) { switch (caseLabel) { case JCTree.JCConstantCaseLabel ccl -> { @@ -3673,11 +3674,15 @@ public Void visitSwitchExpression(SwitchExpressionTree node, Void unused) { RecordPatternResult rpr = parseRecordPattern(pcl.getPattern(), dsb, annotations); if (recordPatternResult == null) { recordPatternResult = rpr; + } else { + // a case with several patterns, e.g. 'case BaseTypeSig _, TypeVarSig _ ->' (JEP 456). + // Java permits this only when none of them declares a (named) pattern variable (JLS + // 14.11.1), so the arm body cannot reference a binding -- but the types are written + // there. ⛔ They used to be dropped: 'case VarcharType _, CharType _' (trino 817f7034667, + // PathEvaluationVisitor) referenced VarcharType only, and a dead-code removal took the + // import of CharType from under two live uses. SwitchEntry.otherPatterns keeps them. + otherPatterns.add(rpr.rp); } - // else: a case with several patterns, e.g. 'case BaseTypeSig _, TypeVarSig _ ->' (JEP 456). - // Java permits this only when none of them declares a (named) pattern variable (JLS - // 14.11.1), so the arm body cannot reference a binding. The switch-entry model carries a - // single pattern, so keep the first and ignore the binding-free alternatives. } case null, default -> throw new UnsupportedOperationException(unexpected("case label", caseLabel)); @@ -3747,14 +3752,15 @@ That leak is the same bug seen from the other side (a `case` binding outliving i } Source source = sourceForNode(jcCase); - SwitchEntry switchEntry = runtime.newSwitchEntryBuilder() + SwitchEntry.Builder entryBuilder = runtime.newSwitchEntryBuilder() .setSource(sourceForNode(jcCase)) .addComments(commentsForNode(source)) .addConditions(conditions) .setStatement(statement) .setWhenExpression(whenExpression) - .setPatternVariable(recordPatternResult == null ? null : recordPatternResult.rp) - .build(); + .setPatternVariable(recordPatternResult == null ? null : recordPatternResult.rp); + otherPatterns.forEach(entryBuilder::addOtherPattern); + SwitchEntry switchEntry = entryBuilder.build(); switchEntries.add(switchEntry); ++i; } diff --git a/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/expression/TestSwitchExpression.java b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/expression/TestSwitchExpression.java index 454cc03af..e9919a96b 100644 --- a/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/expression/TestSwitchExpression.java +++ b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/expression/TestSwitchExpression.java @@ -1,6 +1,9 @@ package io.codelaser.maddi.java.openjdk.expression; import io.codelaser.maddi.cst.api.expression.SwitchExpression; +import io.codelaser.maddi.cst.api.element.RecordPattern; +import io.codelaser.maddi.cst.api.statement.SwitchEntry; +import io.codelaser.maddi.cst.impl.output.QualificationImpl; import io.codelaser.maddi.cst.api.info.MethodInfo; import io.codelaser.maddi.cst.api.info.TypeInfo; import io.codelaser.maddi.cst.api.statement.Statement; @@ -10,8 +13,11 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import java.util.List; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertTrue; public class TestSwitchExpression extends CommonTest { @Language("java") @@ -76,4 +82,43 @@ public void test2() { SwitchExpression switchExpression = (SwitchExpression) returnStatement.expression(); assertEquals(2, switchExpression.entries().size()); } + + // The patterns after the first are part of the entry: their types are referenced (import maintenance counts on it) + // and they are printed. trino 817f7034667, PathEvaluationVisitor: 'case VarcharType _, CharType _ ->' reported + // VarcharType only, and a dead-code removal dropped 'import io.trino.spi.type.CharType;' from under two live uses. + @DisplayName("every pattern of a multi-pattern case label is referenced and printed") + @Test + public void test3() { + TypeInfo X = scan("X", INPUT2); + MethodInfo method = X.findUniqueMethod("method", 1); + Statement returnStatement = method.methodBody().statements().getFirst(); + SwitchExpression switchExpression = (SwitchExpression) returnStatement.expression(); + SwitchEntry entry = switchExpression.entries().getFirst(); + List referenced = entry.typesReferenced(null) + .map(r -> r.typeInfo().simpleName()).distinct().sorted().toList(); + assertEquals(List.of("A", "B"), referenced, + "types the entry 'case A _, B _ -> 1' reports as referenced (left: both patterns; right: reported)"); + // whitespace collapsed: OutputBuilder.toString() separates the tokens of a pattern with two spaces + String printed = entry.print(QualificationImpl.SIMPLE_ONLY).toString().replaceAll("\\s+", " "); + assertTrue(printed.contains("A _") && printed.contains("B _"), + "the entry 'case A _, B _ -> 1' prints both patterns; it printed: " + printed); + } + + // A walk over the whole switch reaches every pattern of every case, not only the first of each. + @DisplayName("walking a switch expression reaches every pattern of a multi-pattern case label") + @Test + public void test4() { + TypeInfo X = scan("X", INPUT2); + MethodInfo method = X.findUniqueMethod("method", 1); + SwitchExpression switchExpression = (SwitchExpression) method.methodBody().statements().getFirst().expression(); + List visited = new java.util.ArrayList<>(); + switchExpression.visit(e -> { + if (e instanceof RecordPattern rp && rp.localVariable() != null) { + visited.add(rp.localVariable().parameterizedType().typeInfo().simpleName()); + } + return true; + }); + assertEquals(List.of("A", "B", "C"), visited, + "patterns reached walking 'case A _, B _ -> 1; case C _ -> 2' (left: all three; right: reached)"); + } } diff --git a/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/statement/TestSwitchNewStyle.java b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/statement/TestSwitchNewStyle.java index 55af2c02f..2966a5534 100644 --- a/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/statement/TestSwitchNewStyle.java +++ b/maddi-java-openjdk/src/test/java/io/codelaser/maddi/java/openjdk/statement/TestSwitchNewStyle.java @@ -366,4 +366,38 @@ public void staticConstantOfAnotherTypeStaysQualified() { .toString(); assertTrue(expression.contains("case Consts.ONE->"), expression); } + + @Language("java") + private static final String INPUT_PATTERNS = """ + class X { + sealed interface S permits A, B, C {} + record A() implements S {} + record B() implements S {} + record C() implements S {} + static void method(S s) { + switch (s) { + case A _, B _ -> System.out.println("ab"); + case C _ -> System.out.println("c"); + } + } + } + """; + + // A walk over the whole switch reaches every pattern of every case, not only the first of each (JEP 456). + @DisplayName("walking a switch statement reaches every pattern of a multi-pattern case label") + @Test + public void testWalkReachesEveryPattern() { + TypeInfo X = scan("X", INPUT_PATTERNS); + MethodInfo method = X.findUniqueMethod("method", 1); + SwitchStatementNewStyle switchStatement = (SwitchStatementNewStyle) method.methodBody().statements().getFirst(); + java.util.List visited = new java.util.ArrayList<>(); + switchStatement.visit(e -> { + if (e instanceof io.codelaser.maddi.cst.api.element.RecordPattern rp && rp.localVariable() != null) { + visited.add(rp.localVariable().parameterizedType().typeInfo().simpleName()); + } + return true; + }); + assertEquals(java.util.List.of("A", "B", "C"), visited, + "patterns reached walking 'case A _, B _ -> ...; case C _ -> ...' (left: all three; right: reached)"); + } } diff --git a/maddi-java-parser/src/main/java/io/codelaser/maddi/parser/java/CommonParse.java b/maddi-java-parser/src/main/java/io/codelaser/maddi/parser/java/CommonParse.java index d780cd685..8a0ceab22 100644 --- a/maddi-java-parser/src/main/java/io/codelaser/maddi/parser/java/CommonParse.java +++ b/maddi-java-parser/src/main/java/io/codelaser/maddi/parser/java/CommonParse.java @@ -386,6 +386,9 @@ protected void parseNewSwitchLabel(String index, Context newContext = Objects.requireNonNullElse(enumContext, context); io.codelaser.maddi.cst.api.expression.Expression whenExpression = runtime.newEmptyExpression(); List conditions = new ArrayList<>(); + // 'case A _, B _ ->' (JEP 456): the first pattern is the entry's pattern variable, the others go to + // SwitchEntry.otherPatterns. Setting each one as THE pattern variable kept only the last. + boolean havePattern = false; int j = 0; while (j < nsl.size()) { Node node = nsl.get(j); @@ -393,11 +396,15 @@ protected void parseNewSwitchLabel(String index, case TypePattern lvd -> { RecordPattern recordPattern = parsers.parseRecordPattern() .parseLocalVariableDeclaration(newContext, lvd, 0, null); - entryBuilder.setPatternVariable(recordPattern); + if (havePattern) entryBuilder.addOtherPattern(recordPattern); + else entryBuilder.setPatternVariable(recordPattern); + havePattern = true; } case org.parsers.java.ast.RecordPattern rp -> { RecordPattern recordPattern = parsers.parseRecordPattern().parseRecordPattern(newContext, rp); - entryBuilder.setPatternVariable(recordPattern); + if (havePattern) entryBuilder.addOtherPattern(recordPattern); + else entryBuilder.setPatternVariable(recordPattern); + havePattern = true; } case WhenClause whenClause -> { ForwardType booleanTypeFwd = newContext.newForwardType(runtime.booleanParameterizedType()); From ccdf580bc5155b17632d0a8402df5196d962fdbb Mon Sep 17 00:00:00 2001 From: Tom Tourwe Date: Mon, 26 Oct 2026 14:49:22 +0100 Subject: [PATCH 2/2] Kotlin and C# translations print every pattern of a multi-pattern case label case String _, Integer _ -> was translated from its first pattern only: Kotlin's is String -> and C#'s case string : (and string => in a switch expression). Both compile and no longer match an Integer. Kotlin now prints is String, is Int ->; an entry counts as a type-pattern arm only when every one of its patterns is a type pattern, since is T tests the type and a deconstruction does not reduce to that. C# prints the or pattern, case string or int: and string or int =>, each alternative as its type alone, because C# allows no designation under or and Java allows none in a multi-pattern label. TestJavaToKotlinStructure.multiPatternCase (the Kotlin compiles) and TestJavaToCSharpStructure.multiPatternCase, whose golden file is the correct translation, written by hand: the first run recorded the defective one. Signed-off-by: Tom Tourwe --- .../print/csharp/CSharpStatementPrinter.java | 36 +++++++++++++++++-- .../csharp/TestJavaToCSharpStructure.java | 29 +++++++++++++++ .../csharp/golden/multiPatternCase-Kinds.cs | 11 ++++++ .../print/kotlin/KotlinStatementPrinter.java | 25 ++++++++++--- .../kotlin/TestJavaToKotlinStructure.java | 21 +++++++++++ 5 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 maddi-cst-print-csharp/src/test/resources/io/codelaser/maddi/cst/print/csharp/golden/multiPatternCase-Kinds.cs diff --git a/maddi-cst-print-csharp/src/main/java/io/codelaser/maddi/cst/print/csharp/CSharpStatementPrinter.java b/maddi-cst-print-csharp/src/main/java/io/codelaser/maddi/cst/print/csharp/CSharpStatementPrinter.java index 964f43d7e..22a4a47eb 100644 --- a/maddi-cst-print-csharp/src/main/java/io/codelaser/maddi/cst/print/csharp/CSharpStatementPrinter.java +++ b/maddi-cst-print-csharp/src/main/java/io/codelaser/maddi/cst/print/csharp/CSharpStatementPrinter.java @@ -576,7 +576,7 @@ private static List entryLabels(SwitchEntry entry, Qualification List conditions = entry.conditions(); // a pattern's arm, case Field field ->, has no condition if (entry.patternVariable() != null) { - return List.of(caseLabel(null, entry.patternVariable(), entry.whenExpression(), q)); + return List.of(caseLabel(entry, q)); } if (conditions.isEmpty() || conditions.stream().allMatch(CSharpStatementPrinter::isDefault)) { return List.of(new OutputBuilderImpl().add(KeywordImpl.DEFAULT).add(SymbolEnum.COLON_LABEL)); @@ -686,7 +686,7 @@ private static OutputBuilder switchArm(SwitchEntry entry, ParameterizedType type List conditions = entry.conditions(); if (entry.patternVariable() != null) { // a record pattern's arm has no condition - head = pattern(entry.patternVariable(), q); + head = entryPattern(entry, q); } else if (conditions.isEmpty() || conditions.stream().allMatch(CSharpStatementPrinter::isDefault)) { head = text("_"); } else { @@ -700,6 +700,38 @@ private static OutputBuilder switchArm(SwitchEntry entry, ParameterizedType type .add(armValue(entry.statement(), type, q)); } + /** {@code case T t when (c):} for a pattern arm, every pattern of the label included. */ + private static OutputBuilder caseLabel(SwitchEntry entry, Qualification q) { + OutputBuilder b = new OutputBuilderImpl().add(KeywordImpl.CASE).add(SpaceEnum.ONE).add(entryPattern(entry, q)); + if (entry.whenExpression() != null && !entry.whenExpression().isEmpty()) { + b.add(SpaceEnum.ONE).add(CSharpKeyword.WHEN).add(SpaceEnum.ONE) + .add(CSharpExpressionPrinter.print(entry.whenExpression(), q)); + } + return b.add(SymbolEnum.COLON_LABEL); + } + + /** + * The pattern of an arm. A label with several patterns ({@code case String _, Integer _ ->}, JEP 456) is C#'s + * {@code or} pattern, {@code string or int}: printing the first alone compiled and no longer matched an Integer. + * Java allows several patterns only when none binds a variable, and C# allows none under {@code or}, so each + * type pattern is written as its type alone. + */ + private static OutputBuilder entryPattern(SwitchEntry entry, Qualification q) { + if (entry.otherPatterns().isEmpty()) return pattern(entry.patternVariable(), q); + OutputBuilder b = new OutputBuilderImpl(); + List all = new ArrayList<>(); + all.add(entry.patternVariable()); + all.addAll(entry.otherPatterns()); + for (int i = 0; i < all.size(); i++) { + if (i > 0) b.add(SpaceEnum.ONE).add(text("or")).add(SpaceEnum.ONE); + RecordPattern p = all.get(i); + b.add(p.localVariable() != null + ? text(CSharpTypeName.pattern(p.localVariable().parameterizedType(), q)) + : pattern(p, q)); + } + return b; + } + private static OutputBuilder joinOr(List conditions, Qualification q) { OutputBuilder b = new OutputBuilderImpl(); for (int i = 0; i < conditions.size(); i++) { diff --git a/maddi-cst-print-csharp/src/test/java/io/codelaser/maddi/cst/print/csharp/TestJavaToCSharpStructure.java b/maddi-cst-print-csharp/src/test/java/io/codelaser/maddi/cst/print/csharp/TestJavaToCSharpStructure.java index 119f444d2..f655864b2 100644 --- a/maddi-cst-print-csharp/src/test/java/io/codelaser/maddi/cst/print/csharp/TestJavaToCSharpStructure.java +++ b/maddi-cst-print-csharp/src/test/java/io/codelaser/maddi/cst/print/csharp/TestJavaToCSharpStructure.java @@ -2657,4 +2657,33 @@ public void innerClasses() { // an inner class that does not use its enclosing instance does not take it contains(cs, "internal sealed class Plain { internal int One() => 1; }"); } + + /** + * A case label with several patterns (JEP 456), {@code case String _, Integer _ ->}: C#'s {@code or} pattern, + * {@code string or int}, in a switch statement and in a switch expression. Translating only the first pattern + * compiles and no longer matches an Integer. + */ + @Test + public void multiPatternCase() { + String cs = translate("Kinds", """ + package a; + class Kinds { + static String statement(Object o) { + switch (o) { + case String _, Integer _ -> { return "known"; } + default -> { } + } + return "?"; + } + static String expression(Object o) { + return switch (o) { + case String _, Integer _ -> "known"; + default -> "?"; + }; + } + } + """); + contains(cs, "case string or int: return \"known\";"); + contains(cs, "string or int => \"known\""); + } } diff --git a/maddi-cst-print-csharp/src/test/resources/io/codelaser/maddi/cst/print/csharp/golden/multiPatternCase-Kinds.cs b/maddi-cst-print-csharp/src/test/resources/io/codelaser/maddi/cst/print/csharp/golden/multiPatternCase-Kinds.cs new file mode 100644 index 000000000..1817f25e0 --- /dev/null +++ b/maddi-cst-print-csharp/src/test/resources/io/codelaser/maddi/cst/print/csharp/golden/multiPatternCase-Kinds.cs @@ -0,0 +1,11 @@ +namespace A; +internal sealed class Kinds { + internal static string Statement(object o) { + switch (o) { + case string or int: return "known"; + default: break; + } + return "?"; + } + internal static string Expression(object o) => o switch { string or int => "known", _ => "?" }; +} diff --git a/maddi-cst-print-kotlin/src/main/java/io/codelaser/maddi/cst/print/kotlin/KotlinStatementPrinter.java b/maddi-cst-print-kotlin/src/main/java/io/codelaser/maddi/cst/print/kotlin/KotlinStatementPrinter.java index 2328dc8ab..a3b80ab7c 100644 --- a/maddi-cst-print-kotlin/src/main/java/io/codelaser/maddi/cst/print/kotlin/KotlinStatementPrinter.java +++ b/maddi-cst-print-kotlin/src/main/java/io/codelaser/maddi/cst/print/kotlin/KotlinStatementPrinter.java @@ -692,11 +692,23 @@ private static boolean isDefault(SwitchEntry e) { && e.conditions().stream().allMatch(c -> c.isEmpty() || c instanceof NullConstant); } - /** {@code case CmdLineException c ->}: a type pattern, Java 21's. Not a record's deconstruction. */ + /** + * {@code case CmdLineException c ->}: a type pattern, Java 21's. Not a record's deconstruction. With several + * patterns ({@code case A _, B _ ->}, JEP 456) every one of them has to be a type pattern: Kotlin's + * {@code is A, is B} tests the type only, which a deconstruction does not reduce to. + */ private static boolean typePattern(SwitchEntry e) { - return e.patternVariable() != null - && (e.patternVariable().patterns() == null || e.patternVariable().patterns().isEmpty()) - && (e.patternVariable().localVariable() != null || e.patternVariable().recordType() != null); + return e.patternVariable() != null && isTypePattern(e.patternVariable()) + && e.otherPatterns().stream().allMatch(KotlinStatementPrinter::isTypePattern); + } + + private static boolean isTypePattern(io.codelaser.maddi.cst.api.element.RecordPattern p) { + return (p.patterns() == null || p.patterns().isEmpty()) + && (p.localVariable() != null || p.recordType() != null); + } + + private static ParameterizedType patternType(io.codelaser.maddi.cst.api.element.RecordPattern p) { + return p.localVariable() != null ? p.localVariable().parameterizedType() : p.recordType(); } private static OutputBuilder whenEntry(SwitchEntry e, Expression selector, boolean intSelector, Qualification q) { @@ -712,6 +724,11 @@ private static OutputBuilder whenEntry(SwitchEntry e, Expression selector, boole } b.add(new TextImpl("is " + KotlinTypeName.of(type.withNullable( io.codelaser.maddi.cst.api.type.NullableState.NONNULL), q))); + // case A _, B _ -> (JEP 456): is A, is B. Printing only the first compiled and stopped matching B. + for (io.codelaser.maddi.cst.api.element.RecordPattern other : e.otherPatterns()) { + b.add(SymbolEnum.COMMA).add(new TextImpl("is " + KotlinTypeName.of(patternType(other).withNullable( + io.codelaser.maddi.cst.api.type.NullableState.NONNULL), q))); + } if (e.whenExpression() != null && !e.whenExpression().isEmpty()) { // case X x when cond: Kotlin's guard b.add(SpaceEnum.ONE).add(KotlinKeyword.IF).add(SpaceEnum.ONE).add(KotlinExpressionPrinter.print(e.whenExpression(), q)); diff --git a/maddi-cst-print-kotlin/src/test/java/io/codelaser/maddi/cst/print/kotlin/TestJavaToKotlinStructure.java b/maddi-cst-print-kotlin/src/test/java/io/codelaser/maddi/cst/print/kotlin/TestJavaToKotlinStructure.java index f3df66724..43e8ccac6 100644 --- a/maddi-cst-print-kotlin/src/test/java/io/codelaser/maddi/cst/print/kotlin/TestJavaToKotlinStructure.java +++ b/maddi-cst-print-kotlin/src/test/java/io/codelaser/maddi/cst/print/kotlin/TestJavaToKotlinStructure.java @@ -242,6 +242,27 @@ static String kind(Object o) { compiles(kotlin); } + /** + * A case label with several patterns (JEP 456), {@code case String _, Integer _ ->}: Kotlin's {@code is String, is + * Integer ->}. Translating only the first, {@code is String ->}, compiles and no longer matches an Integer. + */ + @Test + public void multiPatternCase() { + String kotlin = kotlin(""" + package a; + class Kinds { + static String kind(Object o) { + return switch (o) { + case String _, Integer _ -> "known"; + default -> "?"; + }; + } + } + """); + contains(kotlin, "is String, is Int ->"); + compiles(kotlin); + } + /** * nacos's RpcConstants.ClientSuffix: a member class, not static, of constants only. Kotlin's inner class can have * no companion object; nothing of it uses the outer instance, so it is a nested class.