diff --git a/infer/src/clang/ComponentKit.ml b/infer/src/clang/ComponentKit.ml index 0de6a6a01..9b3f09729 100644 --- a/infer/src/clang/ComponentKit.ml +++ b/infer/src/clang/ComponentKit.ml @@ -48,3 +48,47 @@ let rec is_component_or_controller_descendant_impl decl = Does not recurse into hierarchy. *) and contains_ck_impl decl_list = IList.exists is_component_or_controller_descendant_impl decl_list + +(** An easy way to fix the component kit best practice + http://componentkit.org/docs/avoid-local-variables.html + + Local variables that are const or const pointers by definition cannot be + assigned to after declaration, which means the entire class of bugs stemming + from value mutation after assignment are gone. + + Note we want const pointers, not mutable pointers to const instances. + + OK: + + ``` + const int a; + int *const b; + NSString *const c; + const int *const d; + ``` + + Not OK: + + ``` + const int *z; + const NSString *y; + ``` *) +let mutable_local_vars_advice context decl = + let open CFrontend_utils.Ast_utils in + match decl with + | Clang_ast_t.VarDecl(decl_info, _, qual_type, _) -> + let condition = context.CLintersContext.is_ck_translation_unit + && is_in_main_file decl + && (is_objc () || is_objcpp ()) + && (not (is_global_var decl)) + && (not qual_type.is_const) in + if condition then + Some { + CIssue.issue = CIssue.Mutable_local_variable_in_component_file; + CIssue.description = "Local variables should be const to avoid reassignment"; + CIssue.suggestion = Some "Add a const (after the asterisk for pointer types). \ + http://componentkit.org/docs/avoid-local-variables.html"; + CIssue.loc = CFrontend_checkers.location_from_dinfo decl_info + } + else None + | _ -> assert false (* Should only be called with a VarDecl *) diff --git a/infer/src/clang/ComponentKit.mli b/infer/src/clang/ComponentKit.mli index 13944031a..1af27d769 100644 --- a/infer/src/clang/ComponentKit.mli +++ b/infer/src/clang/ComponentKit.mli @@ -13,3 +13,6 @@ Does not recurse into hierarchy. *) val contains_ck_impl : Clang_ast_t.decl list -> bool + +val mutable_local_vars_advice : + CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc option diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 7ba5d27d5..872257c08 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -49,3 +49,6 @@ val checker_NSNotificationCenter : (* of a program *) val global_var_init_with_calls_warning : CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc option + +val location_from_dinfo : + Clang_ast_t.decl_info -> Location.t diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 2bec5ec25..5ad7b3162 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -41,7 +41,8 @@ let checkers_for_ns decl_info impl_decl_info decls checker context = checker context decl_info impl_decl_info decls (* List of checkers on global variables *) -let global_var_checker_list = [CFrontend_checkers.global_var_init_with_calls_warning] +let global_var_checker_list = [CFrontend_checkers.global_var_init_with_calls_warning; + ComponentKit.mutable_local_vars_advice] (* Invocation of checker belonging to global_var_checker_list *) let checker_for_global_var dec checker context = diff --git a/infer/src/clang/cIssue.ml b/infer/src/clang/cIssue.ml index 71e1c8a27..5bfddae4d 100644 --- a/infer/src/clang/cIssue.ml +++ b/infer/src/clang/cIssue.ml @@ -13,6 +13,7 @@ type issue = | Cxx_reference_captured_in_objc_block | Direct_atomic_property_access | Global_variable_initialized_with_function_or_method_call + | Mutable_local_variable_in_component_file | Registered_observer_being_deallocated | Strong_delegate_warning @@ -25,6 +26,7 @@ let to_string issue = | Direct_atomic_property_access -> "DIRECT_ATOMIC_PROPERTY_ACCESS" | Global_variable_initialized_with_function_or_method_call -> "GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL" + | Mutable_local_variable_in_component_file -> "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE" | Registered_observer_being_deallocated -> Localise.to_string (Localise.registered_observer_being_deallocated) | Strong_delegate_warning -> "STRONG_DELEGATE_WARNING" @@ -36,6 +38,7 @@ let severity_of_issue issue = | Cxx_reference_captured_in_objc_block | Direct_atomic_property_access | Global_variable_initialized_with_function_or_method_call + | Mutable_local_variable_in_component_file -> Exceptions.Kadvice | Registered_observer_being_deallocated | Strong_delegate_warning -> Exceptions.Kwarning diff --git a/infer/src/clang/cIssue.mli b/infer/src/clang/cIssue.mli index b991a33dd..b00559396 100644 --- a/infer/src/clang/cIssue.mli +++ b/infer/src/clang/cIssue.mli @@ -13,6 +13,7 @@ type issue = | Cxx_reference_captured_in_objc_block | Direct_atomic_property_access | Global_variable_initialized_with_function_or_method_call + | Mutable_local_variable_in_component_file | Registered_observer_being_deallocated | Strong_delegate_warning