[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

@@ -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";
+};

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
+      | _
+       -> "" )
+  | _
+   -> ""

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

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
 

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

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

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

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

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 )

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 <Foundation/NSObject.h>
+
+@interface C : NSObject
+
+@property(strong) const C* ptr_to_const_objc_bad;
+
+@property(strong) C* const const_ptr_to_objc_good;
+
+@end

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, []

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, []