From 1617d470f1826fe2ab1fa5599d2a132511696c6a Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Thu, 14 Jul 2016 10:45:02 -0700 Subject: [PATCH] Creating a specific type for frontend issues Reviewed By: sblackshear Differential Revision: D3561522 fbshipit-source-id: f42ba93 --- infer/src/backend/errdesc.mli | 2 +- infer/src/backend/localise.ml | 5 +- infer/src/backend/localise.mli | 2 +- infer/src/clang/cFrontend_checkers.ml | 77 ++++++++++++-------------- infer/src/clang/cFrontend_checkers.mli | 20 ++----- infer/src/clang/cFrontend_errors.ml | 17 +++--- infer/src/clang/cIssue.ml | 44 +++++++++++++++ infer/src/clang/cIssue.mli | 27 +++++++++ 8 files changed, 127 insertions(+), 67 deletions(-) create mode 100644 infer/src/clang/cIssue.ml create mode 100644 infer/src/clang/cIssue.mli diff --git a/infer/src/backend/errdesc.mli b/infer/src/backend/errdesc.mli index d77fad5a0..ea1c4eeeb 100644 --- a/infer/src/backend/errdesc.mli +++ b/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 diff --git a/infer/src/backend/localise.ml b/infer/src/backend/localise.ml index 7c5916fb5..a2f9739fc 100644 --- a/infer/src/backend/localise.ml +++ b/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 diff --git a/infer/src/backend/localise.mli b/infer/src/backend/localise.mli index f47c88577..e31e57560 100644 --- a/infer/src/backend/localise.mli +++ b/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 -> diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index 809b7f7b7..2f1856a15 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/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 diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 0e80dbe84..c7c07d8ce 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/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 diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index faee89811..165feafc1 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/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 diff --git a/infer/src/clang/cIssue.ml b/infer/src/clang/cIssue.ml new file mode 100644 index 000000000..063bf5d45 --- /dev/null +++ b/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 *) +} diff --git a/infer/src/clang/cIssue.mli b/infer/src/clang/cIssue.mli new file mode 100644 index 000000000..e570005c4 --- /dev/null +++ b/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 *) +}