[CTL] Filter linters through a visibility flag

Summary:
This will simplify the InferPrint logic of checking what should/should-not be reported.
I will remove the issue names in Localise in a next diff.

Reviewed By: ddino

Differential Revision: D4334327

fbshipit-source-id: ebcfd6c

Makefile

@@ -16,7 +16,7 @@ endif
 BUILD_SYSTEMS_TESTS = \
   assembly ck_analytics ck_imports clang_multiple_files clang_translation clang_unknown_ext \
   delete_results_dir fail_on_issue gradle j1 javac linters make project_root_rel reactive \
-  utf8_in_procname utf8_in_pwd waf
+  utf8_in_procname utf8_in_pwd waf run_hidden_linters
 ifneq ($(ANT),no)
 BUILD_SYSTEMS_TESTS += ant
 endif

infer/src/IR/Exceptions.ml

@@ -32,7 +32,7 @@ type exception_severity =
   | Low (* low severity bug *)
 
 (** class of error *)
-type err_class = Checker | Prover | Nocat
+type err_class = Checker | Prover | Nocat | Linters
 
 (** kind of error/warning *)
 type err_kind = Kwarning | Kerror | Kinfo | Kadvice [@@deriving compare]
@@ -183,7 +183,7 @@ let recognize_exception exn =
          desc, Some ml_loc, Exn_user, Medium, Some Kwarning, Nocat)
     | Frontend_warning (name, desc, ml_loc) ->
         (Localise.from_string name,
-         desc, Some ml_loc, Exn_user, Medium, None, Nocat)
+         desc, Some ml_loc, Exn_user, Medium, None, Linters)
     | Checkers (kind_s, desc) ->
         (Localise.from_string kind_s,
          desc, None, Exn_user, High, None, Prover)
@@ -335,6 +335,7 @@ let err_class_string = function
   | Checker -> "CHECKER"
   | Prover -> "PROVER"
   | Nocat -> ""
+  | Linters -> "Linters"
 
 (** wether to print the bug key together with the error message *)
 let print_key = false

infer/src/IR/Exceptions.mli

@@ -30,7 +30,7 @@ type exception_severity =
 type err_kind = Kwarning | Kerror | Kinfo | Kadvice [@@deriving compare]
 
 (** class of error *)
-type err_class = Checker | Prover | Nocat
+type err_class = Checker | Prover | Nocat | Linters
 
 exception Abduction_case_not_implemented of Logging.ml_loc
 exception Analysis_stops of Localise.error_desc * Logging.ml_loc option

infer/src/backend/InferPrint.re

@@ -300,8 +300,8 @@ let module ProcsXml = {
   let pp_procs_close fmt () => Io_infer.Xml.pp_close fmt "procedures";
 };
 
-let should_report (issue_kind: Exceptions.err_kind) issue_type error_desc =>
+let should_report (issue_kind: Exceptions.err_kind) issue_type error_desc eclass =>
-  if (not Config.filtering) {
+  if (not Config.filtering || eclass == Exceptions.Linters) {
     true
   } else {
     let analyzer_is_whitelisted =
@@ -357,25 +357,13 @@ let should_report (issue_kind: Exceptions.err_kind) issue_type error_desc =>
             let reportable_issue_types =
               Localise.[
                 Localise.from_string Config.default_failure_name,
-                assign_pointer_warning,
-                bad_pointer_comparison,
-                component_factory_function,
-                component_initializer_with_side_effects,
-                component_with_multiple_factory_methods,
-                component_with_unconventional_superclass,
                 context_leak,
-                cxx_reference_captured_in_objc_block,
-                direct_atomic_property_access,
                 empty_vector_access,
-                global_variable_initialized_with_function_or_method_call,
                 memory_leak,
-                mutable_local_variable_in_component_file,
                 quandary_taint_error,
-                registered_observer_being_deallocated,
                 resource_leak,
                 retain_cycle,
                 static_initialization_order_fiasco,
-                strong_delegate_warning,
                 tainted_value_reaching_sensitive_function,
                 thread_safety_error,
                 unsafe_guarded_by_access
@@ -422,7 +410,8 @@ let module IssuesCsv = {
         };
       if (
         in_footprint &&
-        error_filter source_file error_desc error_name && should_report ekind error_name error_desc
+        error_filter source_file error_desc error_name &&
+        should_report ekind error_name error_desc eclass
       ) {
         let err_desc_string = error_desc_to_csv_string error_desc;
         let err_advice_string = error_advice_to_csv_string error_desc;
@@ -498,7 +487,7 @@ let module IssuesJson = {
       if (
         in_footprint &&
         error_filter source_file error_desc error_name &&
-        should_report_source_file && should_report ekind error_name error_desc
+        should_report_source_file && should_report ekind error_name error_desc eclass
       ) {
         let kind = Exceptions.err_kind_string ekind;
         let bug_type = Localise.to_string error_name;

infer/src/clang/ComponentKit.ml

@@ -118,6 +118,7 @@ let mutable_local_vars_advice context an =
         CTL.True, Some {
           CIssue.name = Localise.to_string Localise.mutable_local_variable_in_component_file;
           severity = Exceptions.Kadvice;
+          mode = CIssue.On;
           description = "Local variable '" ^ named_decl_info.ni_name
                         ^ "' should be const to avoid reassignment";
           suggestion = Some "Add a const (after the asterisk for pointer types).";
@@ -145,6 +146,7 @@ let component_factory_function_advice context an =
         CTL.True, Some {
           CIssue.name = Localise.to_string Localise.component_factory_function;
           severity = Exceptions.Kadvice;
+          mode = CIssue.On;
           description = "Break out composite components";
           suggestion = Some (
               "Prefer subclassing CKCompositeComponent to static helper functions \
@@ -186,6 +188,7 @@ let component_with_unconventional_superclass_advice context an =
             CTL.True, Some {
               CIssue.name = Localise.to_string Localise.component_with_unconventional_superclass;
               severity = Exceptions.Kadvice;
+              mode = CIssue.On;
               description = "Never Subclass Components";
               suggestion = Some (
                   "Instead, create a new subclass of CKCompositeComponent."
@@ -237,6 +240,7 @@ let component_with_multiple_factory_methods_advice context an =
           CTL.True, Some {
             CIssue.name = Localise.to_string Localise.component_with_multiple_factory_methods;
             severity = Exceptions.Kadvice;
+            mode = CIssue.On;
             description = "Avoid Overrides";
             suggestion =
               Some "Instead, always expose all parameters in a single \
@@ -291,6 +295,7 @@ let rec _component_initializer_with_side_effects_advice
              CTL.True, Some {
                CIssue.name = Localise.to_string Localise.component_initializer_with_side_effects;
                severity = Exceptions.Kadvice;
+               mode = CIssue.On;
                description = "No Side-effects";
                suggestion = Some "Your +new method should not modify any \
                                   global variables or global state.";
@@ -324,6 +329,7 @@ let component_file_line_count_info (context: CLintersContext.context) dec =
       IList.map (fun i -> {
             CIssue.name = Localise.to_string Localise.component_file_line_count;
             severity = Exceptions.Kinfo;
+            mode = CIssue.Off;
             description = "Line count analytics";
             suggestion = None;
             loc = {
@@ -369,6 +375,7 @@ let component_file_cyclomatic_complexity_info (context: CLintersContext.context)
       CTL.True, Some {
         CIssue.name = Localise.to_string Localise.component_file_cyclomatic_complexity;
         severity = Exceptions.Kinfo;
+        mode = CIssue.Off;
         description = "Cyclomatic Complexity Incremental Marker";
         suggestion = None;
         loc = loc

infer/src/clang/cFrontend_checkers.ml

@@ -121,6 +121,7 @@ let ctl_ns_notification_warning lctx an =
   let issue_desc = {
     CIssue.name = Localise.to_string Localise.registered_observer_being_deallocated;
     severity = Exceptions.Kwarning;
+    mode = CIssue.On;
     description =
       "Object self is registered in a notification center but not being removed before deallocation";
     suggestion =
@@ -155,6 +156,7 @@ let ctl_bad_pointer_comparison_warning lctx an =
     { CIssue.
       name = Localise.to_string Localise.bad_pointer_comparison;
       severity = Exceptions.Kwarning;
+      mode = CIssue.On;
       description = "Implicitly checking whether NSNumber pointer is nil";
       suggestion =
         Some ("Did you mean to compare against the unboxed value instead? " ^
@@ -179,6 +181,7 @@ let ctl_strong_delegate_warning lctx an =
   let issue_desc = {
     CIssue.name = Localise.to_string Localise.strong_delegate_warning;
     severity = Exceptions.Kwarning;
+    mode = CIssue.On;
     description =
       "Property or ivar %decl_name% declared strong";
     suggestion = Some "In general delegates should be declared weak or assign";
@@ -202,6 +205,7 @@ let ctl_global_var_init_with_calls_warning lctx an =
     CIssue.name =
       Localise.to_string Localise.global_variable_initialized_with_function_or_method_call;
     severity = Exceptions.Kwarning;
+    mode = CIssue.On;
     description =
       "Global variable %decl_name% is initialized using a function or method call";
     suggestion = Some
@@ -219,6 +223,7 @@ let ctl_assign_pointer_warning lctx an =
   let issue_desc =
     { CIssue.name = Localise.to_string Localise.assign_pointer_warning;
       severity = Exceptions.Kwarning;
+      mode = CIssue.On;
       description =
         "Property `%decl_name%` is a pointer type marked with the `assign` attribute";
       suggestion = Some "Use a different attribute like `strong` or `weak`.";
@@ -241,6 +246,7 @@ let ctl_direct_atomic_property_access_warning lctx an =
   let issue_desc = {
     CIssue.name = Localise.to_string Localise.direct_atomic_property_access;
     severity = Exceptions.Kwarning;
+    mode = CIssue.On;
     description = "Direct access to ivar %ivar_name% of an atomic property";
     suggestion =
       Some "Accessing an ivar of an atomic property makes the property nonatomic";
@@ -255,6 +261,7 @@ let ctl_captured_cxx_ref_in_objc_block_warning lctx an  =
   let issue_desc = {
     CIssue.name = Localise.to_string Localise.cxx_reference_captured_in_objc_block;
     severity = Exceptions.Kwarning;
+    mode = CIssue.On;
     description =
       "C++ Reference variable(s) %var_name% captured by Objective-C block";
     suggestion = Some ("C++ References are unmanaged and may be invalid " ^

infer/src/clang/cFrontend_errors.ml

@@ -110,8 +110,6 @@ let expand_checkers checkers =
   let expanded_checkers = IList.map expand_one_checker checkers in
   expanded_checkers
 
-
-
 let get_err_log translation_unit_context method_decl_opt =
   let procname = match method_decl_opt with
     | Some method_decl -> General_utils.procname_of_decl translation_unit_context method_decl
@@ -139,14 +137,15 @@ let invoke_set_of_checkers_an an context =
     | CTL.Stmt st -> stmt_checkers_list, Ast_utils.generate_key_stmt st in
   IList.iter (fun checker ->
       let condition, issue_desc_opt = checker context an in
-      match  CTL.eval_formula condition an context, issue_desc_opt with
+      match issue_desc_opt with
-      | true, Some issue_desc ->
+      | Some issue_desc ->
-
+          if CIssue.should_run_check issue_desc.CIssue.mode &&
-          let desc' = expand_message_string issue_desc.CIssue.description an in
+             CTL.eval_formula condition an context then
-          let issue_desc' = {issue_desc with CIssue.description = desc'} in
+            let desc' = expand_message_string issue_desc.CIssue.description an in
-          log_frontend_issue context.CLintersContext.translation_unit_context
+            let issue_desc' = {issue_desc with CIssue.description = desc'} in
-            context.CLintersContext.current_method key issue_desc'
+            log_frontend_issue context.CLintersContext.translation_unit_context
-      | _, _ -> ()) checkers
+              context.CLintersContext.current_method key issue_desc'
+      | None -> ()) checkers
 
 
 let run_frontend_checkers_on_an (context: CLintersContext.context) an =
@@ -164,7 +163,8 @@ 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 ->
-          let key = Ast_utils.generate_key_decl dec in
+          if (CIssue.should_run_check issue_desc.CIssue.mode) then
-          log_frontend_issue context.CLintersContext.translation_unit_context
+            let key = Ast_utils.generate_key_decl dec in
-            context.CLintersContext.current_method key issue_desc
+            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/cIssue.ml

@@ -9,10 +9,18 @@
 
 open! IStd
 
+type mode = On | Off
+
 type issue_desc = {
   name : string; (* issue name *)
-  severity: Exceptions.err_kind;
+  severity : Exceptions.err_kind;
+  mode : mode;
   description : string; (* Description in the error message *)
   suggestion : string option; (* an optional suggestion or correction *)
   loc : Location.t; (* location in the code *)
 }
+
+let should_run_check mode =
+  match mode with
+  | On -> true
+  | Off -> Config.debug_mode || Config.debug_exceptions || not Config.filtering

infer/src/clang/cIssue.mli

@@ -9,10 +9,15 @@
 
 open! IStd
 
+type mode = On | Off
+
 type issue_desc = {
   name : string; (* issue name *)
   severity : Exceptions.err_kind;
+  mode : mode;
   description : string; (* Description in the error message *)
   suggestion : string option; (* an optional suggestion or correction *)
   loc : Location.t; (* location in the code *)
 }
+
+val should_run_check : mode -> bool

infer/tests/build_systems/run_hidden_linters/Makefile

@@ -0,0 +1,23 @@
+# 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.
+
+# this test checks that hidden linters do not run without --no-filtering,
+# regardless of what other flags are used to enable them (e.g. --compute-analytics)
+
+TESTS_DIR = ../..
+
+ANALYZER = linters
+
+CODETOANALYZE_DIR = ../codetoanalyze/componentkit
+
+CLANG_OPTIONS = -x objective-c++ -std=c++11 -c -fblocks
+INFER_OPTIONS = --compute-analytics --project-root $(CODETOANALYZE_DIR)
+INFERPRINT_OPTIONS = --issues-tests
+
+SOURCES = $(CODETOANALYZE_DIR)/TestComponentKitAnalytics.mm
+
+include $(TESTS_DIR)/clang.make