diff --git a/infer/src/IR/Localise.ml b/infer/src/IR/Localise.ml index d72532ab3..48e1d458c 100644 --- a/infer/src/IR/Localise.ml +++ b/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" diff --git a/infer/src/IR/Localise.mli b/infer/src/IR/Localise.mli index a9f955d4c..168515eec 100644 --- a/infer/src/IR/Localise.mli +++ b/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 diff --git a/infer/src/backend/InferPrint.re b/infer/src/backend/InferPrint.re index 013823a7b..347b19e9d 100644 --- a/infer/src/backend/InferPrint.re +++ b/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, diff --git a/infer/src/clang/ComponentKit.ml b/infer/src/clang/ComponentKit.ml index b66b0af42..ee065ed64 100644 --- a/infer/src/clang/ComponentKit.ml +++ b/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 diff --git a/infer/src/clang/ComponentKit.mli b/infer/src/clang/ComponentKit.mli index 124084d88..89119092c 100644 --- a/infer/src/clang/ComponentKit.mli +++ b/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 diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 00c68ff57..c430c6d11 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/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 diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 9892dacbf..5cf0a1253 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/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 *) -let ns_notification_checker_list = [CFrontend_checkers.checker_NSNotificationCenter] +(* List of checkers on ObjCProtocol decls *) +let objc_protocol_checker_list = [CFrontend_checkers.checker_NSNotificationCenter] -(* Invocation of checker belonging to ns_notification_center_list *) -let checkers_for_ns decl checker context = +(* Invocation of checker belonging to objc_protocol_checker_list *) +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 _ -> - let call_ns_checker = checkers_for_ns dec in + | ObjCImplementationDecl _ -> + 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 diff --git a/infer/src/clang/cIssue.ml b/infer/src/clang/cIssue.ml index acf774efb..13eb9b157 100644 --- a/infer/src/clang/cIssue.ml +++ b/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 diff --git a/infer/src/clang/cIssue.mli b/infer/src/clang/cIssue.mli index 3971440fb..76cdf4620 100644 --- a/infer/src/clang/cIssue.mli +++ b/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 diff --git a/infer/tests/codetoanalyze/objcpp/linters/issues.exp b/infer/tests/codetoanalyze/objcpp/linters/issues.exp index 149d2ac0e..72ca565bd 100644 --- a/infer/tests/codetoanalyze/objcpp/linters/issues.exp +++ b/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