Status Update
Comments
gg...@google.com <gg...@google.com> #2
When we fix nullability in Jetpack libraries, it is very painful for clients. Also for us attempting to land in g3.
I think this is a good idea; however, I'm not sure if we'd run into any issues since these are currently synthetic annotations generated by Metalava. It's possible that creating real annotations might conflict with the SDK stubs.
[Deleted User] <[Deleted User]> #3
If we do this we need to involve the Kotlin team; this basically introduces an "unenforced nullability warnings" level; they added the hardcoded qualified names for those two classes into the Kotlin compiler. intended for the narrow use case it's there for now; if this were to be broadened into something bigger they should be involved. +jvg
[Deleted User] <[Deleted User]> #4
If we use the exact same class names in the androidx.annotation
package for the exact same purpose, would we expect to need any work on the Kotlin compiler side?
gg...@google.com <gg...@google.com> #5
No, but so far those annotations have been hidden from view (even Android Studio tries hide to hide them, so the AndroidOverrideAnnotationsHandler for example rewrites signatures such that when you override a platform with some of these, you don't see them), and the annotations themselves have package private access so nobody can access them directly. This includes platform code; they cannot directly declare themselves to have the special warning nullcontract; it's only a build time transformation that exposes this. When you said you wanted to expose these annotations for usage by libraries, it sounds like you can now have a situation where libraries start using a special type of nullness annotation which doesn't have the normal guarantees and enforcement normally required by Kotlin. It's true, the Android platform APIs have some special handling for that; I'm just saying that if this is going to be broadened out into more widespread usage, that's something I think the Kotlin team should get to weigh in on, because the current scheme (where they hardcoded our two well known annotation names into the compiler) was agreed upon with a very specific narrow use case.
cc...@google.com <cc...@google.com> #6
Do you think it's worth the effort?
This is coming up as the side-effect of a recent Lint Fixit event where we added nullability annotations to a couple hundred APIs. It's a Good Thing (tm) but the fall-out in Kotlin sources is pretty bad, especially for override signature mismatches. There are enough failures in google3 that we've decided to revert the nullability changes for entire library groups.
Maybe there is a better way to address this than using @Recently
to give developers a heads-up? Something we can automate?
sa...@google.com <sa...@google.com> #7
It seems hard to automate. I think helping with library upgrades would be very valuable. We've done a lot of work to help upgrading AGP itself, but obviously, upgrading libraries is where the real pain is. We've started work on the targetSdkVersion migration assistant, which will help update you from one targetSdkVersion to the next. We could generalize this for libraries too. And making nullness API migration could be one of the things we built into that, where we would simply adjust all the overridden method signatures types to match those inherited from androidx. We don't even need to track which ones have changed recently; if your signature isn't compatible, we know it's from an unlucky earlier guess. Obviously, if your code then goes ahead and does something invalid (e.g. if we switch a parameter from non null to nullable, then your code will no longer compile since you have to add null checks, but that can't be helped (and is probably a good thing).
ai...@gmail.com <ai...@gmail.com> #8
And making nullness API migration could be one of the things we built into that, where we would simply adjust all the overridden method signatures types to match those inherited from androidx.
Yes, please. Do you want to use this issue to capture that FR, an existing issue, or should I file a new one?
cc...@google.com <cc...@google.com>
cc...@google.com <cc...@google.com>
vi...@google.com <vi...@google.com>
gm...@gmail.com <gm...@gmail.com> #9
Branch: androidx-main
commit 9160b30f71054706f0a61c1d5d69b99f8a15f5a0
Author: Ryan Mentley <ryanmentley@google.com>
Date: Fri Jul 29 21:46:52 2022
Remove nullability annotations added in 1.3.0-beta01
These were a source-incompatible change in Kotlin, which caused a lot of
pain for clients attempting to update to 1.3-0-beta01. Since we do not
currently have tooling for a proper deprecate/remove cycle
(see
annotations until the ability to do so exists.
annotations back once that is done.
Relnote: "Remove nullability annotations added in 1.3.0-beta01"
Bug: 238355930
Test: API change, validating via presubmit and post-merge drop to google3
Change-Id: I7a25874d17e40de21c6bd2f50192c79746d6b7e5
M recyclerview/recyclerview/api/public_plus_experimental_1.3.0-beta02.txt
M recyclerview/recyclerview/api/restricted_current.ignore
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/ItemTouchHelper.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/SortedList.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/SortedListAdapterCallback.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/DividerItemDecoration.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/SnapHelper.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/BatchingListUpdateCallback.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/AdapterListUpdateCallback.java
M recyclerview/recyclerview/api/public_plus_experimental_current.txt
M recyclerview/recyclerview/api/restricted_current.txt
M recyclerview/recyclerview/api/1.3.0-beta02.txt
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/DefaultItemAnimator.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/RecyclerView.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/SimpleItemAnimator.java
M recyclerview/recyclerview/api/current.ignore
M recyclerview/recyclerview/api/current.txt
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/ThreadUtil.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/ItemTouchUIUtil.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/LinearLayoutManager.java
M recyclerview/recyclerview/api/restricted_1.3.0-beta02.txt
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/LinearSmoothScroller.java
M recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/PagerSnapHelper.java
mc...@gmail.com <mc...@gmail.com> #10
Hello! FYI, automating the migration of removed/deprecate symbols in Kotlin code without breaking anything, is something that I personally worked on for the Gradle plugin refreshVersions, and it's something I'd love to see arrive for Kotlin and Android libraries.
I'd be very happy to help in making that happen, so please let me know when you're ready to brainstorm that, I'd love to contribute in the ideas and show what we've done in refreshVersions for this very similar problem.
Louis CAD
lo...@gmail.com <lo...@gmail.com> #11
Wrapping it is best, especially since Int
is autoboxed here anyway.
rh...@gmail.com <rh...@gmail.com> #12
hi...@gmail.com <hi...@gmail.com> #13
This is something the company I work with (with thousands of devs) would also like to see implemented.
I also believe this should be a simple win without much effort, unless there is something I am missing here. The only thing I see that could potentially complicate this change is the linter rule that warns about incompatibility of types, if that is not an issue, it would really take a few minutes to be implemented and there should be no side effects or down sides, as stated above.
Please help us get this prioritized
tn...@google.com <tn...@google.com> #14
This is not a simple one line fix. Yes, updating the annotation is a one line fix, but the main purpose for the annotation is having associated lint checking, and making lint support type use annotations is quite a bit of work. The way type use annotations are modeled is quite different from normal annotations.
ai...@gmail.com <ai...@gmail.com> #15
It's true, but annotation is helpful even for developers too, of course, it can be replaced with simple comment, but it's more mental load to read than existing annotation with clear semantics I would also prefer to annotate now and hope that it will work with lint in the future, rather than left all my existing code with annotation and review all places to add annotation when it will be fixed
Description
typealias StringProvider = (@StringRes Int) -> String
Maybe adding the same element type to other support annotations will be useful as well.