[clang] Catch a few more occurring incorrect assumptions

Reviewed By: avarun42

Differential Revision: D6976263

fbshipit-source-id: deec623
master
Dulma Churchill 7 years ago committed by Facebook Github Bot
parent fbf0020399
commit c38052bbaa

@ -90,67 +90,75 @@ and contains_ck_impl decl_list =
const NSString *y; const NSString *y;
``` *) ``` *)
let mutable_local_vars_advice context an = let mutable_local_vars_advice context an =
let rec get_referenced_type (qual_type: Clang_ast_t.qual_type) : Clang_ast_t.decl option = try
let typ_opt = CAst_utils.get_desugared_type qual_type.qt_type_ptr in let rec get_referenced_type (qual_type: Clang_ast_t.qual_type) : Clang_ast_t.decl option =
match (typ_opt : Clang_ast_t.c_type option) with let typ_opt = CAst_utils.get_desugared_type qual_type.qt_type_ptr in
| Some ObjCInterfaceType (_, decl_ptr) | Some RecordType (_, decl_ptr) -> match (typ_opt : Clang_ast_t.c_type option) with
CAst_utils.get_decl decl_ptr | Some ObjCInterfaceType (_, decl_ptr) | Some RecordType (_, decl_ptr) ->
| Some PointerType (_, inner_qual_type) CAst_utils.get_decl decl_ptr
| Some ObjCObjectPointerType (_, inner_qual_type) | Some PointerType (_, inner_qual_type)
| Some LValueReferenceType (_, inner_qual_type) -> | Some ObjCObjectPointerType (_, inner_qual_type)
get_referenced_type inner_qual_type | Some LValueReferenceType (_, inner_qual_type) ->
| _ -> get_referenced_type inner_qual_type
None | _ ->
in None
let is_of_whitelisted_type qual_type =
let cpp_whitelist =
[ "CKComponentScope"
; "FBTrackingNodeScope"
; "FBTrackingCodeScope"
; "CKComponentContext"
; "CKComponentKey" ]
in in
let objc_whitelist = ["NSError"] in let is_of_whitelisted_type qual_type =
match get_referenced_type qual_type with let cpp_whitelist =
| Some CXXRecordDecl (_, ndi, _, _, _, _, _, _) -> [ "CKComponentScope"
List.mem ~equal:String.equal cpp_whitelist ndi.ni_name ; "FBTrackingNodeScope"
| Some ObjCInterfaceDecl (_, ndi, _, _, _) -> ; "FBTrackingCodeScope"
List.mem ~equal:String.equal objc_whitelist ndi.ni_name ; "CKComponentContext"
| _ -> ; "CKComponentKey" ]
false
in
match an with
| Ctl_parser_types.Decl (Clang_ast_t.VarDecl (decl_info, named_decl_info, qual_type, _) as decl) ->
let is_const_ref =
match CAst_utils.get_type qual_type.qt_type_ptr with
| Some LValueReferenceType (_, {Clang_ast_t.qt_is_const}) ->
qt_is_const
| _ ->
false
in
let is_const = qual_type.qt_is_const || is_const_ref in
let condition =
is_ck_context context an && not (CAst_utils.is_syntactically_global_var decl)
&& not (CAst_utils.is_static_local_var decl) && not is_const
&& not (is_of_whitelisted_type qual_type) && not decl_info.di_is_implicit
&& not context.CLintersContext.in_for_loop_declaration
&& not (CAst_utils.is_std_vector qual_type) && not (CAst_utils.has_block_attribute decl)
in in
if condition then let objc_whitelist = ["NSError"] in
Some match get_referenced_type qual_type with
{ CIssue.id= "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE" | Some CXXRecordDecl (_, ndi, _, _, _, _, _, _) ->
; name= None List.mem ~equal:String.equal cpp_whitelist ndi.ni_name
; severity= Exceptions.Kadvice | Some ObjCInterfaceDecl (_, ndi, _, _, _) ->
; mode= CIssue.On List.mem ~equal:String.equal objc_whitelist ndi.ni_name
; description= | _ ->
"Local variable " ^ MF.monospaced_to_string named_decl_info.ni_name false
^ " should be const to avoid reassignment" in
; suggestion= Some "Add a const (after the asterisk for pointer types)." if is_ck_context context an then
; doc_url= None match an with
; loc= CFrontend_checkers.location_from_dinfo context decl_info } | Ctl_parser_types.Decl
else None (Clang_ast_t.VarDecl (decl_info, named_decl_info, qual_type, _) as decl) ->
| _ -> let is_const_ref =
None match CAst_utils.get_type qual_type.qt_type_ptr with
| Some LValueReferenceType (_, {Clang_ast_t.qt_is_const}) ->
qt_is_const
| _ ->
false
in
let is_const = qual_type.qt_is_const || is_const_ref in
let should_not_report_mutable_local =
CAst_utils.is_syntactically_global_var decl || CAst_utils.is_static_local_var decl
|| is_const || is_of_whitelisted_type qual_type || decl_info.di_is_implicit
|| context.CLintersContext.in_for_loop_declaration
|| CAst_utils.is_std_vector qual_type || CAst_utils.has_block_attribute decl
in
if should_not_report_mutable_local then None
else
Some
{ CIssue.id= "MUTABLE_LOCAL_VARIABLE_IN_COMPONENT_FILE"
; name= None
; severity= Exceptions.Kadvice
; mode= CIssue.On
; description=
"Local variable " ^ MF.monospaced_to_string named_decl_info.ni_name
^ " should be const to avoid reassignment"
; suggestion= Some "Add a const (after the asterisk for pointer types)."
; doc_url= None
; loc= CFrontend_checkers.location_from_dinfo context decl_info }
| _ ->
None
else None
with CFrontend_config.IncorrectAssumption e ->
let trans_unit_ctx = context.CLintersContext.translation_unit_context in
ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position
e.source_range e.ast_node ;
None
(* Should only be called with a VarDecl *) (* Should only be called with a VarDecl *)
@ -163,26 +171,27 @@ let component_factory_function_advice context an =
let is_component_if decl = let is_component_if decl =
CAst_utils.is_objc_if_descendant decl [CFrontend_config.ckcomponent_cl] CAst_utils.is_objc_if_descendant decl [CFrontend_config.ckcomponent_cl]
in in
match an with if is_ck_context context an then
| Ctl_parser_types.Decl match an with
Clang_ast_t.FunctionDecl (decl_info, _, (qual_type: Clang_ast_t.qual_type), _) -> | Ctl_parser_types.Decl
let objc_interface = CAst_utils.qual_type_to_objc_interface qual_type in Clang_ast_t.FunctionDecl (decl_info, _, (qual_type: Clang_ast_t.qual_type), _) ->
let condition = is_ck_context context an && is_component_if objc_interface in let objc_interface = CAst_utils.qual_type_to_objc_interface qual_type in
if condition then if is_component_if objc_interface then
Some Some
{ CIssue.id= "COMPONENT_FACTORY_FUNCTION" { CIssue.id= "COMPONENT_FACTORY_FUNCTION"
; name= None ; name= None
; severity= Exceptions.Kadvice ; severity= Exceptions.Kadvice
; mode= CIssue.Off ; mode= CIssue.Off
; description= "Break out composite components" ; description= "Break out composite components"
; suggestion= ; suggestion=
Some Some
"Prefer subclassing CKCompositeComponent to static helper functions that return a CKComponent subclass." "Prefer subclassing CKCompositeComponent to static helper functions that return a CKComponent subclass."
; doc_url= None ; doc_url= None
; loc= CFrontend_checkers.location_from_dinfo context decl_info } ; loc= CFrontend_checkers.location_from_dinfo context decl_info }
else None else None
| _ -> | _ ->
None None
else None
(* Should only be called with FunctionDecl *) (* Should only be called with FunctionDecl *)

@ -94,68 +94,77 @@ module CFrontend_decl_funct (T : CModule_type.CTranslation) : CModule_type.CFron
let function_decl trans_unit_ctx tenv cfg func_decl block_data_opt = let function_decl trans_unit_ctx tenv cfg func_decl block_data_opt =
let captured_vars, outer_context_opt = try
match block_data_opt with let captured_vars, outer_context_opt =
| Some (outer_context, _, _, captured_vars) -> match block_data_opt with
(captured_vars, Some outer_context) | Some (outer_context, _, _, captured_vars) ->
(captured_vars, Some outer_context)
| None ->
([], None)
in
let ms, body_opt, extra_instrs =
CMethod_trans.method_signature_of_decl trans_unit_ctx tenv func_decl block_data_opt
in
match body_opt with
| Some body ->
(* Only in the case the function declaration has a defined body we create a procdesc *)
let procname = CMethod_signature.ms_get_name ms in
let return_param_typ_opt = CMethod_signature.ms_get_return_param_typ ms in
if CMethod_trans.create_local_procdesc trans_unit_ctx cfg tenv ms [body] captured_vars
then
add_method trans_unit_ctx tenv cfg CContext.ContextNoCls procname body ms
return_param_typ_opt false outer_context_opt extra_instrs
| None -> | None ->
([], None) ()
in with CFrontend_config.IncorrectAssumption e ->
let ms, body_opt, extra_instrs = ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position
CMethod_trans.method_signature_of_decl trans_unit_ctx tenv func_decl block_data_opt e.source_range e.ast_node
in
match body_opt with
| Some body ->
(* Only in the case the function declaration has a defined body we create a procdesc *)
let procname = CMethod_signature.ms_get_name ms in
let return_param_typ_opt = CMethod_signature.ms_get_return_param_typ ms in
if CMethod_trans.create_local_procdesc trans_unit_ctx cfg tenv ms [body] captured_vars then
add_method trans_unit_ctx tenv cfg CContext.ContextNoCls procname body ms
return_param_typ_opt false outer_context_opt extra_instrs
| None ->
()
let process_method_decl ?(set_objc_accessor_attr= false) ?(is_destructor= false) trans_unit_ctx let process_method_decl ?(set_objc_accessor_attr= false) ?(is_destructor= false) trans_unit_ctx
tenv cfg curr_class meth_decl ~is_objc = tenv cfg curr_class meth_decl ~is_objc =
let ms, body_opt, extra_instrs = try
CMethod_trans.method_signature_of_decl trans_unit_ctx tenv meth_decl None let ms, body_opt, extra_instrs =
in CMethod_trans.method_signature_of_decl trans_unit_ctx tenv meth_decl None
match body_opt with in
| Some body -> match body_opt with
let procname = CMethod_signature.ms_get_name ms in | Some body ->
let return_param_typ_opt = CMethod_signature.ms_get_return_param_typ ms in let procname = CMethod_signature.ms_get_name ms in
let ms', procname' = let return_param_typ_opt = CMethod_signature.ms_get_return_param_typ ms in
if is_destructor then ( let ms', procname' =
(* For a destructor we create two procedures: a destructor wrapper and an inner destructor *) if is_destructor then (
(* A destructor wrapper is called from the outside, i.e. for destructing local variables and fields *) (* For a destructor we create two procedures: a destructor wrapper and an inner destructor *)
(* The destructor wrapper calls the inner destructor which has the actual body *) (* A destructor wrapper is called from the outside, i.e. for destructing local variables and fields *)
if CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg tenv (* The destructor wrapper calls the inner destructor which has the actual body *)
ms [body] [] if CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg
then tenv ms [body] []
add_method trans_unit_ctx tenv cfg curr_class procname body ms return_param_typ_opt then
is_objc None extra_instrs ~is_destructor_wrapper:true ; add_method trans_unit_ctx tenv cfg curr_class procname body ms return_param_typ_opt
let new_method_name = is_objc None extra_instrs ~is_destructor_wrapper:true ;
Config.clang_inner_destructor_prefix ^ Typ.Procname.get_method procname let new_method_name =
in Config.clang_inner_destructor_prefix ^ Typ.Procname.get_method procname
let ms' = in
CMethod_signature.replace_name_ms ms let ms' =
(Typ.Procname.objc_cpp_replace_method_name procname new_method_name) CMethod_signature.replace_name_ms ms
in (Typ.Procname.objc_cpp_replace_method_name procname new_method_name)
let procname' = CMethod_signature.ms_get_name ms' in in
(ms', procname') ) let procname' = CMethod_signature.ms_get_name ms' in
else (ms, procname) (ms', procname') )
in else (ms, procname)
if CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg tenv ms' in
[body] [] if CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg tenv
then ms' [body] []
add_method trans_unit_ctx tenv cfg curr_class procname' body ms' return_param_typ_opt then
is_objc None extra_instrs ~is_destructor_wrapper:false add_method trans_unit_ctx tenv cfg curr_class procname' body ms' return_param_typ_opt
| None -> is_objc None extra_instrs ~is_destructor_wrapper:false
if set_objc_accessor_attr then | None ->
ignore if set_objc_accessor_attr then
(CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg tenv ms ignore
[] []) (CMethod_trans.create_local_procdesc ~set_objc_accessor_attr trans_unit_ctx cfg tenv
ms [] [])
with CFrontend_config.IncorrectAssumption e ->
ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position
e.source_range e.ast_node
let process_property_implementation trans_unit_ctx tenv cfg curr_class let process_property_implementation trans_unit_ctx tenv cfg curr_class

@ -471,8 +471,13 @@ let fill_issue_desc_info_and_log context an (issue_desc: CIssue.issue_desc) lint
let description = process_message issue_desc.description in let description = process_message issue_desc.description in
let suggestion = Option.map ~f:process_message issue_desc.suggestion in let suggestion = Option.map ~f:process_message issue_desc.suggestion in
let issue_desc' = {issue_desc with description; loc; suggestion} in let issue_desc' = {issue_desc with description; loc; suggestion} in
log_frontend_issue context.CLintersContext.translation_unit_context try
context.CLintersContext.current_method an issue_desc' linters_def_file log_frontend_issue context.CLintersContext.translation_unit_context
context.CLintersContext.current_method an issue_desc' linters_def_file
with CFrontend_config.IncorrectAssumption e ->
let trans_unit_ctx = context.CLintersContext.translation_unit_context in
ClangLogging.log_caught_exception trans_unit_ctx "IncorrectAssumption" e.position
e.source_range e.ast_node
(* Calls the set of hard coded checkers (if any) *) (* Calls the set of hard coded checkers (if any) *)

Loading…
Cancel
Save