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

@@ -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
+    CIssue.description =
-        "Property or ivar %s declared strong" (decl_name_from_an an);
+      "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
+    CIssue.description =
-        "Global variable %s is initialized using a function or method call"
+      "Global variable %decl_name% is initialized using a function or method call";
-        (decl_name_from_an an);
     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 `%decl_name%` is a pointer type marked with the `assign` attribute";
-          "Property `%s` is a pointer type marked with the `assign` attribute"
-          (decl_name_from_an an);
       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
+    CIssue.description = "Direct access to ivar %ivar_name% of an atomic property";
-        "Direct access to ivar %s of an atomic property" (ivar_name an);
     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
+    CIssue.description =
-        "C++ Reference variable(s) %s captured by Objective-C block"
+      "C++ Reference variable(s) %var_name% captured by Objective-C block";
-        (var_descs_name an);
     CIssue.suggestion = Some ("C++ References are unmanaged and may be invalid " ^
                               "by the time the block executes.");
     CIssue.loc = match an with

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

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
+      | true, Some issue_desc ->
-                                   context.CLintersContext.current_method key 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
 
 

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 =
+  SET message =
-       "Global variable %decl_name is initialized using a function or method call";
+       "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 =
+	  SET message =
-	        "C++ Reference variable(s) %var_desc_name captured by Objective-C block";
+	        "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.";