diff options
author | Yigit Boyar <yboyar@google.com> | 2020-03-04 21:11:41 -0800 |
---|---|---|
committer | Yigit Boyar <yboyar@google.com> | 2020-03-06 10:13:19 -0800 |
commit | 3f8881b32647b45fd1e29c3865d4d3793cb638d4 (patch) | |
tree | 9eb4f0832c74074351db78a20259b71284bf7f4f | |
parent | d9c256a234ddd5f5390f45453322f114e7baae9f (diff) | |
download | data-binding-3f8881b32647b45fd1e29c3865d4d3793cb638d4.tar.gz |
Handle cases where upperbound of a TypeVariable might return null
This CL adds a safety to how we handle upper bounds.
There is a very interesting bug where if someone bumps the jetbrains
annotations to v15 from 13 (which is depended upon by kotlin), the
compiler weirdly returns null for the upper bound of an annotated type
variable.
Unfortunately, there is no test as I couldn't figure out how to reproduce
this besides that interesting instance. I've looked into the jars for v15
and v13 and nullable class is there both.
I've also added more logging to such failures where we receive an exception
that we don't expect. This will help with errors in the future where we'll
at least know which layout caused the error hence we can diagnose it better.
Sample error output when invoked from IDE:
https://paste.googleplex.com/6688355217571840
Sample error output when invoked from command line:
https://paste.googleplex.com/5133349121163264
FYI this is still an exception not a deferred thing as it is really unexpected
hence no reason to defer execution and lose the cause of the exception.
Bug: 144300600
Test: unfortunately manual but I've verified the repro in the bug is fixed or it
also prints the layout name w/ the stack trace if the exception happens (before
the fix)
Change-Id: Ie42f24703f4dca134e7848a3eac9b7b7c45f55eb
6 files changed, 30 insertions, 5 deletions
diff --git a/compiler/src/main/java/android/databinding/tool/DataBinder.java b/compiler/src/main/java/android/databinding/tool/DataBinder.java index a5b193b4..f37ffd6e 100644 --- a/compiler/src/main/java/android/databinding/tool/DataBinder.java +++ b/compiler/src/main/java/android/databinding/tool/DataBinder.java @@ -16,6 +16,7 @@ package android.databinding.tool; +import android.databinding.tool.processing.ErrorMessages; import android.databinding.tool.processing.Scope; import android.databinding.tool.processing.ScopedException; import android.databinding.tool.store.ResourceBundle; @@ -142,6 +143,10 @@ public class DataBinder { mFileWriter.writeToFile(canonicalName, layoutBinder.writeViewBinder(minSdk)); } catch (ScopedException ex) { Scope.defer(ex); + } catch (Throwable t) { + // if an unexpected error happens, wrap it in a deferred exception so that we can + // know what happened + L.e(t, ErrorMessages.UNEXPECTED_ERROR_IN_LAYOUT, layoutBinder.getLayoutname()); } finally { Scope.exit(); } diff --git a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.kt b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.kt index 5fd3418d..1515703c 100644 --- a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.kt +++ b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationClass.kt @@ -57,7 +57,8 @@ class AnnotationClass( override fun toDeclarationCode(): String { if (typeMirror is TypeVariable) { // if it is a type var, use upper bound - return AnnotationTypeUtil.getInstance().toJava(typeMirror.upperBound) + // see b/144300600 for the fallback to typeMirror itself + return AnnotationTypeUtil.getInstance().toJava(typeMirror.upperBound ?: typeMirror) } return AnnotationTypeUtil.getInstance().toJava(typeMirror) } @@ -275,7 +276,8 @@ class AnnotationClass( } private val computedCanonicalName by lazy(LazyThreadSafetyMode.NONE) { - AnnotationTypeUtil.getInstance().toJava(typeUtils.erasure(typeMirror)) + // see b/144300600 for the fallback to typeMirror itself + AnnotationTypeUtil.getInstance().toJava(typeUtils.erasure(typeMirror) ?: typeMirror) } override val canonicalName: String = computedCanonicalName diff --git a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationTypeUtil.java b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationTypeUtil.java index a4195e7a..16d872d0 100644 --- a/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationTypeUtil.java +++ b/compiler/src/main/java/android/databinding/tool/reflection/annotation/AnnotationTypeUtil.java @@ -21,6 +21,7 @@ import android.databinding.tool.reflection.ModelMethod; import android.databinding.tool.reflection.RecursiveResolutionStack; import android.databinding.tool.reflection.TypeUtil; +import com.android.annotations.NonNull; import java.util.List; import java.util.stream.Collectors; @@ -124,7 +125,7 @@ public class AnnotationTypeUtil extends TypeUtil { * Returns the java representation of a TypeMirror type. For example, this may return * "java.util.Set<java.lang.String>" */ - public String toJava(TypeMirror typeMirror) { + public String toJava(@NonNull TypeMirror typeMirror) { return mToJavaResolutionStack.visit( typeMirror, current -> { @@ -265,6 +266,10 @@ public class AnnotationTypeUtil extends TypeUtil { private String toJava(TypeVariable typeVariable) { StringBuilder sb = new StringBuilder("?"); TypeMirror upperBound = typeVariable.getUpperBound(); + if (upperBound == null) { + // see b/144300600 about his fallback + upperBound = typeVariable; + } String upperBoundString = toJava(upperBound); if (!"java.lang.Object".equals(upperBoundString)) { sb.append(" extends ").append(upperBoundString); diff --git a/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java b/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java index e9f8eab2..c3ebf073 100644 --- a/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java +++ b/compilerCommon/src/main/java/android/databinding/tool/processing/ErrorMessages.java @@ -17,6 +17,8 @@ package android.databinding.tool.processing; public class ErrorMessages { + private static final String BUGANIZER_URL = + "https://issuetracker.google.com/issues/new?component=192721&template=1096850"; public static final String INCLUDE_INSIDE_MERGE = "<include> elements are not supported as direct children of <merge> elements"; @@ -102,4 +104,9 @@ public class ErrorMessages { public static final String DUPLICATE_VIEW_OR_INCLUDE_ID = "<%s id='%s'> conflicts with another tag that has the same ID"; + + public static final String UNEXPECTED_ERROR_IN_LAYOUT = + "Unexpected error while processing layout file: %s.xml\n" + + "\n" + + "Please file a bug on " + BUGANIZER_URL + " with a sample project that reproduces the problem."; } diff --git a/compilerCommon/src/main/java/android/databinding/tool/processing/ScopedException.java b/compilerCommon/src/main/java/android/databinding/tool/processing/ScopedException.java index d800d72c..b5023778 100644 --- a/compilerCommon/src/main/java/android/databinding/tool/processing/ScopedException.java +++ b/compilerCommon/src/main/java/android/databinding/tool/processing/ScopedException.java @@ -20,6 +20,7 @@ package android.databinding.tool.processing; import android.databinding.tool.store.Location; import android.databinding.tool.util.L; import com.android.annotations.NonNullByDefault; +import com.android.annotations.Nullable; import com.google.common.base.Splitter; import com.google.common.base.Strings; import com.google.gson.Gson; @@ -79,8 +80,13 @@ public class ScopedException extends RuntimeException { } public ScopedException(String message, Object... args) { + this(null, message, args); + } + + public ScopedException(@Nullable Throwable cause, String message, Object... args) { super(message == null ? "unknown data binding exception" : - args.length == 0 ? message : String.format(message, args)); + args.length == 0 ? message : String.format(message, args), + cause); mScopedErrorReport = Scope.createReport(); mScopeLog = L.isDebugEnabled() ? Scope.produceScopeLog() : null; } diff --git a/compilerCommon/src/main/java/android/databinding/tool/util/L.java b/compilerCommon/src/main/java/android/databinding/tool/util/L.java index fd238c31..965860e0 100644 --- a/compilerCommon/src/main/java/android/databinding/tool/util/L.java +++ b/compilerCommon/src/main/java/android/databinding/tool/util/L.java @@ -95,7 +95,7 @@ public class L { throw ex; } } - ScopedException ex = new ScopedException(fullMessage); + ScopedException ex = new ScopedException(t, fullMessage); if (ex.isValid()) { throw ex; } |