From 1c819770e20aa88c23b6eb124e113f98b07dfe90 Mon Sep 17 00:00:00 2001 From: Nick Firmani Date: Mon, 14 Mar 2016 08:52:13 -0700 Subject: [PATCH] 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 --- .../annotation/SuppressViewNullability.java | 23 ++++++++++++ infer/src/checkers/annotations.ml | 35 ++++++++++++++++--- infer/src/checkers/annotations.mli | 11 ++++-- infer/src/eradicate/eradicateChecks.ml | 19 +++++----- .../java/eradicate/FieldNotInitialized.java | 10 +++++- .../eradicate/FieldNotInitializedTest.java | 16 ++++++++- 6 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 infer/annotations/com/facebook/infer/annotation/SuppressViewNullability.java diff --git a/infer/annotations/com/facebook/infer/annotation/SuppressViewNullability.java b/infer/annotations/com/facebook/infer/annotation/SuppressViewNullability.java new file mode 100644 index 000000000..6d7ab47e1 --- /dev/null +++ b/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 {} diff --git a/infer/src/checkers/annotations.ml b/infer/src/checkers/annotations.ml index 3f16bbeda..de9cc6c31 100644 --- a/infer/src/checkers/annotations.ml +++ b/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_ diff --git a/infer/src/checkers/annotations.mli b/infer/src/checkers/annotations.mli index c8e304b2a..353057f1b 100644 --- a/infer/src/checkers/annotations.mli +++ b/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 diff --git a/infer/src/eradicate/eradicateChecks.ml b/infer/src/eradicate/eradicateChecks.ml index acf757364..6df3ccd65 100644 --- a/infer/src/eradicate/eradicateChecks.ml +++ b/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; diff --git a/infer/tests/codetoanalyze/java/eradicate/FieldNotInitialized.java b/infer/tests/codetoanalyze/java/eradicate/FieldNotInitialized.java index 8fb1b5e73..3d1cf21b3 100644 --- a/infer/tests/codetoanalyze/java/eradicate/FieldNotInitialized.java +++ b/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 + } } diff --git a/infer/tests/endtoend/java/eradicate/FieldNotInitializedTest.java b/infer/tests/endtoend/java/eradicate/FieldNotInitializedTest.java index 8cdd3eeca..812324894 100644 --- a/infer/tests/endtoend/java/eradicate/FieldNotInitializedTest.java +++ b/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, "", - 1)); + 1 + ) + ); + + String[] procedures = {""}; + assertThat( + "Results should contain " + FIELD_NOT_INITIALIZED, + inferResults, + containsExactly( + FIELD_NOT_INITIALIZED, + SOURCE_FILE, + procedures + ) + ); } }