Age | Commit message (Collapse) | Author |
|
This CL fixes a couple of issues where data binding would generate
wrong code by thinking an included layout also has data binding
even when it only has view binding.
To make the change least destructive, I've changed generated binding
Impl to find view binding views via regular ids (as if they are views).
After finding them, they are bound using GeneratedBindingClass.bind
before passing it to the super class.
Bug: 150397979
Test: ViewBindingWithDataBindingTestApp
Change-Id: I445ce555a2629571447577f58d3d6ce075600fad
|
|
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
|
|
This CL reverts a change from 3.6 which disabled calling Observable#get
in expressions. Unfortunately, two way binding generates similar expressions
just to be executed directly so disabling these might break nested observable
bindings.
IDE still shows an error and it should since this is an error if user does it
but generated code is fine. We could potentially fix this by allowing just the
generated code to do it but it would be a much larger change that i'm not super
comfortable with.
Bug: 147249308
Test: ObservableGetDetectionTest TwoWayBindingAdapterTest
Change-Id: I592bb6404992df6ab3ada34851c59028a6bae994
|
|
This is unnecessary and also causes an infinite loop in the compiler.
It is flat out wrong so it is much better to just detect and print a
proper error.
Bug: 144604674
Test: ObservableGetDetectionTest
Change-Id: I5ca8acaa0696b91aa9806f831e4661bb7c2442ac
|
|
Fixes: 123427765
Test: AppCompatResourcesTest
Change-Id: I66d32c42fb22aa42ebfc26069b68123fba67e448
|
|
This CL fixes a weird bug where if there is a && or || opeartion on
a nullable expression, we could get an NPE.
This avoids it by enforcing such expressions to resolve to
boolean rather than Boolean so that lest of the safeUnboxing
logic kicks in to ensure we reach to a primitive value at the end.
Bug: 144246528
Test: SafeUnboxingTest
Change-Id: Iabbbc7517f0bcb0bb8b32d9dc73b1b153d468583
|
|
This CL fixes an issue in AnnotationModel where it would downcast
java.lang.Void to void, causing issues when Void is used as type
parameter.
Not sure why we were doing that but tests are passing.
Bug: 142405164
Test: AnnotationAnalyzerTest
Change-Id: I7e412cf312de5421a2bbd9c8c3a8df7766f9b34a
|
|
This CL fixes a bug in data binding where in some places,
capitalization did not specify a locale, which created
problems on windows machines that are set to Turkish since
turkish i is still an i when capiltalized (not an I).
Surprisingly, it works fine on linux machines while fails
to compile on windows machines but i was able to create a
test case which broke data bindng when two way binding is used.
This CL is a bit risky if some turkish user depended on this
behavior on a linux machine and accessed those fields in
their code but i think it is better to fix this way since that
is what kotlin does as well.
https://github.com/JetBrains/kotlin/blob/64e85e8a0c7b0c0392296bcd8b3dd9335bbf1ef4/core/descriptors/src/org/jetbrains/kotlin/name/NameUtils.kt
Bug: 141925966
Bug: 118943890
Test: NonEnglishLocaleTest in compilation tests
Change-Id: If879c52639530431a0710537136fc934f421b489
|
|
This CL fixes two bugs in data binding both related to recursive
data structures.
If you provide data binding a class that looks like Foo<T : Foo> or
Foo : LiveData<T : Foo>, it would go into a stackoverflow or OOM
trying to parse it since it would land back into the same class
as it tries to resolve values.
This CL fixes it by adding a tracker into such code and bails out
with whatever information it can.
For cases where the class in question is observable
(e.g. class Foo : LiveData<Foo>), we crash with a message since
data binding will try to create foo.getValue().getValue()...
For cases where it is a normal class (which we won't try to unwrap),
we work as desired.
There is possibly more of these but i can only reproduce these 3
cases so didn't want to overzealously add more coverage without
having a repro case.
Bug: 141633235
Bug: 140999936
Test: RecursiveLayoutTest (TestApp), RecursiveObservableTest
(compileation test)
Change-Id: I9977a1c8ae7c726b924550783d48049e89ca227c
|
|
This CL fixes a regression added in 3.5.1 / 3.6 where if a generic
field with unspecified type is accessed in the expressions, we would
not put its type properly. This only happens if we want to declare a
variable with that undefined VariableType.
Bug: 142130560
Test: AnnotationModelTest, GenericInterfaceTest
Change-Id: I04eccab89aa30d7f77a6655c3207541afdf5f82a
|
|
Bug: 128314294
Test: compile
Change-Id: Ia79e10bdf9db5e7c9cf4439d93d565997fe5059d
|
|
This CL fixes a bug where if there is a class declareted as Foo<T>
but the XML declarates it as Foo (without type args), we would
fail to resolve any type inside it.
e.g.
class Foo<T> {
List<String> getList()
}
would resolve to getList returning just a List.
This CL fixes it by separating how we turn a class into code based
on context (whether as a variable declaration or regular code, e.g.
to access static fields)
It is not super clean but there is a lot of complicated scenerios
so I tried to fix it w/o breaking anything else or changing too
much logic.
Also added infra to compiler tests to be able to create tests that
run in the processing environment to make future debugging easier.
We should ultimately move all JavaModel tests into that and get
rid of JavaModel implementation (if not used by studio).
Bug: 139738910
Test: AnnotationModelTest, GenericInterfaceTest, GenericAdapterTest
Change-Id: I2ea4ad74596fd6145f8dd4665dd550fd6e36f0d6
|
|
This CL fixes a missing feature/bug where if an <include tag resolves to a Binding
class, it would not support any binding adapters but worked perfectly if included
layout is not a binding layout.
This CL moves it to use a common architecture where if a binding target is a view
binding, we'll try to unroll it to its getRoot method if a setter cannot be found
in the binding class.
Bug: 133390436
Test: IncludeTagTest.java
Change-Id: I26e5b458303a185f1f3fa42c3e4fd0790d601dd7
|
|
This CL fixes an incrementality problem where we could've generated different BR
classes w/o any actual code change on the developer side.
I've moved usages of BR to a separate class to ensure ordering as multiple
things access it ~yay v1 compat~. This is still not perfect for caching because the
BR data uses a java serializable class w/ a regular map. We need to move it to
json as well to ensure caching is consistent too (thought this change should help
with incremental javac)
Bug: 131659806
Test: existing tests
Change-Id: I3412845f121798bb474fb503fe2bfdfa93034da1
|
|
When the build flag is set, parse all layouts instead of just ones with a root layout tag. We track whether a layout intends to bind data or not and then partition the set of all layouts when doing code generation.
Bug: 128314294
Test: existing databinding
Change-Id: I514dff76d9ba4ce61d5634765f971d3f467e79a5
|
|
Data Binding was only verifying view ids between multi-config layouts because
lint already warns for duplicate ids in views. On the other hand, not detecting
this error during layout processing causes compilation to continue and fail with
an unrelated error later on.
This CL fixes that issue and detects the problem during base layout generation.
I've also changed base layout generator to not to eat errors. I'll make the
necessary changes on the gradle side to report them with the right encoding.
Bug: 119645762
Test: SimpleCompilationTest
Change-Id: Ib0939e9c160ada5337fb6703205d3140f446bede
|
|
DeclaredFields are supposed not to include fields from
superclass. After investigating their implementation and
usage, they actually include those fields from superclass
and should be renamed to "allFields". Same for
declaredMethods.
Bug: 127974740
Test: JavaClassTest
Change-Id: Ic471238db40b71ec8f0c7ca141e1a95e0541654b
|
|
The documentation lists the parameter as nullable but we are
reading it w/o a null. This guards the error reporting behind
a null check to avoid the NPE.
Bug: 119645762
Test:couldn't figure out what triggers it but issue is obvious via the
docs, added annotation and sending the CL to unblock google3
Change-Id: I34575aba9bccd8440aefc7f8916fee305a1e4fa6
|
|
Bug: N/A
Test: N/A, only test changed
Change-Id: I18b33f5574f7c1eb365bbdeb8c8c146a49657a44
|
|
Currently, data binding writes a layout info file that contains the
absolute path to the original layout file. This is breaking Gradle
caching across machines.
To fix this, we need to write a relative path instead, which will be
relative to the current project directory.
Bug: 121288180
Test: Updated SimpleCompilationTest, MultiLayoutVerificationTest
New RelativizableFileTest
Change-Id: I39e6bacd4c553edef579cf3afb0ba17968dc99bc
|
|
Normally, a field that's marked with @Bindable has an
associated BR constant created for it, e.g.
@Bindable
public int mDemo;
will create an associated
class BR {
...
demo = 234;
...
}
constant.
However, variables defined in layout XMLs will also generate
a BR constant. That is,
<variable name="demo2" ... />
will similarly generate a 'BR.demo2' constant.
However, trouble happens because XML variables also generate
@Binding fields, although their name may get transformed to
follow standard Java camel-case naming conventions.
So, this:
<variable name="snake_case_var" ... />
will generate this:
class BR {
...
snake_case_var = 123;
...
}
as well as this:
@Generated({"Android Data Binding"})
class DemoBinding {
@Bindable
mSnakeCaseVar;
}
which in turn generates this:
class BR {
...
snakeCaseVar = 567;
...
}
This latter constant is particularly problematic because it
doesn't actually get used in code generated by data binding.
In practice, this case should be pretty rare. As such, we
fix the bug by skipping over generated classes when searching
for @Bindable annotations from which to generate BR constants.
Change-Id: I06c6fba165809c42701955ff6485517ba0014ed6
Fixes: 124076237
Test: Covered by existing tests
|
|
Due to a bug in the code generation logic, generic variables
that don't have their type specified cause compile errors.
e.g. this works fine
<variable type="java.util.List<Integer>" ... />
but this was broken:
<variable type="java.util.List" />
Change-Id: I7ef8f23ff01cace11f7613c5fc926b9a100fdd5f
Fixes: 123409929
Test: Integration tests updated
|
|
See CL 1/3 for full context.
This CL adds a META-INF file indicating that data binding is a
"dynamic" incremental annotation processor ("dynamic" means that its
actual type will be determined later at run time).
At run time, if the android.databinding.incremental feature flag is
true, data binding will register with Gradle that it is an "aggregating"
annotation processor. Otherwise, it will remain a non-incremental
annotation processor.
Test: See CL 1/3
Bug: 110061530
Bug: 117725060
Change-Id: If17d83d9c3dd56d743b53808a872e0937fec9879
|
|
This CL adds a new argument to the compiler args that has the list
of packages that are directly accessible. Normally, data binding
discovers this automatically from avaiable classes but blaze/bazel
makes transitive dependencies available, which makes data binding
access them and then break strict deps rules.
This new flag is not used by gradle.
I've also updated the google3 push script to partial jetify classes
since Google3 is jetified in pieces.
Bug: 119645762
Test: google3
Change-Id: I0f2d330671b9b046f591a29a6cae0b7b5e4a6096
|
|
This CL updates data binding so that it can support incremental
annotation processing in the next CL:
1. Add missing supported annotation types to ProcessDataBinding as
Gradle needs to know about all supported annotation types.
2. Remove RetentionPolicy.SOURCE in BindingBuildInfo as Gradle needs
to see the annotations in the *class files*.
Also remove unused buildId() method in BindingBuildInfo.
Test: Existing tests
Bug: 110061530
Bug: 117725060
Change-Id: I0b4255b556a72ad8725a39b95b5061d38f11d3bd
|
|
Also, update how databinding errors are encoded, using
JSON instead of our custom format. This both reduces
boilerplate parsing code, and also helps as error
messages now contain newlines in them, which was causing
issues with the previous logic.
Change-Id: Idcf6c984187121e7759b0c1b86a8024edb7f6679
Fixes: 120626727
Fixes: 62685775
Bug: 119440634
Test: Mostly covered by existing tests; some tests
updated to use ErrorMessage constants; SafeUnboxingTest
removed as the warning is no longer shown
|
|
This CL fixes a bug in FIeldAccessExpr's two way resolution logic.
FieldAccessExpr is the last stop in two way resolution as it is
either targeted to a view (check for two way) or not (nothing to do).
But this breaks in cases where it might have nested access levels.
e.g. textView.text.length has a nested FieldAccessExpr which is
two way (textView.text) but the target for the parent is textView.text
which is a FieldAccessExpression.
Previous logic was stopping resolution when it is not ViewAccess but
it should've continued to call the parent to check nested items.
Bug: 116536168
Test: TwoWayBetweenViewsTest
Change-Id: I74e9c62ee21fc06f6128c6baaebde21496c495b9
|
|
This CL changes some compilation to be compatible with blaze,
updates the google3 push script and adds more logging.
Bug: 112038432
Test: existing tests and g3 taps
Change-Id: I58737cf8c7e6daa6d131cd35cf72d2cff7d3d7a9
|
|
Test: fixed tests on Windows.
Bug: N/A
Change-Id: I2b9f4e86f50361475ec5281fa4bfbf659ac4264a
|
|
We spend a lot of time re-finding same classes and then querying their
properties. This CL introduces a new class finder cache that holds
onto already found classes keyed by its name and available imports.
The hit rate for TestApp is quite crazy (97%) and brings down no-cache
compilation from 3 sec to ~ 2.5 sec.
Bug: 117808327
Test: ClassFinderCacheTest
Change-Id: I1981627588fe411b1970df99c722a91fe4b657b2
|
|
This avoids creating an unnecessary array.
Also moved Injected class to be kotlin as well.
Bug: 117808327
Test: existing tests
Change-Id: I481106b1089e5d1066ec0733683876926d519fad
|
|
Bug: 117808327
Test: existing tests pass
Change-Id: Iec0d01d9c0d76d8889794ee1fa3a69cbca115128
|
|
This is a preliminary cleanup step before adding more
caching to these classes.
Bug: 117808327
Test: n/a
Change-Id: I888d4506b5079b8904d59955c9006397b68f0368
|
|
This CL moves Map<String(alias), String(qname)> imports to a concrete
ImportBag class. It is mostly a cleanup so that in a follow-up CL,
we can start caching findClass calls based on available imports.
Right now it is not possible since imports are all bloated with
java.lang.
This CL also stops adding all java.lang.* as static identifiers to
every single model. Instead, it lazily registers it if an identifier
expression cannot be found and name matches a java.lang class.
Bug: 117808327
Test: ImportBagTest, existing tests pass
Change-Id: Ideb41ab8cfc68aa4c7b55e84ffa218ba65558eb3
|
|
This CL fixes a bug where if a module depends on a V1 library,
we would generate the V1 mapper for it which does not compile
because R files for V1 are not final at that stage.
We should actually not generate it and instead delegate the
application to do it.
We still generate the stub binding for those so that the code
can compile but we'll strip them as usual.
Bug: 117666264
Test: MultiModuleTestApp with LibCompiledWithV1 dependency
Change-Id: I9cc6b04573776141b021d0d516f10fc0213e8270
|
|
This CL fixes an issue in data binding where if a binding adapter
from a dependency was writting in android.support and app is
using androidX, we would not be able to use it.
Now, the annotation class finder automatically jetifies a
class if it cannot find it.
This CL also updates BindingAdapterStore to ignore all
android.databinding.adapter* artifacts which might lurk into
dependencies if the dependency was compiled with V1.
This CL also includes a small optimization in ModelAnalyzer
where we would always query a known class even if it is not
there (e.g. LiveData). This avoids such unnecssary queries.
Bug: 117266727
Test: MultiModuleTestApp
Change-Id: Ic8079160f8a5aa3c14c40f673a6339c090fa9f9a
|
|
This is a quick fix to null check class anayzer
results to avoid NPE while traversing adapters.
It is tricky to test since this only happens with
a dependency that was both compiled with android.support
and pre 3.2 version. This is why I've added LibCompiledWith3.1
to the list of integration tests which is compiled manually
and the final aar is put under MultiModuleTestApp/app/libs.
Bug: 116361870
Test: MultiModuleTestApp
Change-Id: Ibaf6174980de08d50b0ef1b9679db6cf99e1121a
|
|
An included layout might be coming from a dependency and if that
dependency is a feature (base feature), we need to use the correct
R.layout file. This will also be necessary with namespaced
resources.
Bug:112042862
Test: InstantApp integration test
Change-Id: Iaaf99ab7a9f7ac03757cdea19c8ddc111505d9ed
|
|
This CL fixes a bug in binding adapters where it
would crash compilation when there is a conflict in keys.
Now we check the data before failing and also print a
warning instead of an error since app might be overriding
dependencies.
Bug:37100286
Test: MultiModuleTestApp#GeneratedLayoutTest.testOverriddenAdapters
Change-Id: Idee39ed81bc4e52be39df1362fbed29700a4b8b7
|
|
This CL brings back support for old args to support google3 blaze which cannot
update in lock step (e.g. cannot move to new args w/o updating the library). So
in this CL, we make data binding args backwards compatible.
It also fixes the encoding for layout-info files which only affected google3.
Bug: 111400388
Change-Id: I32a6f7807892f938bcdc12d55b19ae38f6750342
Test: in google3
|
|
This CL changes the Intermediate format for SetterStore to use
sorted maps that are later serialized into JSON. This will allow
us to make future changes in the Intermediate format w/o breaking
compatibility.
To accomodate it, GenerationalClassUtil now supports JSON type.
I've also de-coupled the information storage into a BindingAdapterStore
class to make sure that SetterStore cannot dig into details of that
class.
This way, we can serialize just the adapter in this class, minimizing
unexpected invalidations.
Bug: 80553728
Bug: 110855984
Test: DataBindingMultiModuleTest.kt
Change-Id: I9800e075b3317228e7f96ca6dfc84f6f2e0cc22d
|
|
Fixes: 110198434
Bug: 69243050 (related by not fixed by this CL)
Test: Refactoring change - Should be covered by existing tests
Change-Id: I39534def1220cbda3950c39ce26e0d7eba2fab8b
|
|
This CL adds support for using "implementation" dependencies in
gradle, greatly reducing compilation times if app is setup
properly.
There are 2 big changes:
* SetterStore will not use an adapter if it is not available in
the compile classpath
* MergedDataBindingMapper will only initialize the App's mapper.
Each time a mapper is added, we ask it to provide it's dependencies.
This way, an app can initialize all of its dependencies w/o
directly calling them out.
Bug: 77539932
Test: MutliModuleTestApp#connectedCheck
Change-Id: I686b06180a062c071f1577cb782897ff6cce1199
|
|
When there is a binding adapter that receives a kotlin
function which does not return, Kotlin converts it to
Function0<kotlin.Unit>. But if there is a kotlin class
with a method that does not return value, kotlin converts
it to void.
This becomes a problem for data binding because it
cannot match the two when using function references.
We cannot simply convert kotlin.Unit to void because
we still generate code in java so we have to return
a value in the implementation.
Instead, this CL changes function matcher to accept
the two as equals and also changes the code generators
to handle the case manually.
Also added a kotlin test app (finally).
Bug: 70915745
Bug: 78662035
Test: KotlinTestApp
Change-Id: I019ee7eb1dd635b12efd7725ccc22f0c63dc2d72
|
|
This is the preparation step to make data binding work with task output
caching in a subsequent CL.
Test: N/A (no functional changes)
Bug: 69243050
Change-Id: Icff729cb87465d0a158771ed3ee84d731bc819d3
|
|
This CL adds integration tests w/ AndroidX and fixes some androidX
issues that i've discovered during the move.
Old tests are copied to integration-tests-support so that we still
run them.
I tried to keep the tests as similar as possible but sometimes needed
to move classes (e.g. for cases where we have package private access).
I've changed the applicationId of TestApp to use a different package
so that it wo't conflict w/ the other one while testing on the same
device. For other simpler tests, we were not using android.databinding
so i just changed their package names not to conflict w/ their twins.
I've also removed App With Spaces test which does not work w/ gradle5
anymore and we were not running it anyways.
Bug: 77166878
Test: new andoridX tests
Change-Id: I86f68fd6bee2ca849c6472c0411234bb3a08b132
|
|
This CL adds partial support for Android X.
We duplicate baseLibrary and extensions (runtime libs) to have copies
that are in the androidX namespace.
Gradle picks the right version to include based on shared androidX
flag. Right now, we have only 1 test that use androidX
(DataBindingIncrementalTest) so all other tests still use the old
package. I'll add testing for androidX in a followup CL.
Bug: 77166878
Test: existing tests pass
Change-Id: I77ec65b872cae0821513ca78ef9b6ab1b0300ed1
|
|
This CL adds backward compatibility into data binding v2 such that
it can consume dependencies that were compiled with v1.
We achieve this in 2 steps:
* Inside the gradle task, we deserialize v1 layout info and convert
it into GenClassLog. This allows gradle task to generate the right
code. (albeit, it might depend on not yet existing classes but
that is how v1 works anyways).
* Then in the annotation processor, we reload the same layout info,
create a CompilerChef that thinks it is in v1 and let it generate
the code.
Finally, we create 1 mapper for all v1 compat classes.
All code generated for v1 is stripped for libraries. This is
necessary because if 2 separate libraries in an app depend on the
same v1 lbirary, both of them will generate code for it, causing
duplicate class issues.
For apps compiled w/ v2 and has v2 dependencies, cost is almost
none (is just the gradle task has 1 more dependency).
For apps compiled w/ v1 and has v1 dependencies, it is as slow
as v1 is, + a bit more since we end up writing mode classes than
we would but it is negligible.
Bug: 74264651
Test: MultiModuleTestApp (gradle invokes it w/ v1 - v2 setup)
Change-Id: I0e04e7f04b67eb010d4a1bf32e0fce32586a4b90
|
|
From the early days of data binding,we have an easy way to add properties
to java objects w/o adding them the complexity of codegen.
It is causing problems now since they are static and kept foreever.
This CL just adds a hack to clean them up afterwards.
Bug: 68339615
Test: n/a
Change-Id: Ibf79d9cfb53dbc19f0ed590f492a0db60a3f5703
|
|
This CL adds a new build type called FEATURE, which is a new
version where we generate BR classes for only runtime dependencies
(dependencies that are not inherited from base or any other feature).
This new BR file generation logic looks up the existing ids
and re-uses if the BR id matches the id in a dependency.
For any other id, we generate them with an offset (provided by gradle)
so that it cannot conflict w/ other features.
The integration test app is not a real test app right now because
features do not support instrumentation tests. It is more of a
manual testing ground until features have the capability.
Bug: 63814741
Test: in the other CL
Change-Id: I6f3feda908b3925a29ff76a29cd3659cab3f8f8d
|