From 4682393cd6144059d06e316b51e479e8a9317a3c Mon Sep 17 00:00:00 2001 From: Dino Distefano Date: Wed, 14 Dec 2016 07:01:22 -0800 Subject: [PATCH] Use place-holders string in messages Summary: This diff introduces place-holders strings in error messages and evaluates them when an error needs to be reported. Place-holders strings are of the form %name_of_helper_function%. They will be evaluated using and helper function that gives a value. For example if we need to display the name of a variable in the error message we will have: ".... %var_name%...." then %var_name% will be evaluated in the ast node calling the appropriate helper function and the results will replace %var_name% in the message. Reviewed By: dulmarod Differential Revision: D4313133 fbshipit-source-id: bf521ca --- infer/src/clang/cFrontend_checkers.ml | 27 ++++++++---------- infer/src/clang/cFrontend_checkers.mli | 4 +-- infer/src/clang/cFrontend_errors.ml | 37 +++++++++++++++++++++++-- infer/src/clang/linter_rules/linters.al | 18 ++++++------ 4 files changed, 57 insertions(+), 29 deletions(-) diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index f29f829dc..5ae673be0 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/infer/src/clang/cFrontend_checkers.ml @@ -42,7 +42,7 @@ let location_from_an lcxt an = | CTL.Decl d -> location_from_decl lcxt d -let decl_name_from_an an = +let decl_name an = match an with | CTL.Decl dec -> (match Clang_ast_proj.get_named_decl_tuple dec with @@ -62,7 +62,7 @@ let ivar_name an = | _ -> "") | _ -> "" -let var_descs_name an = +let var_name an = let capt_refs = match an with | CTL.Stmt (Clang_ast_t.BlockExpr (_, _ , _, decl)) -> Predicates.captured_variables_cxx_ref decl @@ -121,7 +121,7 @@ let ctl_ns_notification_warning lctx an = let issue_desc = { CIssue.issue = CIssue.Registered_observer_being_deallocated; CIssue.description = - Localise.registered_observer_being_deallocated_str CFrontend_config.self; + "Object self is registered in a notification center but not being removed before deallocation"; CIssue.suggestion = Some "Consider removing the object from the notification center before its deallocation."; CIssue.loc = location_from_an lctx an; @@ -176,8 +176,8 @@ let ctl_strong_delegate_warning lctx an = is_strong_property))) in let issue_desc = { CIssue.issue = CIssue.Strong_delegate_warning; - CIssue.description = Printf.sprintf - "Property or ivar %s declared strong" (decl_name_from_an an); + CIssue.description = + "Property or ivar %decl_name% declared strong"; CIssue.suggestion = Some "In general delegates should be declared weak or assign"; CIssue.loc = location_from_an lctx an } in @@ -197,9 +197,8 @@ let ctl_global_var_init_with_calls_warning lctx an = InNode (["VarDecl"], And (ctl_is_global_var, ctl_is_initialized_with_expensive_call)) in let issue_desc = { CIssue.issue = CIssue.Global_variable_initialized_with_function_or_method_call; - CIssue.description = Printf.sprintf - "Global variable %s is initialized using a function or method call" - (decl_name_from_an an); + CIssue.description = + "Global variable %decl_name% 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_an lctx an @@ -215,9 +214,7 @@ let ctl_assign_pointer_warning lctx an = let issue_desc = { CIssue.issue = CIssue.Assign_pointer_warning; CIssue.description = - Printf.sprintf - "Property `%s` is a pointer type marked with the `assign` attribute" - (decl_name_from_an an); + "Property `%decl_name%` is a pointer type marked with the `assign` attribute"; CIssue.suggestion = Some "Use a different attribute like `strong` or `weak`."; CIssue.loc = location_from_an lctx an } in @@ -237,8 +234,7 @@ let ctl_direct_atomic_property_access_warning lctx an = Not (Atomic ("is_objc_dealloc", [])))) in let issue_desc = { CIssue.issue = CIssue.Direct_atomic_property_access; - CIssue.description = Printf.sprintf - "Direct access to ivar %s of an atomic property" (ivar_name an); + 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_an lctx an @@ -251,9 +247,8 @@ let ctl_captured_cxx_ref_in_objc_block_warning lctx an = let condition = InNode (["BlockDecl"], Atomic ("captures_cxx_references", [])) in let issue_desc = { CIssue.issue = CIssue.Cxx_reference_captured_in_objc_block; - CIssue.description = Printf.sprintf - "C++ Reference variable(s) %s captured by Objective-C block" - (var_descs_name an); + CIssue.description = + "C++ Reference variable(s) %var_name% captured by Objective-C block"; CIssue.suggestion = Some ("C++ References are unmanaged and may be invalid " ^ "by the time the block executes."); CIssue.loc = match an with diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index c7aea9ef6..54b65d62f 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -52,8 +52,8 @@ val location_from_dinfo : val location_from_decl : CLintersContext.context -> Clang_ast_t.decl -> Location.t -val decl_name_from_an : CTL.ast_node -> string +val decl_name : CTL.ast_node -> string val ivar_name : CTL.ast_node -> string -val var_descs_name : CTL.ast_node -> string +val var_name : CTL.ast_node -> string diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 02f88f514..b82e26823 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -32,6 +32,35 @@ let stmt_checkers_list = [CFrontend_checkers.ctl_direct_atomic_property_access_ (* List of checkers on translation unit that potentially output multiple issues *) let translation_unit_checkers_list = [ComponentKit.component_file_line_count_info;] +let evaluate_place_holder ph an = + match ph with + | "%ivar_name%" -> CFrontend_checkers.ivar_name an + | "%decl_name%" -> CFrontend_checkers.decl_name an + | "%var_name%" -> CFrontend_checkers.var_name an + | _ -> (Logging.err "ERROR: helper function %s is unknown. Stop.\n" ph; + assert false) + +(* given a message this function searches for a place-holder identifier, + eg %id%. Then it evaluates id and replaces %id% in message + with the result of its evaluation. The function keeps on checking if + other place-holders exist and repeats the process until there are + no place-holder left. +*) +let rec expand_message_string message an = + (* reg exp should match alphanumeric id with possibly somee _ *) + let re = Str.regexp "%[a-zA-Z0-9_]+%" in + try + let _ = Str.search_forward re message 0 in + let ms = Str.matched_string message in + let res = evaluate_place_holder ms an in + Logging.out "\nMatched string '%s'\n" ms; + let re_ms = Str.regexp_string ms in + let message' = Str.replace_first re_ms res message in + Logging.out "Replacing %s in message: \n %s \n" ms message; + Logging.out "Resulting message: \n %s \n" message'; + expand_message_string message' an + with Not_found -> message + (* expands use of let defined formula id in checkers with their definition *) let expand_checkers checkers = @@ -112,8 +141,12 @@ let invoke_set_of_checkers_an an context = IList.iter (fun checker -> let condition, issue_desc_opt = checker context an in match CTL.eval_formula condition an context, issue_desc_opt with - | true, Some issue_desc -> log_frontend_issue context.CLintersContext.translation_unit_context - context.CLintersContext.current_method key issue_desc + | true, Some issue_desc -> + + let desc' = expand_message_string issue_desc.CIssue.description an in + let issue_desc' = {issue_desc with CIssue.description = desc'} in + log_frontend_issue context.CLintersContext.translation_unit_context + context.CLintersContext.current_method key issue_desc' | _, _ -> ()) checkers diff --git a/infer/src/clang/linter_rules/linters.al b/infer/src/clang/linter_rules/linters.al index a34ac052d..533bc5272 100644 --- a/infer/src/clang/linter_rules/linters.al +++ b/infer/src/clang/linter_rules/linters.al @@ -17,7 +17,7 @@ DEFINE-CHECKER DIRECT_ATOMIC_PROPERTY_ACCESS = { AND NOT is_objc_dealloc() HOLDS-IN-NODE ObjCIvarRefExpr; - SET message = "Direct access to ivar %ivar_name of an atomic property"; + SET message = "Direct access to ivar %ivar_name% of an atomic property"; SET suggestion = "Accessing an ivar of an atomic property makes the property nonatomic"; SET severity = "WARNING"; }; @@ -31,7 +31,7 @@ DEFINE-CHECKER ASSIGN_POINTER_WARNING = { is_assign_property() AND is_property_pointer_type() HOLDS-IN-NODE ObjCPropertyDecl; - SET message = "Property `%decl_name` is a pointer type marked with the `assign` attribute"; + SET message = "Property `%decl_name%` is a pointer type marked with the `assign` attribute"; SET suggestion = "Use a different attribute like `strong` or `weak`."; SET severity = "WARNING"; }; @@ -71,7 +71,7 @@ DEFINE-CHECKER BAD_POINTER_COMPARISON = { etx HOLDS-IN-NODE IfStmt, ForStmt, WhileStmt, ConditionalOperator; - SET description = "Implicitly checking whether NSNumber pointer is nil"; + SET message = "Implicitly checking whether NSNumber pointer is nil"; SET suggestion = "Did you mean to compare against the unboxed value instead? Please either explicitly compare the NSNumber instance to nil, or use one of the NSNumber accessors before the comparison."; @@ -127,7 +127,7 @@ DEFINE-CHECKER REGISTERED_OBSERVER_BEING_DEALLOCATED = { NOT (eventually_addObserver IMPLIES eventually_removeObserver) HOLDS-IN-NODE ObjCImplementationDecl, ObjCProtocolDecl; - SET description = + SET message = "Object self is registered in a notification center but not being removed before deallocation"; SET suggestion = @@ -145,7 +145,7 @@ DEFINE-CHECKER strong_delegate_warning = { name_contains_delegate AND name_does_not_contains_queue AND is_strong_property() HOLDS-IN-NODE ObjCPropertyDecl; - SET description = "Property or ivar %decl_name declared strong"; + SET message = "Property or ivar %decl_name% declared strong"; SET suggestion = "In general delegates should be declared weak or assign"; }; @@ -172,8 +172,8 @@ DEFINE-CHECKER global_var_init_with_calls_warning = { (is_global_var AND is_initialized_with_expensive_call) HOLDS-IN-NODE VarDecl; - SET description = - "Global variable %decl_name is initialized using a function or method call"; + SET message = + "Global variable %decl_name% is initialized using a function or method call"; SET suggestion = "If the function/method call is expensive, it can affect the starting time of the app."; }; @@ -185,8 +185,8 @@ DEFINE-CHECKER ctl_captured_cxx_ref_in_objc_block_warning = { captures_cxx_references() HOLDS-IN-NODE BlockDecl; - SET description = - "C++ Reference variable(s) %var_desc_name captured by Objective-C block"; + SET message = + "C++ Reference variable(s) %var_name% captured by Objective-C block"; SET suggestion = "C++ References are unmanaged and may be invalid by the time the block executes.";