diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index f71a93dbd..35ad91ad0 100644 --- a/infer/src/checkers/annotations.ml +++ b/infer/src/checkers/annotations.ml @@ -67,6 +67,16 @@ let notnull = "NotNull" let not_thread_safe = "NotThreadSafe" +let on_bind = "OnBind" + +let on_event = "OnEvent" + +let on_mount = "OnMount" + +let on_unbind = "OnUnbind" + +let on_unmount = "OnUnmount" + let performance_critical = "PerformanceCritical" let prop = "Prop" @@ -111,10 +121,6 @@ let ma_has_annotation_with ({return; params} : Annot.Method.t) (predicate : Anno has_annot return || List.exists ~f:has_annot params -let get_annot_ending ({class_name} : Annot.t) = - String.rsplit2 class_name ~on:'.' |> Option.value_map ~default:class_name ~f:snd - - (** [annot_ends_with annot ann_name] returns true if the class name of [annot], without the package, is equal to [ann_name] *) let annot_ends_with ({class_name} : Annot.t) ann_name = @@ -232,3 +238,8 @@ let ia_is_suppress_lint ia = ia_ends_with ia suppress_lint let ia_is_thread_confined ia = ia_ends_with ia thread_confined let ia_is_worker_thread ia = ia_ends_with ia worker_thread + +(* methods annotated with the annotations below always run on the UI thread. *) +let ia_is_uithread_equivalent = + let annotations = [mainthread; ui_thread; on_bind; on_event; on_mount; on_unbind; on_unmount] in + fun ia -> List.exists annotations ~f:(ia_ends_with ia) diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index 6489069b2..e61c85d2b 100644 --- a/infer/src/checkers/annotations.mli +++ b/infer/src/checkers/annotations.mli @@ -51,9 +51,6 @@ val visibleForTesting : string val generated_graphql : string -val get_annot_ending : Annot.t -> string -(** get the '.'-last component of an annotation *) - val annot_ends_with : Annot.t -> string -> bool (** [annot_ends_with annot ann_name] returns true if the class name of [annot], without the package, is equal to [ann_name] *) @@ -115,6 +112,8 @@ val ia_is_volatile : Annot.Item.t -> bool val ia_is_worker_thread : Annot.Item.t -> bool +val ia_is_uithread_equivalent : Annot.Item.t -> bool + val pdesc_get_return_annot : Procdesc.t -> Annot.Item.t (** get the list of annotations on the return value of [pdesc] *) diff --git a/infer/src/concurrency/ConcurrencyModels.ml b/infer/src/concurrency/ConcurrencyModels.ml index 0f83b7c0f..5e95f1e22 100644 --- a/infer/src/concurrency/ConcurrencyModels.ml +++ b/infer/src/concurrency/ConcurrencyModels.ml @@ -376,22 +376,10 @@ let if_pred_evalopt ~pred ~f x = IOption.if_none_evalopt x ~f:(fun () -> if pred () then Some (f ()) else None) -(* assume that methods annotated with @MainThread, @UiThread, - and any string starting with "On" always run on the UI thread. - We do the latter because there are too many to precisely list. *) -let is_uithread annots = - let f (annot, _) = - let ending = Annotations.get_annot_ending annot in - String.equal ending Annotations.mainthread - || String.equal ending Annotations.ui_thread - || String.is_prefix ~prefix:"On" ending - in - List.exists annots ~f - - let mono_pname = MF.wrap_monospaced Typ.Procname.pp let runs_on_ui_thread ~attrs_of_pname tenv proc_desc = + let is_uithread = Annotations.ia_is_uithread_equivalent in let pname = Procdesc.get_proc_name proc_desc in if is_method_or_override_annotated ~attrs_of_pname Annotations.ia_is_worker_thread pname tenv diff --git a/infer/tests/codetoanalyze/java/racerd/Annotations.java b/infer/tests/codetoanalyze/java/racerd/Annotations.java index 6eda90250..783f14180 100644 --- a/infer/tests/codetoanalyze/java/racerd/Annotations.java +++ b/infer/tests/codetoanalyze/java/racerd/Annotations.java @@ -440,7 +440,7 @@ class ExtendsClassOnUiThread extends AllMethodsOnUiThread { } } -// All annotations that start with "On" are assumed to be on the main thread +// NOT All annotations that start with "On" are on the main thread @Target(ElementType.METHOD) @Retention(RetentionPolicy.CLASS) @interface OnXYZ {} @@ -450,7 +450,7 @@ class WeirdAnnotation { int f; @OnXYZ - void fooOk() { + void fooBad() { f = 0; } } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index b96f2990d..15eea1d8e 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -12,6 +12,7 @@ codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.Annotati codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.Annotations.read_off_UI_thread_Bad():void, 190, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.f`,,access to `this.f`] codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.ThreadSafeAlias.threadSafeAliasBad1():void, 82, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`] codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.ThreadSafeAlias.threadSafeAliasBad2():void, 87, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`] +codetoanalyze/java/racerd/Annotations.java, codetoanalyze.java.checkers.WeirdAnnotation.fooBad():void, 454, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.f`] codetoanalyze/java/racerd/Arrays.java, Arrays.arrayParameterWriteBad(int[]):void, 26, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `name1.[_]`] codetoanalyze/java/racerd/Arrays.java, Arrays.readWriteRaceBad(java.lang.String):java.lang.String, 47, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.strArr1.[_]`,,access to `this.strArr1.[_]`] codetoanalyze/java/racerd/Arrays.java, Arrays.writeWriteRaceBad(java.lang.String):void, 39, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.strArr1.[_]`]