[componentkit] Unconventional superclass rule

Reviewed By: dulmarod

Differential Revision: D3954639

fbshipit-source-id: 5d61c66

infer/src/IR/Localise.ml

@@ -42,6 +42,7 @@ let class_cast_exception = "CLASS_CAST_EXCEPTION"
 let comparing_floats_for_equality = "COMPARING_FLOAT_FOR_EQUALITY"
 let condition_is_assignment = "CONDITION_IS_ASSIGNMENT"
 let component_factory_function = "COMPONENT_FACTORY_FUNCTION"
+let component_with_unconventional_superclass = "COMPONENT_WITH_UNCONVENTIONAL_SUPERCLASS"
 let condition_always_false = "CONDITION_ALWAYS_FALSE"
 let condition_always_true = "CONDITION_ALWAYS_TRUE"
 let context_leak = "CONTEXT_LEAK"

infer/src/IR/Localise.mli

@@ -42,6 +42,7 @@ val condition_always_false : t
 val condition_always_true : t
 val context_leak : t
 val component_factory_function : t
+val component_with_unconventional_superclass : t
 val cxx_reference_captured_in_objc_block : t
 val dangling_pointer_dereference : t
 val deallocate_stack_variable : t

infer/src/backend/InferPrint.re

@@ -505,6 +505,7 @@ let module IssuesTests = {
             Localise.assign_pointer_warning,
             Localise.bad_pointer_comparison,
             Localise.component_factory_function,
+            Localise.component_with_unconventional_superclass,
             Localise.cxx_reference_captured_in_objc_block,
             Localise.direct_atomic_property_access,
             Localise.field_not_null_checked,

infer/src/clang/ComponentKit.ml

@@ -133,3 +133,53 @@ let component_factory_function_advice context decl =
         }
       else None
   | _ -> assert false (* Should only be called with FunctionDecl *)
+
+(** Components should not inherit from each other. They should instead
+    inherit from CKComponent, CKCompositeComponent, or
+    CKStatefulViewComponent. (Similar rule applies to component controllers.) *)
+let component_with_unconventional_superclass_advice context decl =
+  let check_interface if_decl =
+    match if_decl with
+    | Clang_ast_t.ObjCInterfaceDecl (_, _, _, _, _) ->
+        if is_component_or_controller_if (Some if_decl) then
+          let superclass_name = match Ast_utils.get_super_if (Some if_decl) with
+            | Some Clang_ast_t.ObjCInterfaceDecl (_, named_decl_info, _, _, _) ->
+                Some named_decl_info.ni_name
+            | _ -> None in
+          let has_conventional_superclass =
+            let open CFrontend_config in
+            match superclass_name with
+            | Some name when IList.mem string_equal name [
+                ckcomponent_cl;
+                ckcomponentcontroller_cl;
+                "CKCompositeComponent";
+                "CKStatefulViewComponent";
+                "CKStatefulViewComponentController"
+              ] -> true
+            | _ -> false in
+          let condition =
+            is_component_or_controller_if (Some if_decl)
+            && not has_conventional_superclass in
+          if condition then
+            Some {
+              CIssue.issue = CIssue.Component_with_unconventional_superclass;
+              CIssue.description = "Never Subclass Components";
+              CIssue.suggestion = Some (
+                  "Instead, create a new subclass of CKCompositeComponent."
+                );
+              CIssue.loc = CFrontend_checkers.location_from_decl context if_decl
+            }
+          else
+            None
+        else
+          None
+    | _ -> assert false in
+  match decl with
+  | Clang_ast_t.ObjCImplementationDecl (_, _, _, _, impl_decl_info) ->
+      let if_decl_opt =
+        Ast_utils.get_decl_opt_with_decl_ref impl_decl_info.oidi_class_interface in
+      if Option.is_some if_decl_opt && is_ck_context context decl then
+        check_interface (Option.get if_decl_opt)
+      else
+        None
+  | _ -> assert false

infer/src/clang/ComponentKit.mli

@@ -19,3 +19,6 @@ val mutable_local_vars_advice :
 
 val component_factory_function_advice :
   CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc option
+
+val component_with_unconventional_superclass_advice :
+  CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc option

infer/src/clang/cFrontend_checkers.mli

@@ -45,3 +45,6 @@ val global_var_init_with_calls_warning :
 
 val location_from_dinfo :
   CLintersContext.context -> Clang_ast_t.decl_info -> Location.t
+
+val location_from_decl :
+  CLintersContext.context -> Clang_ast_t.decl -> Location.t

infer/src/clang/cFrontend_errors.ml

@@ -33,11 +33,19 @@ let captured_vars_checker_list = [CFrontend_checkers.captured_cxx_ref_in_objc_bl
 let checkers_for_capture_vars stmt checker context =
   checker context stmt
 
-(* List of checkers on NSNotificationCenter *)
+(* List of checkers on ObjCProtocol decls *)
-let ns_notification_checker_list = [CFrontend_checkers.checker_NSNotificationCenter]
+let objc_protocol_checker_list = [CFrontend_checkers.checker_NSNotificationCenter]
 
-(* Invocation of checker belonging to ns_notification_center_list *)
+(* Invocation of checker belonging to objc_protocol_checker_list *)
-let checkers_for_ns decl checker context =
+let checkers_for_objc_protocol decl checker context =
+  checker context decl
+
+(* List of checkers running on ObjCImpl decls *)
+let objc_impl_checker_list = [CFrontend_checkers.checker_NSNotificationCenter;
+                              ComponentKit.component_with_unconventional_superclass_advice]
+
+(* Invocation of checker belonging to objc_impl_checker_list *)
+let checkers_for_objc_impl decl checker context =
   checker context decl
 
 (* List of checkers on variables *)
@@ -162,10 +170,15 @@ let run_frontend_checkers_on_stmt context instr =
 let run_frontend_checkers_on_decl context dec =
   let open Clang_ast_t in
   match dec with
-  | ObjCImplementationDecl _ | ObjCProtocolDecl _ ->
+  | ObjCImplementationDecl _ ->
-      let call_ns_checker = checkers_for_ns dec in
+      let call_objc_impl_checker = checkers_for_objc_impl dec in
+      let key = Ast_utils.generate_key_decl dec in
+      invoke_set_of_checkers call_objc_impl_checker context key objc_impl_checker_list;
+      context
+  | ObjCProtocolDecl _ ->
+      let call_objc_protocol_checker = checkers_for_objc_protocol dec in
       let key = Ast_utils.generate_key_decl dec in
-      invoke_set_of_checkers call_ns_checker context key ns_notification_checker_list;
+      invoke_set_of_checkers call_objc_protocol_checker context key objc_protocol_checker_list;
       context
   | VarDecl _ ->
       let call_var_checker = checker_for_var dec in

infer/src/clang/cIssue.ml

@@ -11,6 +11,7 @@ type issue =
   | Assign_pointer_warning
   | Bad_pointer_comparison
   | Component_factory_function
+  | Component_with_unconventional_superclass
   | Cxx_reference_captured_in_objc_block
   | Direct_atomic_property_access
   | Global_variable_initialized_with_function_or_method_call
@@ -27,6 +28,8 @@ let to_string issue =
          Localise.bad_pointer_comparison
      | Component_factory_function ->
          Localise.component_factory_function
+     | Component_with_unconventional_superclass ->
+         Localise.component_with_unconventional_superclass
      | Cxx_reference_captured_in_objc_block ->
          Localise.cxx_reference_captured_in_objc_block
      | Direct_atomic_property_access ->
@@ -51,6 +54,7 @@ let severity_of_issue issue =
   | Registered_observer_being_deallocated
   | Strong_delegate_warning -> Exceptions.Kwarning
   | Component_factory_function
+  | Component_with_unconventional_superclass
   | Mutable_local_variable_in_component_file -> Exceptions.Kadvice
 
 

infer/src/clang/cIssue.mli

@@ -11,6 +11,7 @@ type issue =
   | Assign_pointer_warning
   | Bad_pointer_comparison
   | Component_factory_function
+  | Component_with_unconventional_superclass
   | Cxx_reference_captured_in_objc_block
   | Direct_atomic_property_access
   | Global_variable_initialized_with_function_or_method_call

infer/tests/codetoanalyze/objcpp/linters/issues.exp

@@ -9,6 +9,7 @@ componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString:, 55, MUTA
 componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString:, 59, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE
 componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString:, 64, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE
 componentkit/MutableLocalVariablesTest.mm, SomeClass_init, 37, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE
+componentkit/UnconventionalSuperclassTest.h, Linters_dummy_method, 17, COMPONENT_WITH_UNCONVENTIONAL_SUPERCLASS
 cxx_reference_in_block/block.mm, A_foo3:param2:, 37, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK
 cxx_reference_in_block/block.mm, A_foo:, 20, CXX_REFERENCE_CAPTURED_IN_OBJC_BLOCK
 global-var/B.mm, Linters_dummy_method, 30, GLOBAL_VARIABLE_INITIALIZED_WITH_FUNCTION_OR_METHOD_CALL