[linters] Clean up the context passing mechanism and add context for if

Summary:
Passing the context was a bit messy and there was duplicated code. The flow is as follows:
For each node, we analyze that node and then it's children, and in some cases we use the parent node
for specifying a context for its children. Now this should be a bit more clear.
In the case of if, we specify a context based on the condition that is expected to be true inside the
if body, but not inside the else body. Here only the framework is set, more if conditions will be added to the
context in future diffs.

Reviewed By: martinoluca

Differential Revision: D4462247

fbshipit-source-id: 3512bd2

infer/src/clang/CLintersContext.ml

@@ -9,6 +9,10 @@
 
 open! IStd
 
+type if_context = {
+  in_responds_to_selector_block : string list;
+}
+
 type context = {
   translation_unit_context : CFrontend_config.translation_unit_context;
   current_method : Clang_ast_t.decl option;
@@ -21,6 +25,7 @@ type context = {
   (** True if inside an objc static factory method (a class-level initializer, like +new) *)
   in_objc_static_factory_method : bool;
   et_evaluation_node : string option;
+  if_context : if_context option;
 }
 
 let empty translation_unit_context = {
@@ -31,4 +36,5 @@ let empty translation_unit_context = {
   current_objc_impl = None;
   in_objc_static_factory_method = false;
   et_evaluation_node = None;
+  if_context = None;
 }

infer/src/clang/ComponentKit.ml

@@ -322,11 +322,11 @@ let component_initializer_with_side_effects_advice
 let component_file_line_count_info (context: CLintersContext.context) dec =
   let condition = Config.compute_analytics && context.is_ck_translation_unit in
   match dec with
-  | Clang_ast_t.TranslationUnitDecl _ when condition ->
+  | CTL.Decl Clang_ast_t.TranslationUnitDecl _ when condition ->
       let source_file =
         context.translation_unit_context.CFrontend_config.source_file in
       let line_count = SourceFile.line_count source_file in
-      IList.map (fun i -> {
+      CTL.True, IList.map (fun i -> {
             CIssue.name = "COMPONENT_FILE_LINE_COUNT";
             severity = Exceptions.Kinfo;
             mode = CIssue.Off;
@@ -339,7 +339,7 @@ let component_file_line_count_info (context: CLintersContext.context) dec =
             }
           }
         ) (IList.range 1 line_count)
-  | _ -> []
+  | _ -> CTL.False, []
 
 (** Computes a component file's cyclomatic complexity.
 

infer/src/clang/ComponentKit.mli

@@ -32,7 +32,7 @@ val component_initializer_with_side_effects_advice :
   CLintersContext.context -> CTL.ast_node -> CTL.t * CIssue.issue_desc option
 
 val component_file_line_count_info :
-  CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc list
+  CLintersContext.context -> CTL.ast_node -> CTL.t * CIssue.issue_desc list
 
 val component_file_cyclomatic_complexity_info :
   CLintersContext.context -> CTL.ast_node -> CTL.t * CIssue.issue_desc option

infer/src/clang/cFrontend_checkers_main.ml

@@ -41,68 +41,80 @@ let parse_ctl_file linters_files =
        | None -> Logging.out "No linters found.\n");
       In_channel.close inx) linters_files
 
+let compute_if_context _ _ =
+  None (* to be extended *)
+
+let is_factory_method (context: CLintersContext.context) decl =
+  let interface_decl_opt =
+    (match context.current_objc_impl with
+     | Some ObjCImplementationDecl (_, _, _, _, impl_decl_info) ->
+         CAst_utils.get_decl_opt_with_decl_ref impl_decl_info.oidi_class_interface
+     | _ -> None) in
+  (match interface_decl_opt with
+   | Some interface_decl -> CAst_utils.is_objc_factory_method interface_decl decl
+   | _ -> false)
 
 let rec do_frontend_checks_stmt (context:CLintersContext.context) stmt =
   let open Clang_ast_t in
-  let context' = CFrontend_errors.run_frontend_checkers_on_an context (CTL.Stmt stmt) in
+  let do_all_checks_on_stmts context stmt =
-  let do_all_checks_on_stmts stmt =
     (match stmt with
      | DeclStmt (_, _, decl_list) ->
-         IList.iter (do_frontend_checks_decl context') decl_list
+         IList.iter (do_frontend_checks_decl context) decl_list
      | BlockExpr (_, _, _, decl) ->
-         IList.iter (do_frontend_checks_decl context') [decl]
+         IList.iter (do_frontend_checks_decl context) [decl]
      | _ -> ());
-    do_frontend_checks_stmt context' stmt in
+    do_frontend_checks_stmt context stmt in
-  let stmts = CAst_utils.get_stmts_from_stmt stmt in
+  CFrontend_errors.invoke_set_of_checkers_on_node context (CTL.Stmt stmt);
-  IList.iter (do_all_checks_on_stmts) stmts
+  match stmt with
+  | ObjCAtSynchronizedStmt (_, stmt_list) ->
+      let stmt_context = { context with CLintersContext.in_synchronized_block = true } in
+      IList.iter (do_all_checks_on_stmts stmt_context) stmt_list
+  | IfStmt (_, [stmt1; stmt2; cond_stmt; inside_if_stmt; inside_else_stmt]) ->
+      (* here we analyze the children of the if stmt with the standard context,
+         except for inside_if_stmt... *)
+      IList.iter (do_all_checks_on_stmts context) [stmt1; stmt2; cond_stmt; inside_else_stmt];
+      let inside_if_stmt_context =
+        {context with CLintersContext.if_context = compute_if_context context cond_stmt } in
+      (* ...and here we analyze the stmt inside the if with the context
+         extended with the condition of the if *)
+      do_all_checks_on_stmts inside_if_stmt_context inside_if_stmt
+  | _ ->
+      let stmts = CAst_utils.get_stmts_from_stmt stmt in
+      IList.iter (do_all_checks_on_stmts context) stmts
 
 and do_frontend_checks_decl (context: CLintersContext.context) decl =
   let open Clang_ast_t in
-  let context' =
+  CFrontend_errors.invoke_set_of_checkers_on_node context (CTL.Decl decl);
-    (match decl with
+  match decl with
-     | FunctionDecl(_, _, _, fdi)
+  | FunctionDecl(_, _, _, fdi)
-     | CXXMethodDecl (_, _, _, fdi, _)
+  | CXXMethodDecl (_, _, _, fdi, _)
-     | CXXConstructorDecl (_, _, _, fdi, _)
+  | CXXConstructorDecl (_, _, _, fdi, _)
-     | CXXConversionDecl (_, _, _, fdi, _)
+  | CXXConversionDecl (_, _, _, fdi, _)
-     | CXXDestructorDecl (_, _, _, fdi, _) ->
+  | CXXDestructorDecl (_, _, _, fdi, _) ->
-         let context' = {context with CLintersContext.current_method = Some decl } in
+      let context' = {context with CLintersContext.current_method = Some decl } in
-         (match fdi.Clang_ast_t.fdi_body with
+      (match fdi.Clang_ast_t.fdi_body with
-          | Some stmt ->
+       | Some stmt -> do_frontend_checks_stmt context' stmt
-              do_frontend_checks_stmt context' stmt
+       | None -> ())
-          | None -> ());
+  | ObjCMethodDecl (_, _, mdi) ->
-         context'
+      let context' = {context with
-     | ObjCMethodDecl (_, _, mdi) ->
+                      CLintersContext.current_method = Some decl;
-         let if_decl_opt =
+                      CLintersContext.in_objc_static_factory_method =
-           (match context.current_objc_impl with
+                        is_factory_method context decl} in
-            | Some ObjCImplementationDecl (_, _, _, _, impl_decl_info) ->
+      (match mdi.Clang_ast_t.omdi_body with
-                CAst_utils.get_decl_opt_with_decl_ref impl_decl_info.oidi_class_interface
+       | Some stmt -> do_frontend_checks_stmt context' stmt
-            | _ -> None) in
+       | None -> ())
-         let is_factory_method =
+  | BlockDecl (_, block_decl_info) ->
-           (match if_decl_opt with
+      let context' = {context with CLintersContext.current_method = Some decl } in
-            | Some if_decl -> CAst_utils.is_objc_factory_method if_decl decl
+      (match block_decl_info.Clang_ast_t.bdi_body with
-            | _ -> false) in
+       | Some stmt -> do_frontend_checks_stmt context' stmt
-         let context' = {context with
+       | None -> ())
-                         CLintersContext.current_method = Some decl;
+  | ObjCImplementationDecl (_, _, decls, _, _) ->
-                         CLintersContext.in_objc_static_factory_method = is_factory_method} in
+      let context' = {context with current_objc_impl = Some decl} in
-         (match mdi.Clang_ast_t.omdi_body with
+      IList.iter (do_frontend_checks_decl context') decls
-          | Some stmt ->
+  | _ -> match Clang_ast_proj.get_decl_context_tuple decl with
-              do_frontend_checks_stmt context' stmt
+    | Some (decls, _) ->
-          | None -> ());
+        IList.iter (do_frontend_checks_decl context) decls
-         context'
+    | None -> ()
-     | BlockDecl (_, block_decl_info) ->
-         let context' = {context with CLintersContext.current_method = Some decl } in
-         (match block_decl_info.Clang_ast_t.bdi_body with
-          | Some stmt ->
-              do_frontend_checks_stmt context' stmt
-          | None -> ());
-         context'
-     | _ -> context) in
-  let context'' = CFrontend_errors.run_frontend_checkers_on_an context' (CTL.Decl decl) in
-  let context_with_orig_current_method =
-    {context'' with CLintersContext.current_method = context.current_method } in
-  match Clang_ast_proj.get_decl_context_tuple decl with
-  | Some (decls, _) -> IList.iter (do_frontend_checks_decl context_with_orig_current_method) decls
-  | None -> ()
 
 let context_with_ck_set context decl_list =
   let is_ck = context.CLintersContext.is_ck_translation_unit
@@ -129,12 +141,12 @@ let do_frontend_checks trans_unit_ctx ast =
     | Clang_ast_t.TranslationUnitDecl(_, decl_list, _, _) ->
         let context =
           context_with_ck_set (CLintersContext.empty trans_unit_ctx) decl_list in
-        ignore (CFrontend_errors.run_translation_unit_checker context ast);
-        ignore (CFrontend_errors.run_frontend_checkers_on_an context (CTL.Decl ast));
         let is_decl_allowed decl =
           let decl_info = Clang_ast_proj.get_decl_tuple decl in
           CLocation.should_do_frontend_check trans_unit_ctx decl_info.Clang_ast_t.di_source_range in
         let allowed_decls = IList.filter is_decl_allowed decl_list in
+        (* We analyze the top level and then all the allowed declarations *)
+        CFrontend_errors.invoke_set_of_checkers_on_node context (CTL.Decl ast);
         IList.iter (do_frontend_checks_decl context) allowed_decls;
         if (LintIssues.exists_issues ()) then
           store_issues source_file;

infer/src/clang/cFrontend_errors.ml

@@ -24,7 +24,8 @@ let decl_single_checkers_list =
 (* List of checkers on decls *)
 let decl_checkers_list =
   ComponentKit.component_with_multiple_factory_methods_advice::
-  (IList.map single_to_multi decl_single_checkers_list)
+  (ComponentKit.component_file_line_count_info::
+   (IList.map single_to_multi decl_single_checkers_list))
 
 (* List of checkers on stmts *that return 0 or 1 issue* *)
 let stmt_single_checkers_list =
@@ -34,9 +35,6 @@ let stmt_single_checkers_list =
 
 let stmt_checkers_list = IList.map single_to_multi stmt_single_checkers_list
 
-(* List of checkers on translation unit that potentially output multiple issues *)
-let translation_unit_checkers_list = [ComponentKit.component_file_line_count_info;]
-
 (* List of checkers that will be filled after parsing them from a file *)
 let checkers_decl_stmt = ref []
 
@@ -195,15 +193,26 @@ let log_frontend_issue translation_unit_context method_decl_opt key issue_desc =
   Reporting.log_issue_from_errlog err_kind errlog exn ~loc ~ltr:trace
     ~node_id:(0, key)
 
+let get_current_method context an =
+  match an with
+  | CTL.Decl (FunctionDecl _ as d)
+  | CTL.Decl (CXXMethodDecl _ as d)
+  | CTL.Decl (CXXConstructorDecl _ as d)
+  | CTL.Decl (CXXConversionDecl _ as d)
+  | CTL.Decl (CXXDestructorDecl _ as d)
+  | CTL.Decl (ObjCMethodDecl _ as d)
+  | CTL.Decl (BlockDecl _ as d) -> Some d
+  | _ -> context.CLintersContext.current_method
+
 let fill_issue_desc_info_and_log context an key issue_desc loc =
   let desc = expand_message_string issue_desc.CIssue.description an in
   let issue_desc' =
     {issue_desc with CIssue.description = desc; CIssue.loc = loc } in
   log_frontend_issue context.CLintersContext.translation_unit_context
-    context.CLintersContext.current_method key issue_desc'
+    (get_current_method context an) key issue_desc'
 
 (* Calls the set of hard coded checkers (if any) *)
-let invoke_set_of_hard_coded_checkers_an an context =
+let invoke_set_of_hard_coded_checkers_an context an =
   let checkers, key  = match an with
     | CTL.Decl dec -> decl_checkers_list, CAst_utils.generate_key_decl dec
     | CTL.Stmt st -> stmt_checkers_list, CAst_utils.generate_key_stmt st in
@@ -218,7 +227,7 @@ let invoke_set_of_hard_coded_checkers_an an context =
     ) checkers
 
 (* Calls the set of checkers parsed from files (if any) *)
-let invoke_set_of_parsed_checkers_an an context =
+let invoke_set_of_parsed_checkers_an context an =
   let key = match an with
     | CTL.Decl dec -> CAst_utils.generate_key_decl dec
     | CTL.Stmt st -> CAst_utils.generate_key_stmt st in
@@ -230,28 +239,6 @@ let invoke_set_of_parsed_checkers_an an context =
     ) !checkers_decl_stmt
 
 (* We decouple the hardcoded checkers from the parsed ones *)
-let invoke_set_of_checkers_an an context =
+let invoke_set_of_checkers_on_node context an =
-  invoke_set_of_parsed_checkers_an an context;
+  invoke_set_of_parsed_checkers_an context an;
-  invoke_set_of_hard_coded_checkers_an an context
+  invoke_set_of_hard_coded_checkers_an context an
-
-
-let run_frontend_checkers_on_an (context: CLintersContext.context) an =
-  let open Clang_ast_t in
-  let context' = match an with
-    | CTL.Decl (ObjCImplementationDecl _ as dec) ->
-        {context with current_objc_impl = Some dec}
-    | CTL.Stmt (ObjCAtSynchronizedStmt _ )->
-        { context with CLintersContext.in_synchronized_block = true }
-    | _ -> context in
-  invoke_set_of_checkers_an an context';
-  context'
-
-let run_translation_unit_checker (context: CLintersContext.context) dec =
-  IList.iter (fun checker ->
-      let issue_desc_list = checker context dec in
-      IList.iter (fun issue_desc ->
-          if (CIssue.should_run_check issue_desc.CIssue.mode) then
-            let key = CAst_utils.generate_key_decl dec in
-            log_frontend_issue context.CLintersContext.translation_unit_context
-              context.CLintersContext.current_method key issue_desc
-        ) issue_desc_list) translation_unit_checkers_list

infer/src/clang/cFrontend_errors.mli

@@ -13,14 +13,7 @@ open! IStd
 (* Module for warnings detected at translation time by the frontend *)
 
 (* Run frontend checkers on an AST node *)
-val run_frontend_checkers_on_an :
+val invoke_set_of_checkers_on_node : CLintersContext.context -> CTL.ast_node -> unit
-  CLintersContext.context -> CTL.ast_node -> CLintersContext.context
-
-(** Same as run_frontend_checkers_on_an except special-cased on the translation
-    unit. Translation unit level checkers may return multiple issues, which is
-    why special-casing is necessary here. *)
-val run_translation_unit_checker :
-  CLintersContext.context -> Clang_ast_t.decl -> unit
 
 val expand_checkers : Ctl_parser_types.ctl_checker list -> Ctl_parser_types.ctl_checker list