From 4769c481ada22f695b3c2de83f5b6119ae286001 Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Fri, 6 Oct 2017 10:07:44 -0700 Subject: [PATCH] [linters] Add a linter for checking const pointers to Objective-C classes Reviewed By: jvillard Differential Revision: D5985910 fbshipit-source-id: 9abbb78 --- infer/lib/linter_rules/linters.al | 12 +++++++++ infer/src/clang/cFrontend_checkers.ml | 27 +++++++++++++++++++ infer/src/clang/cFrontend_checkers.mli | 2 ++ infer/src/clang/cFrontend_errors.ml | 16 ++++++++--- infer/src/clang/cFrontend_errors.mli | 2 +- infer/src/clang/cPredicates.ml | 15 +++++++++++ infer/src/clang/cPredicates.mli | 6 +++++ infer/src/clang/cTL.ml | 4 +++ infer/src/unit/clang/CFrontend_errorsTests.ml | 4 +-- .../linters/Pointer_to_const_objc_class.m | 17 ++++++++++++ .../codetoanalyze/objc/linters/issues.exp | 3 +++ .../codetoanalyze/objcpp/linters/issues.exp | 2 ++ 12 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 infer/tests/codetoanalyze/objc/linters/Pointer_to_const_objc_class.m diff --git a/infer/lib/linter_rules/linters.al b/infer/lib/linter_rules/linters.al index dbc4ddea9..002b99dce 100644 --- a/infer/lib/linter_rules/linters.al +++ b/infer/lib/linter_rules/linters.al @@ -244,3 +244,15 @@ DEFINE-CHECKER POINTER_TO_INTEGRAL_IMPLICIT_CAST = { SET message = "Implicit conversion from %child_type% to %type% in usage of %name%"; SET doc_url = "https://clang.llvm.org/docs/DiagnosticsReference.html#wint-conversion"; }; + +DEFINE-CHECKER POINTER_TO_CONST_OBJC_CLASS = { + SET name = "Pointer To const Objective-C Class"; + SET report_when = is_decl() AND has_type_const_ptr_to_objc_class(); + SET message = "`const %class_name%*` may not mean what you want: + it represents a mutable pointer pointing to an Objective-C + class where the ivars cannot be changed."; + SET suggestion = "Consider using `%class_name% *const` instead, meaning + the destination of the pointer cannot be changed."; + SET severity = "WARNING"; + SET mode = "OFF"; +}; diff --git a/infer/src/clang/cFrontend_checkers.ml b/infer/src/clang/cFrontend_checkers.ml index 7966c0c38..4d9b38c13 100644 --- a/infer/src/clang/cFrontend_checkers.ml +++ b/infer/src/clang/cFrontend_checkers.ml @@ -118,3 +118,30 @@ let cxx_ref_captured_in_block an = in let var_desc vars var_named_decl_info = vars ^ "'" ^ var_named_decl_info.ni_name ^ "'" in List.fold ~f:var_desc ~init:"" capt_refs + +let class_name node = + let open Clang_ast_t in + let class_name_of_interface_type typ = + match typ with + | Some ObjCInterfaceType (_, ptr) -> ( + match CAst_utils.get_decl ptr with + | Some ObjCInterfaceDecl (_, ndi, _, _, _) + -> ndi.ni_name + | _ + -> "" ) + | _ + -> "" + in + match CPredicates.get_ast_node_type_ptr node with + | Some type_ptr + -> ( + let typ = CAst_utils.get_desugared_type type_ptr in + match typ with + | Some ObjCObjectPointerType (_, {Clang_ast_t.qt_type_ptr}) + -> class_name_of_interface_type (CAst_utils.get_desugared_type qt_type_ptr) + | Some ObjCInterfaceType _ + -> class_name_of_interface_type typ + | _ + -> "" ) + | _ + -> "" diff --git a/infer/src/clang/cFrontend_checkers.mli b/infer/src/clang/cFrontend_checkers.mli index 210a66f41..204e420c5 100644 --- a/infer/src/clang/cFrontend_checkers.mli +++ b/infer/src/clang/cFrontend_checkers.mli @@ -32,3 +32,5 @@ val class_available_ios_sdk : Ctl_parser_types.ast_node -> string val receiver_method_call : Ctl_parser_types.ast_node -> string val tag_name_of_node : Ctl_parser_types.ast_node -> string + +val class_name : Ctl_parser_types.ast_node -> string diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 4cf6549bb..cfb7f277d 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -113,6 +113,8 @@ let evaluate_place_holder context ph an = -> MF.monospaced_to_string (CFrontend_checkers.class_available_ios_sdk an) | "%type%" -> MF.monospaced_to_string (Ctl_parser_types.ast_node_type an) + | "%class_name%" + -> CFrontend_checkers.class_name an | "%child_type%" -> MF.monospaced_to_string (Ctl_parser_types.stmt_node_child_type an) | "%name%" @@ -141,7 +143,9 @@ let rec expand_message_string context message an = expand_message_string context message' an with Not_found -> message -let remove_new_lines message = String.substr_replace_all ~pattern:"\n" ~with_:" " message +let remove_new_lines_and_whitespace message = + let words = List.map ~f:String.strip (String.split ~on:'\n' message) in + String.concat words ~sep:" " let string_to_err_kind = function | "WARNING" @@ -438,9 +442,13 @@ let log_frontend_issue translation_unit_context method_decl_opt (node: Ctl_parse Reporting.log_issue_from_errlog err_kind errlog exn ~loc:issue_desc.loc ~ltr:trace ~node_id:(0, key) ?linters_def_file ?doc_url:issue_desc.doc_url -let fill_issue_desc_info_and_log context an issue_desc linters_def_file loc = - let desc = remove_new_lines (expand_message_string context issue_desc.CIssue.description an) in - let issue_desc' = {issue_desc with CIssue.description= desc; CIssue.loc= loc} in +let fill_issue_desc_info_and_log context an (issue_desc: CIssue.issue_desc) linters_def_file loc = + let process_message message = + remove_new_lines_and_whitespace (expand_message_string context message an) + in + let description = process_message issue_desc.description in + let suggestion = Option.map ~f:process_message issue_desc.suggestion in + let issue_desc' = {issue_desc with description; loc; suggestion} in log_frontend_issue context.CLintersContext.translation_unit_context context.CLintersContext.current_method an issue_desc' linters_def_file diff --git a/infer/src/clang/cFrontend_errors.mli b/infer/src/clang/cFrontend_errors.mli index bab0ca7dc..dbfb23d6e 100644 --- a/infer/src/clang/cFrontend_errors.mli +++ b/infer/src/clang/cFrontend_errors.mli @@ -48,7 +48,7 @@ val expand_checkers : macros_map -> paths_map -> CTL.ctl_checker list -> CTL.ctl val create_parsed_linters : string -> CTL.ctl_checker list -> linter list -val remove_new_lines : string -> string +val remove_new_lines_and_whitespace : string -> string val fill_issue_desc_info_and_log : CLintersContext.context -> Ctl_parser_types.ast_node -> CIssue.issue_desc -> string option diff --git a/infer/src/clang/cPredicates.ml b/infer/src/clang/cPredicates.ml index b58d18dd1..aab846829 100644 --- a/infer/src/clang/cPredicates.ml +++ b/infer/src/clang/cPredicates.ml @@ -608,6 +608,21 @@ let has_type an _typ = | _ -> false +let has_type_const_ptr_to_objc_class node = + let open Clang_ast_t in + match get_ast_node_type_ptr node with + | Some type_ptr -> ( + match CAst_utils.get_desugared_type type_ptr with + | Some ObjCObjectPointerType (_, qt) + -> qt.qt_is_const + | _ + -> false ) + | None + -> false + +let is_decl node = + match node with Ctl_parser_types.Decl _ -> true | Ctl_parser_types.Stmt _ -> false + let method_return_type an _typ = L.(debug Linters Verbose) "@\n Executing method_return_type..." ; match (an, _typ) with diff --git a/infer/src/clang/cPredicates.mli b/infer/src/clang/cPredicates.mli index 46a796a58..db93dd375 100644 --- a/infer/src/clang/cPredicates.mli +++ b/infer/src/clang/cPredicates.mli @@ -128,3 +128,9 @@ val iphoneos_target_sdk_version_by_path : CLintersContext.context -> string opti val iphoneos_target_sdk_version_greater_or_equal : CLintersContext.context -> string -> bool val within_available_class_block : CLintersContext.context -> Ctl_parser_types.ast_node -> bool + +val has_type_const_ptr_to_objc_class : Ctl_parser_types.ast_node -> bool + +val is_decl : Ctl_parser_types.ast_node -> bool + +val get_ast_node_type_ptr : Ctl_parser_types.ast_node -> Clang_ast_t.type_ptr option diff --git a/infer/src/clang/cTL.ml b/infer/src/clang/cTL.ml index b143841f5..cd126e802 100644 --- a/infer/src/clang/cTL.ml +++ b/infer/src/clang/cTL.ml @@ -894,6 +894,8 @@ let rec eval_Atomic _pred_name args an lcxt = -> CPredicates.is_class an cname | "is_const_var", [], an -> CPredicates.is_const_expr_var an + | "is_decl", [], an + -> CPredicates.is_decl an | "is_enum_constant", [cname], an -> CPredicates.is_enum_constant an cname | "is_enum_constant_of_enum", [name], an @@ -942,6 +944,8 @@ let rec eval_Atomic _pred_name args an lcxt = -> CPredicates.using_namespace an namespace | "is_at_selector_with_name", [name], an -> CPredicates.is_at_selector_with_name an name + | "has_type_const_ptr_to_objc_class", [], an + -> CPredicates.has_type_const_ptr_to_objc_class an | "has_type_subprotocol_of", [protname], an -> CPredicates.has_type_subprotocol_of an protname | "has_visibility_attribute", [vis], an diff --git a/infer/src/unit/clang/CFrontend_errorsTests.ml b/infer/src/unit/clang/CFrontend_errorsTests.ml index bcb86c92b..7beb57f58 100644 --- a/infer/src/unit/clang/CFrontend_errorsTests.ml +++ b/infer/src/unit/clang/CFrontend_errorsTests.ml @@ -15,12 +15,12 @@ let test_correct_removing_new_lines = Format.fprintf fmt "Expected: [%s] Found: [%s]" expected actual in let create_test (desc: string) (expected_desc: string) _ = - let output = CFrontend_errors.remove_new_lines desc in + let output = CFrontend_errors.remove_new_lines_and_whitespace desc in let cmp s1 s2 = String.equal s1 s2 in assert_equal ~pp_diff:pp_diff_of_desc ~cmp expected_desc output in [ ( "test_correct_removing_new_lines" - , "The selector m is not available in the required iOS SDK version\n8.0" + , "The selector m is not available in the required iOS SDK version \n 8.0" , "The selector m is not available in the required iOS SDK version 8.0" ) ] |> List.map ~f:(fun (name, test_input, expected_output) -> name >:: create_test test_input expected_output ) diff --git a/infer/tests/codetoanalyze/objc/linters/Pointer_to_const_objc_class.m b/infer/tests/codetoanalyze/objc/linters/Pointer_to_const_objc_class.m new file mode 100644 index 000000000..3a4db95ea --- /dev/null +++ b/infer/tests/codetoanalyze/objc/linters/Pointer_to_const_objc_class.m @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2017 - 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. + */ +#import + +@interface C : NSObject + +@property(strong) const C* ptr_to_const_objc_bad; + +@property(strong) C* const const_ptr_to_objc_good; + +@end diff --git a/infer/tests/codetoanalyze/objc/linters/issues.exp b/infer/tests/codetoanalyze/objc/linters/issues.exp index 30f0327e0..a37bcd1ee 100644 --- a/infer/tests/codetoanalyze/objc/linters/issues.exp +++ b/infer/tests/codetoanalyze/objc/linters/issues.exp @@ -1,3 +1,6 @@ +codetoanalyze/objc/linters/Pointer_to_const_objc_class.m, C_ptr_to_const_objc_bad, 13, POINTER_TO_CONST_OBJC_CLASS, [] +codetoanalyze/objc/linters/Pointer_to_const_objc_class.m, C_setPtr_to_const_objc_bad, 13, POINTER_TO_CONST_OBJC_CLASS, [] +codetoanalyze/objc/linters/Pointer_to_const_objc_class.m, Linters_dummy_method, 13, POINTER_TO_CONST_OBJC_CLASS, [] codetoanalyze/objc/linters/assign_pointer.m, Linters_dummy_method, 18, ASSIGN_POINTER_WARNING, [] codetoanalyze/objc/linters/assign_pointer.m, Linters_dummy_method, 20, ASSIGN_POINTER_WARNING, [] codetoanalyze/objc/linters/assign_pointer.m, Linters_dummy_method, 22, ASSIGN_POINTER_WARNING, [] diff --git a/infer/tests/codetoanalyze/objcpp/linters/issues.exp b/infer/tests/codetoanalyze/objcpp/linters/issues.exp index a5673d114..5592aea93 100644 --- a/infer/tests/codetoanalyze/objcpp/linters/issues.exp +++ b/infer/tests/codetoanalyze/objcpp/linters/issues.exp @@ -19,6 +19,8 @@ codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, BarCompo codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 52, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, [] codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 57, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, [] codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 60, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, [] +codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 60, POINTER_TO_CONST_OBJC_CLASS, [] +codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 61, POINTER_TO_CONST_OBJC_CLASS, [] codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 64, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, [] codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, FooComponent_newWithString, 69, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, [] codetoanalyze/objcpp/linters/componentkit/MutableLocalVariablesTest.mm, SomeClass_init, 40, MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE, []