Skip to content

Commit

Permalink
Added ThrowingEx.unwrapAsRuntimeOrRethrow for unwrapping exceptions e…
Browse files Browse the repository at this point in the history
…asily, and used this logic to enhance the error messages thrown by KtLintStep.
  • Loading branch information
nedtwigg committed Jan 11, 2017
1 parent 8d8d542 commit d8f5eee
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
33 changes: 32 additions & 1 deletion lib/src/main/java/com/diffplug/spotless/ThrowingEx.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,42 @@ public static RuntimeException asRuntime(Exception e) {
}
}

/**
* Utility method for rethrowing an exception's cause with as few wrappers as possible.
*
* ```java
* try {
* doSomething();
* } catch (Throwable e) {
* throw unwrapAsRuntimeOrRethrow(e);
* }
* ```
*/
public static RuntimeException unwrapAsRuntimeOrRethrow(Throwable e) {

This comment has been minimized.

Copy link
@jbduncan

jbduncan Jan 15, 2017

Member

@nedtwigg I find this method name to be confusing.

How about naming it something like throwCauseAsIsIfErrorElseAsRuntime?

This comment has been minimized.

Copy link
@nedtwigg

nedtwigg Jan 15, 2017

Author Member

What do you think about just throw unwrapCause(e)? That's the method's only goal, the other stuff is just noise to get the exception types to work with a minimum of wrapping. The current name puts some of the details in, and your name puts more details in. The method fulfills a very simple goal, its implementation just happens to be weirdly surprising because of the checked exception nightmare. So long as it's only used as throw unwrapCause(e), it will work perfect. The reason for the long name is to make sure people aren't too surprised if they call it in some other context, but at this point I'm not so sure it's worth it.

I agree that the current name is bad. I'd rather use your suggestion thisIsExactlyHowThisMethodWorks, or methodGoal, but not the current halfway solution.

This comment has been minimized.

Copy link
@jbduncan

jbduncan Jan 15, 2017

Member

Yes, renaming it to unwrapCause and replacing the code with throw unwrapCause sounds like a neat alternative to me!

I think it's better than my own suggestion in a way because, as you said, it describes the goal rather than how exactly it works, which makes it more concise/easier to read. After all, even if we give the method a nice, long name describing what it does, it doesn't really substitute reading the code of the method itself (or good javadoc) to understand unambiguously what it does (as well as how it differs to just exception.getCause()).

Therefore I'm happy for this method to be renamed as unwrapCause!

Throwable cause = e.getCause();
if (cause == null) {
return ifErrorRethrowElseAsRuntime(e);
} else {
return ifErrorRethrowElseAsRuntime(cause);
}
}

/** Rethrows errors, wraps and returns everything else as a runtime exception. */
private static RuntimeException ifErrorRethrowElseAsRuntime(Throwable e) {
if (e instanceof Error) {
throw (Error) e;
} else if (e instanceof RuntimeException) {
return (RuntimeException) e;
} else {
return new WrappedAsRuntimeException(e);
}
}

/** A RuntimeException specifically for the purpose of wrapping non-runtime Exceptions as RuntimeExceptions. */
public static class WrappedAsRuntimeException extends RuntimeException {
private static final long serialVersionUID = -912202209702586994L;

public WrappedAsRuntimeException(Exception e) {
public WrappedAsRuntimeException(Throwable e) {
super(e);
}
}
Expand Down
10 changes: 8 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/kotlin/KtLintStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.io.IOException;
import java.io.Serializable;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
import java.util.Collections;
Expand All @@ -25,6 +26,7 @@
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.JarState;
import com.diffplug.spotless.Provisioner;
import com.diffplug.spotless.ThrowingEx;

/** Wraps up [ktlint](https://github.com/shyiko/ktlint) as a FormatterStep. */
public class KtLintStep {
Expand Down Expand Up @@ -91,8 +93,12 @@ FormatterFunc createFormat() throws Exception {
Method formatterMethod = ktlintClass.getMethod("format", String.class, Iterable.class, function2Interface);

return input -> {
String formatted = (String) formatterMethod.invoke(ktlint, input, ruleSets, formatterCallback);
return formatted;
try {
String formatted = (String) formatterMethod.invoke(ktlint, input, ruleSets, formatterCallback);
return formatted;
} catch (InvocationTargetException e) {
throw ThrowingEx.unwrapAsRuntimeOrRethrow(e);
}
};
}
}
Expand Down

0 comments on commit d8f5eee

Please sign in to comment.