Status Update
Comments
tn...@google.com <tn...@google.com> #2
Issue with notification access code in core. Unclear where, but probably safe to assume NotificationCompat
.
al...@google.com <al...@google.com> #3
Note that this is a lint error and can be suppressed or lowered to warning-level on the client side. Lowering back to P2
.
al...@google.com <al...@google.com> #4
androidx/platform/frameworks/support/+/androidx-main:buildSrc/public/src/main/kotlin/androidx/build/SupportConfig.kt;l=50 to 33 and then run ./gradlew :core:core:lintDebug
tn...@google.com <tn...@google.com> #5
It's at
frameworks/support/core/core/src/main/java/androidx/core/app/NotificationManagerCompat.java:223: Error: When targeting Android 13 or higher, posting a permission requires holding the POST_NOTIFICATIONS permission [NotificationPermission]
mNotificationManager.notify(tag, id, notification);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Explanation for issues of type "NotificationPermission":
When targeting Android 13 and higher, posting permissions requires holding
the runtime permission android.permission.POST_NOTIFICATIONS.
al...@google.com <al...@google.com> #6
Handling this as part of
al...@google.com <al...@google.com>
bu...@google.com <bu...@google.com> #7
Bugjuggler:
an...@google.com <an...@google.com> #8
Please help us out! If you are planning to work on this issue within the next year, please reply indicating the timeline you intend to complete the work and remove the "Android Blintz Close Candidate" hotlist. If it is expected for this issue to take multiple years to resolve, please add it to either the android_icebox (
Stale issues that aren't removed from the "Android Blintz Close Candidate" will be closed in 7 days, so if you think this should be closed there's no need to do anything. For more information please see go/android-blintz.
If you are not the assignee of this issue, and feel this issue needs to be escalated, please email the components default assignee with the issue number, and what you'd like to escalate.
ow...@google.com <ow...@google.com> #9
I believe we still want to do this.
al...@google.com <al...@google.com> #10
Pulling off BugJuggler so we can discuss this in weekly meeting.
al...@google.com <al...@google.com>
al...@google.com <al...@google.com> #11
Looking into whether this is really blocked on
al...@google.com <al...@google.com> #12
Detected these failing tasks: [':annotation:annotation:compileKotlinJvm']
> Task :annotation:annotation:compileKotlinJvm
e: file://$SUPPORT/annotation/annotation/src/jvmMain/kotlin/androidx/annotation/Px.kt:28:1 This annotation is not applicable to target 'annotation class'
e: file://$SUPPORT/annotation/annotation/src/jvmMain/kotlin/androidx/annotation/RequiresApi.kt:52:5 This annotation is not applicable to target 'value parameter'
e: file://$SUPPORT/annotation/annotation/src/jvmMain/kotlin/androidx/annotation/RequiresApi.kt:54:5 This annotation is not applicable to target 'value parameter'
e: file://$SUPPORT/annotation/annotation/src/jvmMain/kotlin/androidx/annotation/RequiresExtension.kt:64:5 This annotation is not applicable to target 'value parameter'
e: file://$SUPPORT/annotation/annotation/src/jvmMain/kotlin/androidx/annotation/RequiresExtension.kt:66:5 This annotation is not applicable to target 'value parameter'
And we're immediately hit by this ourselves.
public annotation class RequiresApi(
/**
* The API level to require. Alias for [.api] which allows you to leave out the `api=` part.
*/
@IntRange(from = 1) val value: Int = 1,
/** The API level to require */
@IntRange(from = 1) val api: Int = 1
)
The annotation needs to be moved next to Int
. This would be highly disruptive for clients.
Maybe we don't?
al...@google.com <al...@google.com> #13
Or, at the very least, maybe we add TYPE_USE
but don't remove the targets?
tn...@google.com <tn...@google.com> #14
Please don't add type use to these annotations without also updating the lint analysis to support/check on types.
al...@google.com <al...@google.com> #15
without also updating the lint analysis to support/check on types.
Lint team would need to do that, but I'm happy to provide motivation. We have a parallel bug for Metalava that my team would be handling.
al...@google.com <al...@google.com> #16
Or, at the very least, maybe we add
TYPE_USE
but don't remove the targets?
We could consider adding TYPE_USE
, implementing a lint check with auto-fix to migrate, and effectively "deprecating" the other targets. It would still be extremely disruptive, though. And we'd be on the hook for migrating google3, which would be somewhere between difficult and impossible.
tn...@google.com <tn...@google.com> #17
Lint team would need to do that, but I'm happy to provide motivation. We have a parallel bug for Metalava that my team would be handling.
Right -- just making it clear that this isn't currently in the plans from the lint team, so let's not update the public annotations with type use and then leaving tools support as TODO, since the primary purpose of these annotations is enforcement.
al...@google.com <al...@google.com> #18
Putting this on the backburner again. There are still some big questions around whether this is a good idea given the trade-offs around (1) disrupting clients and (2) ownership in g3.
bu...@google.com <bu...@google.com> #19
Bugjuggler:
al...@google.com <al...@google.com> #20
We have an owner and time scheduled for the blocking issue, so this is likely to make progress in Q3.
Right -- just making it clear that this isn't currently in the plans from the lint team, so let's not update the public annotations with type use and then leaving tools support as TODO, since the primary purpose of these annotations is enforcement.
Unclear what we'll do about lint, though. Tor, I'm finding a couple of bugs filed against Lint public component that touch on type annotations but nothing that's obviously "implement support for TYPE
." Is there an existing bug that I can use as blocking, or do you want a new one?
bu...@google.com <bu...@google.com>
ga...@linecorp.com <ga...@linecorp.com> #21
Related with
b/209843426 https://youtrack.jetbrains.com/issue/KT-48900 https://youtrack.jetbrains.com/issue/KT-67981
Since Kotlin 1.6, kotlin compiler warns when use androidx.annotation in Container types. Also, Since kotlin 2.0, kotlin compiler warning cannot be suppressed and it looks that Kotlin compiler team will not provide workaround to suppress.
Description
When we have
@Nullable
and@UiContext
conceptually refer to theContext
, the return type, while@override
and@MainThread
refer to the function.As far as I know, this is because pre-Java-8,
AnnotationTarget.TYPE
did not exist. I believe the most correct way to have that code in the post-Java-8-world isProposal: migrate all (androidx) annotations that conceptually refer to types away from
AnnotationTarget.Method
(/Getter
?) toAnnotationTarget.TYPE
.Because:
@TypeBound
annotation added to Kotlin, which would allowfun foo(c: @UiContext Context)
to throw a compile-time error when passed a non-@UiContext
context
, ifannotation class UiContext
was@TypeBound
. This could be useful for@*Context
and@*Res
, among others. Not everything is Kotlin yet, but we should be forward-looking.The annotations I believe this applies to:
KNOWN_TYPEBOUND_ANNOTATION_NAMES = "Dimension", "Px", "Size"
KNOWN_TYPEBOUND_ANNOTATION_SUFFIXES = listOf("Res", "Range", "Long", "Int", "Float", "Context")
Including nullability annotations, this comes out to 35 androidx annotations at the current time.
Plan:
AnnotationTarget.TYPE
.@Deprecate
ing certainAnnotationTargets
and deprecateAnnotationTarget.METHOD
for the applicable androidx annotations.androidx.annotation
, remove the deprecatedAnnotationTarget.METHOD
.@TypeBound
is accepted, consider how it could be made use of.Caveat: this is assuming the minimum java version we support is Java8. I believe this is the case, or will be by the time we reach step 5 (and that lower versions are deprecated by step 3). Do we have an official policy on this topic?