From d044809b3270593e3bea1088dd17bb44bc0dde1e Mon Sep 17 00:00:00 2001 From: Martino Luca Date: Fri, 16 Dec 2016 08:45:43 -0800 Subject: [PATCH] [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 | 2 +- infer/src/IR/Exceptions.ml | 5 ++-- infer/src/IR/Exceptions.mli | 2 +- infer/src/backend/InferPrint.re | 21 ++++----------- infer/src/clang/ComponentKit.ml | 7 +++++ infer/src/clang/cFrontend_checkers.ml | 7 +++++ infer/src/clang/cFrontend_errors.ml | 26 +++++++++---------- infer/src/clang/cIssue.ml | 10 ++++++- infer/src/clang/cIssue.mli | 5 ++++ .../build_systems/run_hidden_linters/Makefile | 23 ++++++++++++++++ .../run_hidden_linters/issues.exp | 0 11 files changed, 74 insertions(+), 34 deletions(-) create mode 100644 infer/tests/build_systems/run_hidden_linters/Makefile create mode 100644 infer/tests/build_systems/run_hidden_linters/issues.exp diff --git a/Makefile b/Makefile index b1f4fd1b0..42e1a83af 100644 --- a/Makefile +++ b/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 diff --git a/infer/src/IR/Exceptions.ml b/infer/src/IR/Exceptions.ml index ef5f18fd2..03ef36c95 100644 --- a/infer/src/IR/Exceptions.ml +++ b/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 diff --git a/infer/src/IR/Exceptions.mli b/infer/src/IR/Exceptions.mli index 53f891cc7..f767114d9 100644 --- a/infer/src/IR/Exceptions.mli +++ b/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 diff --git a/infer/src/backend/InferPrint.re b/infer/src/backend/InferPrint.re index b72a0a902..e4b9847f2 100644 --- a/infer/src/backend/InferPrint.re +++ b/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 => - if (not Config.filtering) { +let should_report (issue_kind: Exceptions.err_kind) issue_type error_desc eclass => + 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; diff --git a/infer/src/clang/ComponentKit.ml b/infer/src/clang/ComponentKit.ml index ce1b1ebf4..36ff2c40d 100644 --- a/infer/src/clang/ComponentKit.ml +++ b/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 diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index da3db313c..110543155 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/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 " ^ diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index aa5a1f917..c416712d9 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/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 - | 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 + match issue_desc_opt with + | Some issue_desc -> + if CIssue.should_run_check issue_desc.CIssue.mode && + CTL.eval_formula condition an context then + 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' + | 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 - log_frontend_issue context.CLintersContext.translation_unit_context - context.CLintersContext.current_method key issue_desc + if (CIssue.should_run_check issue_desc.CIssue.mode) then + let key = Ast_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 diff --git a/infer/src/clang/cIssue.ml b/infer/src/clang/cIssue.ml index 1b2db4fc7..5f84b6772 100644 --- a/infer/src/clang/cIssue.ml +++ b/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 diff --git a/infer/src/clang/cIssue.mli b/infer/src/clang/cIssue.mli index 34dd7d668..9e85f3c28 100644 --- a/infer/src/clang/cIssue.mli +++ b/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 diff --git a/infer/tests/build_systems/run_hidden_linters/Makefile b/infer/tests/build_systems/run_hidden_linters/Makefile new file mode 100644 index 000000000..e9f58e8e9 --- /dev/null +++ b/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 diff --git a/infer/tests/build_systems/run_hidden_linters/issues.exp b/infer/tests/build_systems/run_hidden_linters/issues.exp new file mode 100644 index 000000000..e69de29bb