Scala parser: eliminate J.Unknown, add S types, fix all test failures including match guards and template matching

Author: jkschneider

rewrite-scala/CLAUDE.md

@@ -1,28 +1,42 @@
 # rewrite-scala Guidelines
 
+## J.Unknown is forbidden
+
+**NOTHING should map to J.Unknown.** The parser must throw an exception for any
+Scala syntax it cannot map to a proper J-type or S-type. This ensures we discover
+gaps immediately rather than silently degrading to lossy source-text preservation.
+
+If the parser encounters a tree node it doesn't handle:
+1. Map it to the correct J-type — think carefully about which one fits.
+2. If no J-type works, create an S-type (Scala-specific AST node).
+3. If the syntax is genuinely new/unknown, **throw an exception** so the gap is
+   caught by tests, not silently swallowed.
+
+**Never use J.Unknown, visitUnknown, or raw source text as the value of a J-type
+field.** These break the semantic model and prevent recipes from operating on code.
+
 ## LST Mapping Rules
 
 **Never fall back to raw source text as the value of a J-type field.** Every AST element
 must be mapped to a proper J-type (or S-type) with correct structure. Stuffing source
 text into an identifier name, unknown source, or string field breaks the semantic model
 and prevents recipes from operating on that code.
 
-If a Scala construct doesn't map cleanly to an existing J-type:
-1. Think harder about which J-type fits — type bounds, annotations, modifiers, and
-   containers are more expressive than they first appear.
-2. If no J-type works, consider whether an S-type (Scala-specific AST node) is needed.
-3. Ask before resorting to `J.Unknown` — it should only be used as a last resort for
-   truly unmappable syntax, and even then the tree inside the Unknown should be
-   properly structured when possible.
+**Map to the semantically correct type.** Don't use `J.TypeCast` for type ascription
+(`expr: Type`) — that's not a cast. Don't use `J.Literal` for interpolated strings —
+they have internal structure. If the right type doesn't exist in J.*, create an S.* type.
+
+**Never store LST elements inside markers.** Markers are metadata that influence how
+an LST element is printed (like `Curried`, `OmitBraces`), not containers for additional
+AST subtrees.
 
 ## Critical Principles
 
 **Never regress from rich types to J.Unknown.** Once a syntax element has been mapped
 to a rich type (J.* or S.*), never revert it back to J.Unknown.
 
 **Cursor management:** When a visitor method calls `extractPrefix` or `extractSource`
-and then falls back to `visitUnknown`, always restore the cursor first so the Unknown
-node gets the correct prefix whitespace.
+and then falls back, always restore the cursor first.
 
 ## Testing
 

rewrite-scala/src/main/java/org/openrewrite/scala/ScalaIsoVisitor.java

@@ -38,8 +38,8 @@ public S.Wildcard visitWildcard(S.Wildcard wildcard, P p) {
     }
 
     @Override
-    public S.BlockExpression visitBlockExpression(S.BlockExpression blockExpression, P p) {
-        return (S.BlockExpression) super.visitBlockExpression(blockExpression, p);
+    public J visitStatementExpression(S.StatementExpression statementExpression, P p) {
+        return super.visitStatementExpression(statementExpression, p);
     }
 
     @Override

rewrite-scala/src/main/java/org/openrewrite/scala/ScalaPrinter.java

@@ -34,7 +34,6 @@
 import org.openrewrite.scala.marker.SObject;
 import org.openrewrite.scala.marker.TypeProjection;
 import org.openrewrite.scala.marker.ScalaForLoop;
-import org.openrewrite.scala.marker.AsInstanceOfPrefix;
 import org.openrewrite.scala.marker.TypeAscription;
 import org.openrewrite.scala.marker.UnderscorePlaceholderLambda;
 import org.openrewrite.scala.tree.S;
@@ -200,12 +199,18 @@ public J visitTry(J.Try tryable, PrintOutputCapture<P> p) {
         p.append("try");
         visit(tryable.getBody(), p);
         if (!tryable.getCatches().isEmpty()) {
-            J.Try.Catch firstCatch = tryable.getCatches().get(0);
-            visitSpace(firstCatch.getPrefix(), Space.Location.CATCH_PREFIX, p);
-            p.append("catch {");
-            for (J.Try.Catch aCatch : tryable.getCatches()) {
+            // Print catch block with cases from AST whitespace
+            for (int i = 0; i < tryable.getCatches().size(); i++) {
+                J.Try.Catch aCatch = tryable.getCatches().get(i);
+                if (i == 0) {
+                    // First catch — prefix is the space before "catch"
+                    visitSpace(aCatch.getPrefix(), Space.Location.CATCH_PREFIX, p);
+                    p.append("catch {");
+                }
+                // Print case with AST whitespace
                 J.VariableDeclarations varDecl = aCatch.getParameter().getTree();
-                p.append("\n  case");
+                visitSpace(varDecl.getPrefix(), Space.Location.VARIABLE_DECLARATIONS_PREFIX, p);
+                p.append("case");
                 if (!varDecl.getVariables().isEmpty()) {
                     visit(varDecl.getVariables().get(0).getName(), p);
                 }
@@ -218,7 +223,10 @@ public J visitTry(J.Try tryable, PrintOutputCapture<P> p) {
                     visit(stmt, p);
                 }
             }
-            p.append("\n}");
+            // Close catch block — use end space from last catch body if available
+            J.Try.Catch lastCatch = tryable.getCatches().get(tryable.getCatches().size() - 1);
+            visitSpace(lastCatch.getBody().getEnd(), Space.Location.BLOCK_END, p);
+            p.append("}");
         }
         if (tryable.getPadding().getFinally() != null) {
             visitSpace(tryable.getPadding().getFinally().getBefore(), Space.Location.TRY_FINALLY, p);
@@ -249,29 +257,21 @@ public J visitSwitch(J.Switch switch_, PrintOutputCapture<P> p) {
     public J visitCase(J.Case case_, PrintOutputCapture<P> p) {
         beforeSyntax(case_, Space.Location.CASE_PREFIX, p);
         p.append("case");
-        List<JRightPadded<J>> labels = case_.getPadding().getCaseLabels().getPadding().getElements();
-        for (JRightPadded<J> label : labels) {
-            visit(label.getElement(), p);
-            visitSpace(label.getAfter(), Space.Location.CASE_LABEL, p);
+        for (J label : case_.getCaseLabels()) {
+            visit(label, p);
         }
         if (case_.getGuard() != null) {
-            p.append("if");
+            p.append(" if");
             visit(case_.getGuard(), p);
         }
         p.append(" =>");
         if (case_.getPadding().getBody() != null) {
             J bodyElement = case_.getPadding().getBody().getElement();
-            // For Scala match cases, unwrap blocks to avoid printing { }
+            // For Scala match cases, unwrap single-statement blocks to avoid { }
             if (bodyElement instanceof J.Block) {
                 J.Block block = (J.Block) bodyElement;
-                visitSpace(block.getPrefix(), Space.Location.BLOCK_PREFIX, p);
-                List<JRightPadded<Statement>> paddedStatements = block.getPadding().getStatements();
-                for (int i = 0; i < paddedStatements.size(); i++) {
-                    JRightPadded<Statement> paddedStmt = paddedStatements.get(i);
-                    visit(paddedStmt.getElement(), p);
-                    if (i < paddedStatements.size() - 1) {
-                        visitSpace(paddedStmt.getAfter(), Space.Location.BLOCK_STATEMENT_SUFFIX, p);
-                    }
+                for (Statement stmt : block.getStatements()) {
+                    visit(stmt, p);
                 }
             } else {
                 visit(bodyElement, p);
@@ -401,7 +401,19 @@ public J visitMethodDeclaration(J.MethodDeclaration method, PrintOutputCapture<P
                 if (!procedureSyntax) {
                     p.append(" =");
                 }
-                visit(actualBody, p);
+                // If body is OmitBraces block with single statement, print just the statement
+                if (actualBody instanceof J.Block) {
+                    J.Block bodyBlock = (J.Block) actualBody;
+                    boolean omit = bodyBlock.getMarkers().findFirst(
+                            org.openrewrite.scala.marker.OmitBraces.class).isPresent();
+                    if (omit && bodyBlock.getStatements().size() == 1) {
+                        visit(bodyBlock.getStatements().get(0), p);
+                    } else {
+                        visit(actualBody, p);
+                    }
+                } else {
+                    visit(actualBody, p);
+                }
             }
         } else if (method.getBody() != null) {
             // Normal method body
@@ -481,10 +493,15 @@ public J visit(@Nullable Tree tree, PrintOutputCapture<P> p) {
             return visitWildcard((S.Wildcard) tree, p);
         } else if (tree instanceof S.TuplePattern) {
             return visitTuplePattern((S.TuplePattern) tree, p);
-        } else if (tree instanceof S.BlockExpression) {
-            return visitBlockExpression((S.BlockExpression) tree, p);
-        } else if (tree instanceof S.ExpressionStatement) {
-            return visitExpressionStatement((S.ExpressionStatement) tree, p);
+        } else if (tree instanceof S.StatementExpression) {
+            // Transparent — visit the inner statement
+            return visit(((S.StatementExpression) tree).getStatement(), p);
+        } else if (tree instanceof S.TypeAscription) {
+            return visitTypeAscription((S.TypeAscription) tree, p);
+        } else if (tree instanceof S.TypeAlias) {
+            return visitTypeAlias((S.TypeAlias) tree, p);
+        } else if (tree instanceof S.PatternDefinition) {
+            return visitPatternDefinition((S.PatternDefinition) tree, p);
         }
         return super.visit(tree, p);
     }
@@ -794,8 +811,13 @@ private void visitTypeParameters(@Nullable JContainer<J.TypeParameter> typeParam
 
     @Override
     public J visitBlock(J.Block block, PrintOutputCapture<P> p) {
-        // Check if this block has the OmitBraces marker (for objects without body)
+        // OmitBraces blocks print statements without { } — used for braceless bodies,
+        // synthetic lambda body blocks, and expression-position blocks
         if (block.getMarkers().findFirst(org.openrewrite.scala.marker.OmitBraces.class).isPresent()) {
+            beforeSyntax(block, Space.Location.BLOCK_PREFIX, p);
+            visitStatements(block.getPadding().getStatements(), JRightPadded.Location.BLOCK_STATEMENT, p);
+            visitSpace(block.getEnd(), Space.Location.BLOCK_END, p);
+            afterSyntax(block, p);
             return block;
         }
         // Scala 3 braceless (indentation-based) blocks use `:` instead of `{}`
@@ -865,26 +887,20 @@ public J visitForEachLoop(J.ForEachLoop forEachLoop, PrintOutputCapture<P> p) {
         return super.visitForEachLoop(forEachLoop, p);
     }
 
+    public J visitTypeAscription(S.TypeAscription typeAscription, PrintOutputCapture<P> p) {
+        beforeSyntax(typeAscription, Space.Location.LANGUAGE_EXTENSION, p);
+        visit(typeAscription.getExpression(), p);
+        p.append(':');
+        visit(typeAscription.getTypeTree(), p);
+        afterSyntax(typeAscription, p);
+        return typeAscription;
+    }
+
     @Override
     public J visitTypeCast(J.TypeCast typeCast, PrintOutputCapture<P> p) {
-        if (typeCast.getMarkers().findFirst(TypeAscription.class).isPresent()) {
-            // Scala type ascription: expr: Type
-            beforeSyntax(typeCast, Space.Location.TYPE_CAST_PREFIX, p);
-            visit(typeCast.getExpression(), p);
-            if (typeCast.getClazz() instanceof J.ControlParentheses) {
-                J.ControlParentheses<?> controlParens = (J.ControlParentheses<?>) typeCast.getClazz();
-                visitSpace(controlParens.getPrefix(), Space.Location.CONTROL_PARENTHESES_PREFIX, p);
-                p.append(':');
-                visitRightPadded(controlParens.getPadding().getTree(), JRightPadded.Location.PARENTHESES, "", p);
-            }
-            afterSyntax(typeCast, p);
-            return typeCast;
-        }
-        // Existing asInstanceOf handling
+        // asInstanceOf handling
         beforeSyntax(typeCast, Space.Location.TYPE_CAST_PREFIX, p);
         visit(typeCast.getExpression(), p);
-        typeCast.getMarkers().findFirst(AsInstanceOfPrefix.class)
-                .ifPresent(sp -> visitSpace(sp.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p));
         p.append(".asInstanceOf");
         if (typeCast.getClazz() instanceof J.ControlParentheses) {
             J.ControlParentheses<?> controlParens = (J.ControlParentheses<?>) typeCast.getClazz();
@@ -967,8 +983,12 @@ public J visitVariable(J.VariableDeclarations.NamedVariable variable, PrintOutpu
         // In Scala, type annotation comes after the name
         J.VariableDeclarations parent = getCursor().getParentOrThrow().getValue();
         if (parent.getTypeExpression() != null) {
+            // Print space before colon if present (e.g., `given IntSchema : SchemaFor[Int]`)
+            // Stored in varargs field (repurposed, unused in Scala)
+            if (parent.getVarargs() != null) {
+                visitSpace(parent.getVarargs(), Space.Location.VARARGS, p);
+            }
             p.append(":");
-            // The type expression should have the space after colon in its prefix
             visit(parent.getTypeExpression(), p);
 
             // If there's an initializer, use visitLeftPadded to handle it properly
@@ -1111,7 +1131,8 @@ public J visitMethodInvocation(J.MethodInvocation method, PrintOutputCapture<P>
                 visit(method.getName(), p);
             }
 
-            // Print the block argument directly (no parentheses)
+            // Print the block argument — it's typically an S.StatementExpression(J.Block)
+            // The J.Block contains the lambda. visitBlock prints the { } braces.
             if (method.getArguments() != null) {
                 for (Expression arg : method.getArguments()) {
                     visit(arg, p);
@@ -1229,16 +1250,17 @@ public J visitWildcard(S.Wildcard wildcard, PrintOutputCapture<P> p) {
         return wildcard;
     }
 
-    public J visitExpressionStatement(S.ExpressionStatement expressionStatement, PrintOutputCapture<P> p) {
-        visit(expressionStatement.getExpression(), p);
-        return expressionStatement;
+    public J visitTypeAlias(S.TypeAlias typeAlias, PrintOutputCapture<P> p) {
+        beforeSyntax(typeAlias, Space.Location.LANGUAGE_EXTENSION, p);
+        p.append(typeAlias.getText());
+        afterSyntax(typeAlias, p);
+        return typeAlias;
     }
 
-    public J visitBlockExpression(S.BlockExpression blockExpression, PrintOutputCapture<P> p) {
-        beforeSyntax(blockExpression, Space.Location.LANGUAGE_EXTENSION, p);
-        // Simply visit the contained block - it will print itself with braces
-        visit(blockExpression.getBlock(), p);
-        afterSyntax(blockExpression, p);
-        return blockExpression;
+    public J visitPatternDefinition(S.PatternDefinition patDef, PrintOutputCapture<P> p) {
+        beforeSyntax(patDef, Space.Location.LANGUAGE_EXTENSION, p);
+        p.append(patDef.getText());
+        afterSyntax(patDef, p);
+        return patDef;
     }
 }

rewrite-scala/src/main/java/org/openrewrite/scala/ScalaVisitor.java

@@ -20,7 +20,6 @@
 import org.openrewrite.internal.ListUtils;
 import org.openrewrite.internal.lang.Nullable;
 import org.openrewrite.java.JavaVisitor;
-import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.JContainer;
 import org.openrewrite.java.tree.JRightPadded;
@@ -129,22 +128,31 @@ public J visitWildcard(S.Wildcard wildcard, P p) {
         return w;
     }
 
-    public J visitExpressionStatement(S.ExpressionStatement expressionStatement, P p) {
-        S.ExpressionStatement e = expressionStatement;
-        J j = visit(e.getExpression(), p);
-        if (j instanceof S.ExpressionStatement) {
-            return j;
-        } else if (j instanceof Expression) {
-            return e.withExpression((Expression) j);
-        }
-        return j;
+    public J visitStatementExpression(S.StatementExpression statementExpression, P p) {
+        // Transparent — just visit the inner statement
+        return statementExpression.acceptScala(this, p);
+    }
+
+    public J visitTypeAscription(S.TypeAscription typeAscription, P p) {
+        S.TypeAscription t = typeAscription;
+        t = t.withPrefix(visitSpace(t.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p));
+        t = t.withMarkers(visitMarkers(t.getMarkers(), p));
+        t = t.withExpression(visitAndCast(t.getExpression(), p));
+        t = t.withTypeTree(visitAndCast(t.getTypeTree(), p));
+        return t;
+    }
+
+    public J visitTypeAlias(S.TypeAlias typeAlias, P p) {
+        S.TypeAlias t = typeAlias;
+        t = t.withPrefix(visitSpace(t.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p));
+        t = t.withMarkers(visitMarkers(t.getMarkers(), p));
+        return t;
     }
 
-    public J visitBlockExpression(S.BlockExpression blockExpression, P p) {
-        S.BlockExpression b = blockExpression;
-        b = b.withPrefix(visitSpace(b.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p));
-        b = b.withMarkers(visitMarkers(b.getMarkers(), p));
-        b = b.withBlock(visitAndCast(b.getBlock(), p));
-        return b;
+    public J visitPatternDefinition(S.PatternDefinition patDef, P p) {
+        S.PatternDefinition pd = patDef;
+        pd = pd.withPrefix(visitSpace(pd.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p));
+        pd = pd.withMarkers(visitMarkers(pd.getMarkers(), p));
+        return pd;
     }
 }