Mutable local vars

Reviewed By: jvillard

Differential Revision: D3693969

fbshipit-source-id: e2cbd63

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 *)

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

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

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 =

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
 

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