[annotation reachability] fix bug on android annotation check

Summary:
Java method annotations are ambiguous in that there is no difference between
annotating the return value of a method, and annotating the method itself.
The disambiguation is done entirely based on the meaning of the annotation.

Here, while `UiThread`/`MainThread` are genuine method/class annotations
and not return annotations, the reverse is true for `ForUiThread`/`ForNonUiThread`.
This means that these latter annotations do not determine the thread status of
the method they are attached to.

Here we fix that misunderstanding.

Reviewed By: jvillard

Differential Revision: D17960994

fbshipit-source-id: 5aecfb124
master
Nikos Gorogiannis 5 years ago committed by Facebook Github Bot
parent 0a06353bce
commit a9c90368e8

@ -475,16 +475,14 @@ let annot_specs =
StandardAnnotationSpec.from_annotations str_src_annots str_snk_annot )
in
let open Annotations in
let cannot_call_ui_annots = [any_thread; for_non_ui_thread; worker_thread] in
let cannot_call_non_ui_annots = [any_thread; for_ui_thread; mainthread; ui_thread] in
let cannot_call_ui_annots = [any_thread; worker_thread] in
let cannot_call_non_ui_annots = [any_thread; mainthread; ui_thread] in
[ (Language.Clang, CxxAnnotationSpecs.from_config ())
; ( Language.Java
, ExpensiveAnnotationSpec.spec :: NoAllocationAnnotationSpec.spec
:: StandardAnnotationSpec.from_annotations cannot_call_ui_annots ui_thread
:: StandardAnnotationSpec.from_annotations cannot_call_ui_annots mainthread
:: StandardAnnotationSpec.from_annotations cannot_call_ui_annots for_ui_thread
:: StandardAnnotationSpec.from_annotations cannot_call_non_ui_annots worker_thread
:: StandardAnnotationSpec.from_annotations cannot_call_non_ui_annots for_non_ui_thread
:: user_defined_specs ) ]

@ -29,9 +29,9 @@ val performance_critical : string
val prop : string
val for_non_ui_thread : string
val for_non_ui_thread : string [@@warning "-32"]
val for_ui_thread : string
val for_ui_thread : string [@@warning "-32"]
val guarded_by : string

@ -19,13 +19,25 @@ import java.lang.annotation.Target;
@Retention(RetentionPolicy.CLASS)
@interface AnyThread {}
@Target({ElementType.METHOD})
@Retention(RetentionPolicy.CLASS)
@interface ForUiThread {}
/*
Sources:
@Target({ElementType.METHOD})
@Retention(RetentionPolicy.CLASS)
@interface ForNonUiThread {}
https://developer.android.com/reference/android/support/annotation/UiThread
"Denotes that the annotated method or constructor should only be called on the UI thread."
https://developer.android.com/reference/android/support/annotation/MainThread
"Denotes that the annotated method should only be called on the main thread."
"Note: Ordinarily, an app's main thread is also the UI thread."
(this is what's assumed here also)
https://developer.android.com/reference/android/support/annotation/WorkerThread
"Denotes that the annotated method should only be called on a worker thread."
https://developer.android.com/reference/android/support/annotation/AnyThread
"Denotes that the annotated method can be called from any thread (e.g. it is "thread safe".) [...]
static tools can then check that nothing you call from within this method or class have more
strict threading requirements."
*/
public class UiThreads {
@ -38,12 +50,6 @@ public class UiThreads {
@AnyThread
void anyThread() {}
@ForUiThread
void forUiThread() {}
@ForNonUiThread
void forNonUiThread() {}
@WorkerThread
void workerThread() {}
@ -52,11 +58,9 @@ public class UiThreads {
void callUiThreadMethod() {
uiThread();
mainThread();
forUiThread();
}
void callNonUiThreadMethod() {
forNonUiThread();
workerThread();
}
@ -84,30 +88,6 @@ public class UiThreads {
unannotated();
}
@ForUiThread
void callsFromForUiThreadBad() {
callNonUiThreadMethod();
}
@ForUiThread
void callsFromForUiThreadOk() {
callUiThreadMethod();
anyThread();
unannotated();
}
@ForNonUiThread
void callsFromNonUiThreadBad() {
callUiThreadMethod();
}
@ForNonUiThread
void callsFromNonUiThreadOk() {
callNonUiThreadMethod();
anyThread();
unannotated();
}
@WorkerThread
void callsFromWorkerThreadBad() {
callUiThreadMethod();

@ -27,18 +27,8 @@ codetoanalyze/java/annotreach/NoAllocationExample.java, codetoanalyze.java.check
codetoanalyze/java/annotreach/TwoCheckersExample.java, codetoanalyze.java.checkers.TwoCheckersExample.shouldRaisePerformanceCriticalError():java.util.List, 1, CHECKERS_CALLS_EXPENSIVE_METHOD, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromAnyThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromAnyThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromAnyThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromAnyThreadBad():void, 2, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromAnyThreadBad():void, 2, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromForUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromForUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromMainThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromMainThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromNonUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromNonUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromNonUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromUiThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromWorkerThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromWorkerThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []
codetoanalyze/java/annotreach/UiThreads.java, codetoanalyze.java.checkers.UiThreads.callsFromWorkerThreadBad():void, 1, CHECKERS_ANNOTATION_REACHABILITY_ERROR, no_bucket, ERROR, []

Loading…
Cancel
Save