From 4c3ba414ad5ad5fb2961c7b27412b7c185882256 Mon Sep 17 00:00:00 2001 From: Nick McKinney Date: Tue, 17 Jan 2023 14:58:49 -0500 Subject: [PATCH 1/2] Adding visited-file details to unexpected exceptions (#2655) --- .../java/org/openrewrite/TreeVisitor.java | 14 ++++++++ .../internal/FindRecipeRunException.java | 2 +- .../internal/RecipeRunException.java | 5 +++ .../java/org/openrewrite/marker/Markup.java | 17 ++++++--- .../org/openrewrite/RecipeSchedulerTest.java | 5 ++- .../org/openrewrite/java/JavaVisitorTest.java | 36 +++++++++++++++++++ .../org/openrewrite/java/JavaVisitor.java | 17 +++++++++ 7 files changed, 90 insertions(+), 6 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java b/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java index 43b0eed39af..b90ea809fac 100644 --- a/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java +++ b/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java @@ -321,6 +321,11 @@ public T visit(@Nullable Tree tree, P p) { throw e; } + final String visitedLocation = describeLocation(getCursor()); + if (visitedLocation != null) { + throw new RecipeRunException(e, getCursor(), + String.format("Exception while visiting project file '%s'", visitedLocation)); + } throw new RecipeRunException(e, getCursor()); } @@ -410,4 +415,13 @@ public > V adapt(Class } return TreeVisitorAdapter.adapt(this, adaptTo); } + + @Nullable + protected String describeLocation(Cursor cursor) { + SourceFile sourceFile = cursor.firstEnclosing(SourceFile.class); + if (sourceFile == null) { + return null; + } + return sourceFile.getSourcePath().toString(); + } } diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/FindRecipeRunException.java b/rewrite-core/src/main/java/org/openrewrite/internal/FindRecipeRunException.java index 61a2011652c..72e04697b46 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/FindRecipeRunException.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/FindRecipeRunException.java @@ -34,7 +34,7 @@ public FindRecipeRunException(RecipeRunException rre) { @Override public Tree preVisit(Tree tree, Integer integer) { if (tree == nearestTree) { - return Markup.error(tree, vt.getCause()); + return Markup.error(tree, vt); } return tree; } diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java b/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java index 22acb02ac87..e769e38a441 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java @@ -31,4 +31,9 @@ public RecipeRunException(Throwable cause, @Nullable Cursor cursor) { super(cause); this.cursor = cursor; } + + public RecipeRunException(Throwable cause, @Nullable Cursor cursor, String message) { + super(String.format("%s, caused by: %s", message, cause.toString()), cause); + this.cursor = cursor; + } } diff --git a/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java b/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java index a14c32b190c..5befaf7bf93 100644 --- a/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java +++ b/rewrite-core/src/main/java/org/openrewrite/marker/Markup.java @@ -23,6 +23,7 @@ import org.openrewrite.RecipeScheduler; import org.openrewrite.Tree; import org.openrewrite.internal.ExceptionUtils; +import org.openrewrite.internal.RecipeRunException; import org.openrewrite.internal.lang.NonNull; import org.openrewrite.internal.lang.Nullable; @@ -84,13 +85,17 @@ class Error implements Markup { @Override public String getMessage() { - return exception.getMessage(); + return getCause().getMessage(); } @Override @NonNull public String getDetail() { - return ExceptionUtils.sanitizeStackTrace(exception, RecipeScheduler.class); + return ExceptionUtils.sanitizeStackTrace(getCause(), RecipeScheduler.class); + } + + private Throwable getCause() { + return exception instanceof RecipeRunException ? exception.getCause() : exception; } @Override @@ -115,13 +120,17 @@ class Warn implements Markup { @Override public String getMessage() { - return exception.getMessage(); + return getCause().getMessage(); } @Override @NonNull public String getDetail() { - return ExceptionUtils.sanitizeStackTrace(exception, RecipeScheduler.class); + return ExceptionUtils.sanitizeStackTrace(getCause(), RecipeScheduler.class); + } + + private Throwable getCause() { + return exception instanceof RecipeRunException ? exception.getCause() : exception; } @Override diff --git a/rewrite-core/src/test/java/org/openrewrite/RecipeSchedulerTest.java b/rewrite-core/src/test/java/org/openrewrite/RecipeSchedulerTest.java index b10c6c6939c..e06a0e9934d 100644 --- a/rewrite-core/src/test/java/org/openrewrite/RecipeSchedulerTest.java +++ b/rewrite-core/src/test/java/org/openrewrite/RecipeSchedulerTest.java @@ -33,7 +33,10 @@ void exceptionsCauseResult() { spec -> spec .executionContext(new InMemoryExecutionContext()) .recipe(new BoomRecipe()) - .afterRecipe(run -> assertThat(run.getResults().get(0).getRecipeErrors()).isNotEmpty()), + .afterRecipe(run -> assertThat(run.getResults().get(0).getRecipeErrors()) + .singleElement() + .satisfies(t -> assertThat(t.getMessage()).isEqualTo("Exception while visiting project file 'file.txt', caused by: org.openrewrite.BoomException: boom")) + ), text( "hello", "~~(boom)~~>hello" diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaVisitorTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaVisitorTest.java index ccba7f12ba8..65032d4ff02 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/JavaVisitorTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/JavaVisitorTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.ExecutionContext; +import org.openrewrite.InMemoryExecutionContext; import org.openrewrite.java.tree.J; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; import static org.openrewrite.test.RewriteTest.toRecipe; @@ -74,4 +76,38 @@ void removeMethod() {} ) ); } + + @Test + void thrownExceptionsAreSpecific() { + rewriteRun( + spec -> spec + .executionContext(new InMemoryExecutionContext()) + .afterRecipe(run -> assertThat(run.getResults().get(0).getRecipeErrors()) + .singleElement() + .satisfies(t -> assertThat(t.getMessage()).containsSubsequence("A.java", "A", "allTheThings")) + ) + .recipe(toRecipe(() -> new JavaIsoVisitor<>() { + @Override + public J.Literal visitLiteral(final J.Literal literal, final ExecutionContext executionContext) { + throw new IllegalStateException("boom"); + } + })), + java( + """ + class A { + void allTheThings() { + String var = "qwe"; + } + } + """, + """ + class A { + void allTheThings() { + String var = /*~~(boom)~~>*/"qwe"; + } + } + """ + ) + ); + } } diff --git a/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java b/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java index f230adac9f2..d7c5ffbf903 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/JavaVisitor.java @@ -22,6 +22,7 @@ import org.openrewrite.java.tree.*; import org.openrewrite.marker.Markers; +import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -1379,4 +1380,20 @@ protected boolean isInSameNameScope(Cursor base, Cursor child) { protected boolean isInSameNameScope(Cursor child) { return isInSameNameScope(getCursor(), child); } + + @Override + protected @Nullable String describeLocation(Cursor cursor) { + List namedElements = new ArrayList<>(); + for (Iterator it = cursor.getPath(); it.hasNext(); ) { + final Object tree = it.next(); + if (tree instanceof J.ClassDeclaration) { + namedElements.add(0, ((J.ClassDeclaration) tree).getSimpleName()); + } else if (tree instanceof J.MethodDeclaration) { + namedElements.add(0, ((J.MethodDeclaration) tree).getSimpleName()); + } + } + String location = String.join(".", namedElements); + String filename = super.describeLocation(cursor); + return "".equals(location) ? filename : String.format("%s (in %s)", filename, location); + } } From c2779d7e5be1991581e1cc91f53273a268c71ae8 Mon Sep 17 00:00:00 2001 From: Nick McKinney Date: Tue, 17 Jan 2023 15:47:05 -0500 Subject: [PATCH 2/2] polish --- .../src/main/java/org/openrewrite/TreeVisitor.java | 7 +------ .../openrewrite/internal/RecipeRunException.java | 13 +++++++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java b/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java index b90ea809fac..68233fbeb02 100644 --- a/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java +++ b/rewrite-core/src/main/java/org/openrewrite/TreeVisitor.java @@ -321,12 +321,7 @@ public T visit(@Nullable Tree tree, P p) { throw e; } - final String visitedLocation = describeLocation(getCursor()); - if (visitedLocation != null) { - throw new RecipeRunException(e, getCursor(), - String.format("Exception while visiting project file '%s'", visitedLocation)); - } - throw new RecipeRunException(e, getCursor()); + throw new RecipeRunException(e, getCursor(), describeLocation(getCursor())); } //noinspection unchecked diff --git a/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java b/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java index e769e38a441..ffb891c6128 100644 --- a/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java +++ b/rewrite-core/src/main/java/org/openrewrite/internal/RecipeRunException.java @@ -28,12 +28,17 @@ public class RecipeRunException extends RuntimeException { private final Cursor cursor; public RecipeRunException(Throwable cause, @Nullable Cursor cursor) { - super(cause); - this.cursor = cursor; + this(cause, cursor, null); } - public RecipeRunException(Throwable cause, @Nullable Cursor cursor, String message) { - super(String.format("%s, caused by: %s", message, cause.toString()), cause); + public RecipeRunException(Throwable cause, @Nullable Cursor cursor, @Nullable String visitedLocation) { + super(message(visitedLocation, cause), cause); this.cursor = cursor; } + + @Nullable + private static String message(@Nullable String visitedLocation, Throwable cause) { + return visitedLocation == null ? null + : String.format("Exception while visiting project file '%s', caused by: %s", visitedLocation, cause); + } }