Add SuppressViewNullability annotation

Summary:This pull request adds the SuppressViewNullability annotation.

The reasoning behind this is that in libraries, one cannot use Butterknife for view binding, which forces you to do it manually. Basically, this makes a new annotation that infer treats the same way as Bind/InjectView
Closes https://github.com/facebook/infer/pull/301

Reviewed By: jvillard

Differential Revision: D3047235

Pulled By: cristianoc

fb-gh-sync-id: 6286d2b
shipit-source-id: 6286d2b

infer/annotations/com/facebook/infer/annotation/SuppressViewNullability.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2016 - present Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package com.facebook.infer.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * View can be annotated with @SuppressViewNullability to silence warnings when
+ * a view is set to null in a destructor, and created in an initializer.
+ */
+@Retention(RetentionPolicy.CLASS)
+@Target(ElementType.FIELD)
+public @interface SuppressViewNullability {}

infer/src/checkers/annotations.ml

@@ -98,6 +98,11 @@ let initializer_ = "Initializer"
 let inject = "Inject"
 let inject_view = "InjectView"
 let bind = "Bind"
+let bind_array = "BindArray"
+let bind_bitmap = "BindBitmap"
+let bind_drawable = "BindDrawable"
+let bind_string = "BindString"
+let suppress_view_nullability = "SuppressViewNullability"
 let false_on_null = "FalseOnNull"
 let mutable_ = "Mutable"
 let nullable = "Nullable"
@@ -134,13 +139,35 @@ let ia_is_true_on_null ia =
 let ia_is_initializer ia =
   ia_ends_with ia initializer_
 
-let ia_is_inject ia =
+let field_injector_readwrite_list =
+  [
+    inject_view;
+    bind;
+    bind_array;
+    bind_bitmap;
+    bind_drawable;
+    bind_string;
+    suppress_view_nullability;
+  ]
+
+let field_injector_readonly_list =
+  inject
+  ::
+  field_injector_readwrite_list
+
+(** Annotations for readonly injectors.
+    The injector framework initializes the field but does not write null into it. *)
+let ia_is_field_injector_readonly ia =
   IList.exists
     (ia_ends_with ia)
-    [inject; inject_view; bind]
+    field_injector_readonly_list
 
-let ia_is_inject_view ia =
-  ia_ends_with ia inject_view
+(** Annotations for read-write injectors.
+    The injector framework initializes the field and can write null into it. *)
+let ia_is_field_injector_readwrite ia =
+  IList.exists
+    (ia_ends_with ia)
+    field_injector_readwrite_list
 
 let ia_is_mutable ia =
   ia_ends_with ia mutable_

infer/src/checkers/annotations.mli

@@ -64,8 +64,15 @@ val ia_get_strict : Sil.item_annotation -> Sil.annotation option
 
 val ia_is_false_on_null : Sil.item_annotation -> bool
 val ia_is_initializer : Sil.item_annotation -> bool
-val ia_is_inject : Sil.item_annotation -> bool
-val ia_is_inject_view : Sil.item_annotation -> bool
+
+(** Annotations for readonly injectors.
+    The injector framework initializes the field but does not write null into it. *)
+val ia_is_field_injector_readonly : Sil.item_annotation -> bool
+
+(** Annotations for read-write injectors.
+    The injector framework initializes the field and can write null into it. *)
+val ia_is_field_injector_readwrite : Sil.item_annotation -> bool
+
 val ia_is_mutable : Sil.item_annotation -> bool
 val ia_is_nonnull : Sil.item_annotation -> bool
 val ia_is_nullable : Sil.item_annotation -> bool

infer/src/eradicate/eradicateChecks.ml

@@ -194,14 +194,16 @@ let check_field_assignment
     typecheck_expr node instr_ref curr_pname typestate exp_rhs
       (typ, TypeAnnotation.const Annotations.Nullable false TypeOrigin.ONone, [loc]) loc in
   let should_report_nullable =
-    let field_is_inject_view () = match t_ia_opt with
-      | Some (_, ia) -> Annotations.ia_is_inject_view ia
-      | _ -> false in
+    let field_is_field_injector_readwrite () = match t_ia_opt with
+      | Some (_, ia) ->
+          Annotations.ia_is_field_injector_readwrite ia
+      | _ ->
+          false in
     TypeAnnotation.get_value Annotations.Nullable ta_lhs = false &&
     TypeAnnotation.get_value Annotations.Nullable ta_rhs = true &&
     PatternMatch.type_is_class t_lhs &&
     not (Ident.java_fieldname_is_outer_instance fname) &&
-    not (field_is_inject_view ()) in
+    not (field_is_field_injector_readwrite ()) in
   let should_report_absent =
     activate_optional_present &&
     TypeAnnotation.get_value Annotations.Present ta_lhs = true &&
@@ -259,7 +261,8 @@ let check_constructor_initialization
             | Some (_, ia) -> f ia in
           let nullable_annotated = annotated_with Annotations.ia_is_nullable in
           let nonnull_annotated = annotated_with Annotations.ia_is_nonnull in
-          let inject_annotated = annotated_with Annotations.ia_is_inject in
+          let injector_readonly_annotated =
+            annotated_with Annotations.ia_is_field_injector_readonly in
 
           let final_type_annotation_with unknown list f =
             let filter_range_opt = function
@@ -285,18 +288,18 @@ let check_constructor_initialization
               (Lazy.force final_constructor_typestates)
               (fun ta -> TypeAnnotation.get_value Annotations.Nullable ta = true) in
 
-          let should_check_field =
+          let should_check_field_initialization =
             let in_current_class =
               let fld_cname = Ident.java_fieldname_get_class fn in
               match struct_name with
               | None -> false
               | Some name -> Mangled.equal name (Mangled.from_string fld_cname) in
-            not inject_annotated &&
+            not injector_readonly_annotated &&
             PatternMatch.type_is_class ft &&
             in_current_class &&
             not (Ident.java_fieldname_is_outer_instance fn) in
 
-          if should_check_field then
+          if should_check_field_initialization then
             begin
               if Models.Inference.enabled then Models.Inference.field_add_nullable_annotation fn;
 

infer/tests/codetoanalyze/java/eradicate/FieldNotInitialized.java

@@ -15,6 +15,8 @@ import android.widget.EditText;
 
 import butterknife.Bind;
 
+import com.facebook.infer.annotation.SuppressViewNullability;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -31,9 +33,15 @@ public class FieldNotInitialized {
 
   @NonNull String e;
 
-  @Bind(42) EditText f;
+  @Bind(42) EditText f; // Means: assume it will be initialized, and ignore null assignment
+
+  @SuppressViewNullability EditText g;
 
   //  Eradicate should only report one initialization error
   FieldNotInitialized() {}
 
+  void testNullifyFields() {
+    f = null; // OK  the framework could write null into the field
+    g = null; // OK  the framework could write null into the field
+  }
 }

infer/tests/endtoend/java/eradicate/FieldNotInitializedTest.java

@@ -10,6 +10,7 @@
 package endtoend.java.eradicate;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static utils.matchers.ResultContainsExactly.containsExactly;
 import static utils.matchers.ResultContainsNumberOfErrorsInMethod.containsNumberOfErrors;
 
 import org.junit.BeforeClass;
@@ -46,7 +47,20 @@ public class FieldNotInitializedTest {
             FIELD_NOT_INITIALIZED,
             SOURCE_FILE,
             "<init>",
-            1));
+            1
+        )
+    );
+
+    String[] procedures = {"<init>"};
+    assertThat(
+        "Results should contain " + FIELD_NOT_INITIALIZED,
+        inferResults,
+        containsExactly(
+            FIELD_NOT_INITIALIZED,
+            SOURCE_FILE,
+            procedures
+        )
+    );
   }
 
 }