From 57dd45dd8237cb76b7ba41ebb0693de952b8d717 Mon Sep 17 00:00:00 2001 From: Dulma Churchill Date: Thu, 8 Jun 2017 06:35:08 -0700 Subject: [PATCH] [linters] Improve the error message in implicit cast error Reviewed By: jvillard Differential Revision: D5202360 fbshipit-source-id: 78611f1 --- infer/lib/linter_rules/linters.al | 2 +- infer/src/clang/cFrontend_errors.ml | 2 +- infer/src/clang/ctl_parser_types.ml | 145 +++++++++++------- infer/src/clang/ctl_parser_types.mli | 2 - .../objc/linters/implicit_cast.m | 31 ++++ .../codetoanalyze/objc/linters/issues.exp | 4 + 6 files changed, 123 insertions(+), 63 deletions(-) diff --git a/infer/lib/linter_rules/linters.al b/infer/lib/linter_rules/linters.al index 2cbe88070..0206b513c 100644 --- a/infer/lib/linter_rules/linters.al +++ b/infer/lib/linter_rules/linters.al @@ -228,5 +228,5 @@ DEFINE-CHECKER POINTER_TO_INTEGRAL_IMPLICIT_CAST = { SET report_when = WHEN has_cast_kind("PointerToIntegral") HOLDS-IN-NODE ImplicitCastExpr; - SET message = "Implicit conversion from %child_type% to %type% in usage of %eventual_child_name%"; + SET message = "Implicit conversion from %child_type% to %type% in usage of %name%"; }; diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index 244ca8ba2..68959978f 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -99,7 +99,7 @@ let evaluate_place_holder ph an = | "%available_ios_sdk%" -> MF.monospaced_to_string (CFrontend_checkers.available_ios_sdk an) | "%type%" -> MF.monospaced_to_string (Ctl_parser_types.ast_node_type an) | "%child_type%" -> MF.monospaced_to_string (Ctl_parser_types.stmt_node_child_type an) - | "%eventual_child_name%" -> MF.monospaced_to_string (Ctl_parser_types.eventual_child_name an) + | "%name%" -> MF.monospaced_to_string (Ctl_parser_types.ast_node_name an) | _ -> L.internal_error "ERROR: helper function %s is unknown. Stop.@\n" ph; assert false diff --git a/infer/src/clang/ctl_parser_types.ml b/infer/src/clang/ctl_parser_types.ml index 178c6e78b..8e096e16b 100644 --- a/infer/src/clang/ctl_parser_types.ml +++ b/infer/src/clang/ctl_parser_types.ml @@ -18,7 +18,7 @@ type ast_node = | Stmt of Clang_ast_t.stmt | Decl of Clang_ast_t.decl -let ast_node_name an = +let rec ast_node_name an = let open Clang_ast_t in match an with | Decl dec -> @@ -36,19 +36,36 @@ let ast_node_name an = ndi.ni_name | Stmt (ObjCMessageExpr (_, _, _, {omei_selector})) -> omei_selector - | _ -> "" - -let rec eventual_child_name an = - match an with - | Stmt stmt -> - (let _, stmts = Clang_ast_proj.get_stmt_tuple stmt in - match stmts with - | [stmt] -> - let name = ast_node_name (Stmt stmt) in - if String.length name > 0 then - name - else eventual_child_name (Stmt stmt) + | Stmt (IntegerLiteral (_, _, _, integer_literal_info)) -> + integer_literal_info.ili_value + | Stmt CStyleCastExpr (_, _, _, cast_expr_info, _) -> + (match cast_expr_info.cei_cast_kind with + | `NullToPointer -> "nil" | _ -> "") + | Stmt ObjCSubscriptRefExpr (_, [stmt; stmt_index], _, _) -> + (ast_node_name (Stmt stmt)) ^ "["^ (ast_node_name (Stmt stmt_index)) ^"]" + | Stmt OpaqueValueExpr (_, _, _, opaque_value_expr_info) -> + (match opaque_value_expr_info.ovei_source_expr with + | Some stmt -> ast_node_name (Stmt stmt) + | None -> "") + | Stmt ImplicitCastExpr (_, [stmt], _, _) + | Stmt PseudoObjectExpr (_, stmt::_, _) + | Stmt ParenExpr (_, [stmt], _) -> + ast_node_name (Stmt stmt) + | Stmt CallExpr (_, func::_, _) -> + let func_str = ast_node_name (Stmt func) in + func_str ^ "(...)" + | Stmt ObjCPropertyRefExpr (_, [stmt], _, obj_c_property_ref_expr_info) -> + let property_str = + (match obj_c_property_ref_expr_info.oprei_kind with + | `MethodRef obj_c_method_ref_info -> + (match obj_c_method_ref_info.mri_getter, obj_c_method_ref_info.mri_setter with + | Some name, _ -> name + | _, Some name -> name + | _ -> "") + | `PropertyRef decl_ref -> + match decl_ref.dr_name with Some name -> name.ni_name | None -> "") in + (ast_node_name (Stmt stmt)) ^ "." ^ property_str | _ -> "" let infer_prefix = "__infer_ctl_" @@ -87,11 +104,13 @@ type builtin_kind = | NullPtr (** nullptr_t *) | ObjCId (** id *) | ObjCClass (** Class *) - | ObjCSel (** SEL *) + | ObjCSel (** SEL *)[@@deriving compare] (* | OCLSampler | OCLEvent | OCLClkEvent | OCLQueue | OCLNDRange | OCLReserveID | Dependent | Overload | BoundMember | PseudoObject | UnknownAny | BuiltinFn | ARCUnbridgedCast | OMPArraySection *) +let equal_builtin_kind = [%compare.equal : builtin_kind] + let builtin_kind_to_string t = match t with | Char_U -> "char" @@ -139,40 +158,43 @@ let rec abs_ctype_to_string t = | Pointer t' -> "Pointer (" ^ (abs_ctype_to_string t') ^ ")" | TypeName ae -> "TypeName (" ^ (ALVar.alexp_to_string ae) ^ ")" +let builtin_type_kind_assoc = + [ + (`Char_U, Char_U); + (`Char_S, Char_U); + (`Char16, Char16); + (`Char32, Char32); + (`WChar_U, WChar_U); + (`WChar_S, WChar_U); + (`Bool, Bool); + (`Short, Short); + (`Int, Int); + (`Long, Long); + (`Float, Float); + (`Double, Double); + (`Void, Void); + (`SChar, SChar); + (`LongLong, LongLong); + (`UChar, UChar); + (`UShort, UShort); + (`UInt, UInt); + (`ULong, ULong); + (`ULongLong, ULongLong); + (`LongDouble, LongDouble); + (`Int128, Int128); + (`UInt128, UInt128); + (`Float128, Float128); + (`NullPtr, NullPtr); + (`ObjCId, ObjCId); + (`ObjCClass, ObjCClass); + (`ObjCSel, ObjCSel); + (`Half, Half) + ] -let builtin_equal bi abi = - match bi, abi with - | `Char_U, Char_U - | `Char_S, Char_U - | `Char16, Char16 - | `Char32, Char32 - | `WChar_U, WChar_U - | `WChar_S, WChar_U - | `Bool, Bool - | `Short, Short - | `Int, Int - | `Long, Long - | `Float, Float - | `Double, Double - | `Void, Void - | `SChar, SChar - | `LongLong, LongLong - | `UChar, UChar - | `UShort, UShort - | `UInt, UInt - | `ULong, ULong - | `ULongLong, ULongLong - | `LongDouble, LongDouble - | `Int128, Int128 - | `UInt128, UInt128 - | `Float128, Float128 - | `NullPtr, NullPtr - | `ObjCId, ObjCId - | `ObjCClass, ObjCClass - | `ObjCSel, ObjCSel - | `Half, Half -> true - | _, _ -> display_equality_warning (); - false +let builtin_equal (bi : Clang_ast_t.builtin_type_kind) (abi : builtin_kind) = + match List.Assoc.find ~equal:PVariant.(=) builtin_type_kind_assoc bi with + | Some assoc_abi when equal_builtin_kind assoc_abi abi -> true + | _ -> display_equality_warning ();false let typename_to_string pointer = match CAst_utils.get_decl pointer with @@ -228,7 +250,9 @@ let rec typ_string_of_type_ptr type_ptr = let open Clang_ast_t in match CAst_utils.get_type type_ptr with | Some BuiltinType (_, bt) -> - Clang_ast_j.string_of_builtin_type_kind bt + (match List.Assoc.find builtin_type_kind_assoc bt with + | Some abt -> builtin_kind_to_string abt + | None -> "") | Some PointerType (_, qt) | Some ObjCObjectPointerType (_, qt) -> (typ_string_of_type_ptr qt.qt_type_ptr) ^ "*" @@ -239,17 +263,20 @@ let rec typ_string_of_type_ptr type_ptr = | _ -> "" let ast_node_type an = - match an with - | Stmt stmt -> - (match Clang_ast_proj.get_expr_tuple stmt with - | Some (_, _, expr_info) -> - typ_string_of_type_ptr expr_info.ei_qual_type.qt_type_ptr - | _ -> "") - | Decl decl -> - (match CAst_utils.type_of_decl decl with - | Some type_ptr -> - typ_string_of_type_ptr type_ptr - | _ -> "") + let typ_str = + match an with + | Stmt stmt -> + (match Clang_ast_proj.get_expr_tuple stmt with + | Some (_, _, expr_info) -> + typ_string_of_type_ptr expr_info.ei_qual_type.qt_type_ptr + | _ -> "") + | Decl decl -> + (match CAst_utils.type_of_decl decl with + | Some type_ptr -> + typ_string_of_type_ptr type_ptr + | _ -> "") in + if String.length typ_str > 0 then typ_str + else "" let stmt_node_child_type an = match an with diff --git a/infer/src/clang/ctl_parser_types.mli b/infer/src/clang/ctl_parser_types.mli index 024e425d2..42716eaf6 100644 --- a/infer/src/clang/ctl_parser_types.mli +++ b/infer/src/clang/ctl_parser_types.mli @@ -18,8 +18,6 @@ type ast_node = val ast_node_name : ast_node -> string -val eventual_child_name : ast_node -> string - val ast_node_type : ast_node -> string val stmt_node_child_type : ast_node -> string diff --git a/infer/tests/codetoanalyze/objc/linters/implicit_cast.m b/infer/tests/codetoanalyze/objc/linters/implicit_cast.m index 5212f298f..94d7a85b6 100644 --- a/infer/tests/codetoanalyze/objc/linters/implicit_cast.m +++ b/infer/tests/codetoanalyze/objc/linters/implicit_cast.m @@ -21,3 +21,34 @@ void calling_funct_with_pointer1() { NSString* s = @"Dulma"; func_with_integer_param(s); } + +void integer_nil() { NSInteger n = nil; } + +NSString* returns_s() { return @""; } + +void calling_funct_with_pointer_from_a_method() { + func_with_integer_param(returns_s()); +} + +@interface Implicit_cast : NSObject + +@property NSString* p; + +@end + +extern NSString* const key; + +@implementation Implicit_cast { + + NSDictionary* _userInfo; +} + +- (void)ivar_dictionary_item_call_funct_with_int { + func_with_integer_param(_userInfo[key]); +} + +- (void)property_with_int { + func_with_integer_param(self.p); +} + +@end diff --git a/infer/tests/codetoanalyze/objc/linters/issues.exp b/infer/tests/codetoanalyze/objc/linters/issues.exp index 708049141..133f4b351 100644 --- a/infer/tests/codetoanalyze/objc/linters/issues.exp +++ b/infer/tests/codetoanalyze/objc/linters/issues.exp @@ -23,8 +23,12 @@ codetoanalyze/objc/linters/badpointer.m, bad6, 106, BAD_POINTER_COMPARISON, [] codetoanalyze/objc/linters/badpointer.m, bad7, 121, BAD_POINTER_COMPARISON, [] codetoanalyze/objc/linters/badpointer.m, bad8, 128, BAD_POINTER_COMPARISON, [] codetoanalyze/objc/linters/badpointer.m, bad9, 135, BAD_POINTER_COMPARISON, [] +codetoanalyze/objc/linters/implicit_cast.m, Implicit_cast_ivar_dictionary_item_call_funct_with_int, 47, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] +codetoanalyze/objc/linters/implicit_cast.m, Implicit_cast_property_with_int, 51, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] codetoanalyze/objc/linters/implicit_cast.m, calling_funct_with_pointer, 17, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] codetoanalyze/objc/linters/implicit_cast.m, calling_funct_with_pointer1, 22, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] +codetoanalyze/objc/linters/implicit_cast.m, calling_funct_with_pointer_from_a_method, 30, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] +codetoanalyze/objc/linters/implicit_cast.m, integer_nil, 25, POINTER_TO_INTEGRAL_IMPLICIT_CAST, [] codetoanalyze/objc/linters/nsnumber.m, bad1, 13, BAD_POINTER_COMPARISON, [] codetoanalyze/objc/linters/nsnumber.m, bad2, 18, BAD_POINTER_COMPARISON, [] codetoanalyze/objc/linters/nsnumber.m, bad3, 23, BAD_POINTER_COMPARISON, []