Creating a specific type for frontend issues

Reviewed By: sblackshear

Differential Revision: D3561522

fbshipit-source-id: f42ba93

infer/src/backend/errdesc.mli

@@ -97,7 +97,7 @@ val explain_stack_variable_address_escape :
   Location.t -> Pvar.t -> DecompiledExp.t option -> Localise.error_desc
 
 (** explain frontend warning *)
-val explain_frontend_warning : string -> string -> Location.t -> Localise.error_desc
+val explain_frontend_warning : string -> string option -> Location.t -> Localise.error_desc
 
 (** explain a return statement missing *)
 val explain_return_statement_missing : Location.t -> Localise.error_desc

infer/src/backend/localise.ml

@@ -660,8 +660,11 @@ let desc_empty_vector_access pname_opt object_str loc =
 
 let is_empty_vector_access_desc desc = has_tag desc Tags.empty_vector_access
 
-let desc_frontend_warning desc sugg loc =
+let desc_frontend_warning desc sugg_opt loc =
   let tags = Tags.create () in
+  let sugg = match sugg_opt with
+    | Some sugg -> sugg
+    | None -> "" in
   let description = Format.sprintf
       "%s %s. %s"
       desc

infer/src/backend/localise.mli

@@ -207,7 +207,7 @@ val desc_empty_vector_access : Procname.t option -> string -> Location.t -> erro
 
 val is_empty_vector_access_desc : error_desc -> bool
 
-val desc_frontend_warning : string -> string -> Location.t -> error_desc
+val desc_frontend_warning : string -> string option -> Location.t -> error_desc
 
 val desc_leak :
   Sil.exp option -> string option -> Sil.resource option -> Sil.res_action option ->

infer/src/clang/cFrontend_checkers.ml

@@ -22,14 +22,6 @@ open CFrontend_utils
 (*    run_frontend_checkers_on_stmt in CFrontend_error module.*)
 (*    - If it is a declaration invoke it from run_frontend_checkers_on_decl *)
 
-type issue_desc = {
-  name : string; (* name for the checker, this will be a kind of bug *)
-  description : string; (* Description in the error message *)
-  suggestion : string; (* an optional suggestion or correction *)
-  loc : Location.t; (* location in the code *)
-  kind : Exceptions.err_kind; (* issue kind *)
-}
-
 (* Helper functions *)
 
 let get_ivar_attributes ivar_decl =
@@ -177,14 +169,13 @@ let assign_pointer_warning decl_info pname obj_c_property_decl_info =
     has_assign_property() && is_pointer_type () in
   if condition then
     Some
-      { name = "ASSIGN_POINTER_WARNING";
-        description =
+      { CIssue.issue = CIssue.Assign_pointer_warning;
+        CIssue.description =
           Printf.sprintf
             "Property `%s` is a pointer type marked with the `assign` attribute"
             pname.ni_name;
-        suggestion = "Use a different attribute like `strong` or `weak`.";
-        loc = location_from_dinfo decl_info;
-        kind = Exceptions.Kwarning
+        CIssue.suggestion = Some "Use a different attribute like `strong` or `weak`.";
+        CIssue.loc = location_from_dinfo decl_info
       }
   else None
 
@@ -194,11 +185,10 @@ let strong_delegate_warning decl_info pname obj_c_property_decl_info =
                   && not (name_contains_word pname "queue")
                   && ObjcProperty_decl.is_strong_property obj_c_property_decl_info in
   if condition then
-    Some { name = "STRONG_DELEGATE_WARNING";
-           description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong";
-           suggestion = "In general delegates should be declared weak or assign";
-           loc = location_from_dinfo decl_info;
-           kind = Exceptions.Kwarning
+    Some { CIssue.issue = CIssue.Strong_delegate_warning;
+           CIssue.description = "Property or ivar "^pname.Clang_ast_t.ni_name^" declared strong";
+           CIssue.suggestion = Some "In general delegates should be declared weak or assign";
+           CIssue.loc = location_from_dinfo decl_info
          }
   else None
 
@@ -216,12 +206,12 @@ let global_var_init_with_calls_warning decl =
                   && is_initialized_with_expensive_call decl in
   if condition then
     Some {
-      name = "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL";
-      description = "Global variable " ^ gvar ^ 
-                    " is initialized using a function or method call";
-      suggestion = "If the function/method call is expensive, it can affect the starting time of the app.";
-      loc = location_from_dinfo decl_info;
-      kind = Exceptions.Kwarning
+      CIssue.issue = CIssue.Global_variable_initialized_with_function_or_method_call;
+      CIssue.description = "Global variable " ^ gvar ^
+                           " is initialized using a function or method call";
+      CIssue.suggestion = Some
+          "If the function/method call is expensive, it can affect the starting time of the app.";
+      CIssue.loc = location_from_dinfo decl_info
     }
   else None
 
@@ -247,12 +237,13 @@ let direct_atomic_property_access_warning method_decl stmt_info ivar_decl_ref =
         && not (Procname.is_objc_dealloc method_name) in
       if condition then
         Some {
-          name = "DIRECT_ATOMIC_PROPERTY_ACCESS";
-          description = "Direct access to ivar " ^ ivar_name ^
-                        " of an atomic property";
-          suggestion = "Accessing an ivar of an atomic property makes the property nonatomic";
-          loc = location_from_sinfo stmt_info;
-          kind = Exceptions.Kwarning }
+          CIssue.issue = CIssue.Direct_atomic_property_access;
+          CIssue.description = "Direct access to ivar " ^ ivar_name ^
+                               " of an atomic property";
+          CIssue.suggestion =
+            Some "Accessing an ivar of an atomic property makes the property nonatomic";
+          CIssue.loc = location_from_sinfo stmt_info
+        }
       else None
   | _ -> None
 
@@ -269,13 +260,13 @@ let captured_cxx_ref_in_objc_block_warning stmt_info captured_vars =
   let condition = IList.length capt_refs > 0 in
   if condition then
     Some {
-      name = "CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK";
-      description = "C++ Reference variable(s) " ^ var_descs ^
-                    " captured by Objective-C block";
-      suggestion = "C++ References are unmanaged and may be invalid " ^
-                   "by the time the block executes.";
-      loc = location_from_sinfo stmt_info;
-      kind = Exceptions.Kwarning}
+      CIssue.issue = CIssue.Cxx_reference_captured_in_objc_block;
+      CIssue.description = "C++ Reference variable(s) " ^ var_descs ^
+                           " captured by Objective-C block";
+      CIssue.suggestion = Some ("C++ References are unmanaged and may be invalid " ^
+                                "by the time the block executes.");
+      CIssue.loc = location_from_sinfo stmt_info
+    }
   else None
 
 
@@ -296,9 +287,11 @@ let checker_NSNotificationCenter decl_info decls =
   let condition = eventually_addObserver && (not eventually_removeObserver) in
   if condition then
     Some {
-      name = Localise.to_string (Localise.registered_observer_being_deallocated);
-      description = Localise.registered_observer_being_deallocated_str CFrontend_config.self;
-      suggestion =  "Consider removing the object from the notification center before its deallocation.";
-      loc = location_from_dinfo decl_info;
-      kind = Exceptions.Kwarning }
+      CIssue.issue = CIssue.Registered_observer_being_deallocated;
+      CIssue.description =
+        Localise.registered_observer_being_deallocated_str CFrontend_config.self;
+      CIssue.suggestion =
+        Some "Consider removing the object from the notification center before its deallocation.";
+      CIssue.loc = location_from_dinfo decl_info
+    }
   else None

infer/src/clang/cFrontend_checkers.mli

@@ -9,40 +9,32 @@
 
 open! Utils
 
-type issue_desc = {
-  name : string; (* name for the checker, this will be a kind of bug *)
-  description : string; (* Description in the error message *)
-  suggestion : string; (* an optional suggestion or correction *)
-  loc : Location.t; (* location in the code *)
-  kind : Exceptions.err_kind (* issue kind *)
-}
-
 (* === Warnings on properties === *)
 
 (* Strong Delegate Warning: a property with name delegate should not be declared strong *)
 val strong_delegate_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
-  Clang_ast_t.obj_c_property_decl_info -> issue_desc option
+  Clang_ast_t.obj_c_property_decl_info -> CIssue.issue_desc option
 
 (* Assing Pointer Warning: a property with a pointer type should not be declared `assign` *)
 val assign_pointer_warning : Clang_ast_t.decl_info -> Clang_ast_t.named_decl_info ->
-  Clang_ast_t.obj_c_property_decl_info -> issue_desc option
+  Clang_ast_t.obj_c_property_decl_info -> CIssue.issue_desc option
 
 (* Direct Atomic Property access:
    a property declared atomic should not be accesses directly via its iva *)
 val direct_atomic_property_access_warning : Clang_ast_t.decl -> Clang_ast_t.stmt_info ->
-  Clang_ast_t.decl_ref -> issue_desc option
+  Clang_ast_t.decl_ref -> CIssue.issue_desc option
 
 (* CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK: C++ references
    should not be captured in blocks.  *)
 val captured_cxx_ref_in_objc_block_warning : Clang_ast_t.stmt_info ->
-  Clang_ast_t.block_captured_variable list -> issue_desc option
+  Clang_ast_t.block_captured_variable list -> CIssue.issue_desc option
 
 (* REGISTERED_OBSERVER_BEING_DEALLOCATED: an object is registered in a notification center
    but not removed before deallocation *)
 val checker_NSNotificationCenter : Clang_ast_t.decl_info -> Clang_ast_t.decl list ->
-  issue_desc option
+  CIssue.issue_desc option
 
 (* GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL warning: a global variable initialization should not *)
 (* contain calls to functions or methods as these can be expensive an delay the starting time *)
 (* of a program *)
-val global_var_init_with_calls_warning : Clang_ast_t.decl -> issue_desc option
+val global_var_init_with_calls_warning : Clang_ast_t.decl -> CIssue.issue_desc option

infer/src/clang/cFrontend_errors.ml

@@ -49,20 +49,21 @@ let checker_for_global_var dec checker =
 
 (* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *)
 let log_frontend_issue cfg cg issue_desc =
-  let open CFrontend_checkers in
-  let loc = issue_desc.loc in
+  let issue = issue_desc.CIssue.issue in
+  let loc = issue_desc.CIssue.loc in
   let pdesc = CMethod_trans.get_method_for_frontend_checks cfg cg loc in
   let errlog = Cfg.Procdesc.get_err_log pdesc in
-  let err_desc =
-    Errdesc.explain_frontend_warning issue_desc.description issue_desc.suggestion loc in
-  let exn = Exceptions.Frontend_warning
-      (issue_desc.name, err_desc, __POS__) in
+  let err_desc = Errdesc.explain_frontend_warning issue_desc.CIssue.description
+      issue_desc.CIssue.suggestion loc in
+  let name = CIssue.to_string issue in
+  let exn = Exceptions.Frontend_warning (name, err_desc, __POS__) in
   let trace = [
     { Errlog.lt_level = 0;
-      Errlog.lt_loc = issue_desc.loc;
+      Errlog.lt_loc = issue_desc.CIssue.loc;
       Errlog.lt_description = "";
       Errlog.lt_node_tags = []}] in
-  Reporting.log_issue_from_errlog issue_desc.kind errlog exn ~loc:(Some loc) ~ltr:(Some trace)
+  let err_kind = CIssue.severity_of_issue issue in
+  Reporting.log_issue_from_errlog err_kind errlog exn ~loc:(Some loc) ~ltr:(Some trace)
 
 (* General invocation function for checkers
    Takes

infer/src/clang/cIssue.ml

@@ -0,0 +1,44 @@
+(*
+ * 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.
+ *)
+
+type issue =
+  | Assign_pointer_warning
+  | Strong_delegate_warning
+  | Global_variable_initialized_with_function_or_method_call
+  | Direct_atomic_property_access
+  | Cxx_reference_captured_in_objc_block
+  | Registered_observer_being_deallocated
+
+let to_string issue =
+  match issue with
+  | Assign_pointer_warning -> "ASSIGN_POINTER_WARNING"
+  | Strong_delegate_warning -> "STRONG_DELEGATE_WARNING"
+  | Global_variable_initialized_with_function_or_method_call ->
+      "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL"
+  | Direct_atomic_property_access -> "DIRECT_ATOMIC_PROPERTY_ACCESS"
+  | Cxx_reference_captured_in_objc_block -> "CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK"
+  | Registered_observer_being_deallocated ->
+      Localise.to_string (Localise.registered_observer_being_deallocated)
+
+let severity_of_issue issue =
+  match issue with
+  | Assign_pointer_warning
+  | Strong_delegate_warning
+  | Global_variable_initialized_with_function_or_method_call
+  | Direct_atomic_property_access
+  | Cxx_reference_captured_in_objc_block
+  | Registered_observer_being_deallocated -> Exceptions.Kwarning
+
+
+type issue_desc = {
+  issue : issue; (* issue *)
+  description : string; (* Description in the error message *)
+  suggestion : string option; (* an optional suggestion or correction *)
+  loc : Location.t; (* location in the code *)
+}

infer/src/clang/cIssue.mli

@@ -0,0 +1,27 @@
+(*
+ * 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.
+ *)
+
+type issue =
+  | Assign_pointer_warning
+  | Strong_delegate_warning
+  | Global_variable_initialized_with_function_or_method_call
+  | Direct_atomic_property_access
+  | Cxx_reference_captured_in_objc_block
+  | Registered_observer_being_deallocated
+
+val to_string : issue -> string
+
+val severity_of_issue : issue -> Exceptions.err_kind
+
+type issue_desc = {
+  issue : issue; (* issue *)
+  description : string; (* Description in the error message *)
+  suggestion : string option; (* an optional suggestion or correction *)
+  loc : Location.t; (* location in the code *)
+}