[CTL] Let issue name be a string for linters

Summary:
This change is to support the development of CTL's DSL, where issues can be specified directly from the language, in the form of strings.
Severity is specified locally to the place where the check is defined

Reviewed By: ddino

Differential Revision: D4326594

fbshipit-source-id: 7b146ac
master
Martino Luca 8 years ago committed by Facebook Github Bot
parent ee90e10491
commit ebb6931358

@ -116,11 +116,12 @@ let mutable_local_vars_advice context an =
&& not decl_info.di_is_implicit in && not decl_info.di_is_implicit in
if condition then if condition then
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Mutable_local_variable_in_component_file; CIssue.name = Localise.to_string Localise.mutable_local_variable_in_component_file;
CIssue.description = "Local variable '" ^ named_decl_info.ni_name severity = Exceptions.Kadvice;
^ "' should be const to avoid reassignment"; description = "Local variable '" ^ named_decl_info.ni_name
CIssue.suggestion = Some "Add a const (after the asterisk for pointer types)."; ^ "' should be const to avoid reassignment";
CIssue.loc = CFrontend_checkers.location_from_dinfo context decl_info suggestion = Some "Add a const (after the asterisk for pointer types).";
loc = CFrontend_checkers.location_from_dinfo context decl_info
} }
else CTL.False, None else CTL.False, None
| _ -> CTL.False, None (* Should only be called with a VarDecl *) | _ -> CTL.False, None (* Should only be called with a VarDecl *)
@ -142,13 +143,14 @@ let component_factory_function_advice context an =
is_ck_context context an && is_component_if objc_interface in is_ck_context context an && is_component_if objc_interface in
if condition then if condition then
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Component_factory_function; CIssue.name = Localise.to_string Localise.component_factory_function;
CIssue.description = "Break out composite components"; severity = Exceptions.Kadvice;
CIssue.suggestion = Some ( description = "Break out composite components";
suggestion = Some (
"Prefer subclassing CKCompositeComponent to static helper functions \ "Prefer subclassing CKCompositeComponent to static helper functions \
that return a CKComponent subclass." that return a CKComponent subclass."
); );
CIssue.loc = CFrontend_checkers.location_from_dinfo context decl_info loc = CFrontend_checkers.location_from_dinfo context decl_info
} }
else CTL.False, None else CTL.False, None
| _ -> CTL.False, None (* Should only be called with FunctionDecl *) | _ -> CTL.False, None (* Should only be called with FunctionDecl *)
@ -182,12 +184,13 @@ let component_with_unconventional_superclass_advice context an =
&& not has_conventional_superclass in && not has_conventional_superclass in
if condition then if condition then
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Component_with_unconventional_superclass; CIssue.name = Localise.to_string Localise.component_with_unconventional_superclass;
CIssue.description = "Never Subclass Components"; severity = Exceptions.Kadvice;
CIssue.suggestion = Some ( description = "Never Subclass Components";
suggestion = Some (
"Instead, create a new subclass of CKCompositeComponent." "Instead, create a new subclass of CKCompositeComponent."
); );
CIssue.loc = CFrontend_checkers.location_from_decl context if_decl loc = CFrontend_checkers.location_from_decl context if_decl
} }
else else
CTL.False, None CTL.False, None
@ -232,12 +235,13 @@ let component_with_multiple_factory_methods_advice context an =
let factory_methods = IList.filter (is_available_factory_method if_decl) decls in let factory_methods = IList.filter (is_available_factory_method if_decl) decls in
if (IList.length factory_methods) > 1 then if (IList.length factory_methods) > 1 then
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Component_with_multiple_factory_methods; CIssue.name = Localise.to_string Localise.component_with_multiple_factory_methods;
CIssue.description = "Avoid Overrides"; severity = Exceptions.Kadvice;
CIssue.suggestion = description = "Avoid Overrides";
suggestion =
Some "Instead, always expose all parameters in a single \ Some "Instead, always expose all parameters in a single \
designated initializer and document which are optional."; designated initializer and document which are optional.";
CIssue.loc = CFrontend_checkers.location_from_dinfo context decl_info loc = CFrontend_checkers.location_from_dinfo context decl_info
} }
else else
CTL.False, None CTL.False, None
@ -285,11 +289,12 @@ let rec _component_initializer_with_side_effects_advice
| Some "dispatch_async" | Some "dispatch_async"
| Some "dispatch_sync" -> | Some "dispatch_sync" ->
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Component_initializer_with_side_effects; CIssue.name = Localise.to_string Localise.component_initializer_with_side_effects;
CIssue.description = "No Side-effects"; severity = Exceptions.Kadvice;
CIssue.suggestion = Some "Your +new method should not modify any \ description = "No Side-effects";
global variables or global state."; suggestion = Some "Your +new method should not modify any \
CIssue.loc = CFrontend_checkers.location_from_stmt context call_stmt global variables or global state.";
loc = CFrontend_checkers.location_from_stmt context call_stmt
} }
| _ -> | _ ->
CTL.False, None) CTL.False, None)
@ -317,10 +322,11 @@ let component_file_line_count_info (context: CLintersContext.context) dec =
context.translation_unit_context.CFrontend_config.source_file in context.translation_unit_context.CFrontend_config.source_file in
let line_count = SourceFile.line_count source_file in let line_count = SourceFile.line_count source_file in
IList.map (fun i -> { IList.map (fun i -> {
CIssue.issue = CIssue.Component_file_line_count; CIssue.name = Localise.to_string Localise.component_file_line_count;
CIssue.description = "Line count analytics"; severity = Exceptions.Kinfo;
CIssue.suggestion = None; description = "Line count analytics";
CIssue.loc = { suggestion = None;
loc = {
Location.line = i; Location.line = i;
Location.col = 0; Location.col = 0;
Location.file = source_file Location.file = source_file
@ -361,9 +367,10 @@ let component_file_cyclomatic_complexity_info (context: CLintersContext.context)
match cyclo_loc_opt an with match cyclo_loc_opt an with
| Some loc -> | Some loc ->
CTL.True, Some { CTL.True, Some {
CIssue.issue = CIssue.Component_file_cyclomatic_complexity; CIssue.name = Localise.to_string Localise.component_file_cyclomatic_complexity;
CIssue.description = "Cyclomatic Complexity Incremental Marker"; severity = Exceptions.Kinfo;
CIssue.suggestion = None; description = "Cyclomatic Complexity Incremental Marker";
CIssue.loc = loc suggestion = None;
loc = loc
} }
| _ -> CTL.False, None | _ -> CTL.False, None

@ -119,12 +119,13 @@ let ctl_ns_notification_warning lctx an =
let condition = InNode (["ObjCImplementationDecl"; "ObjCProtocolDecl"], let condition = InNode (["ObjCImplementationDecl"; "ObjCProtocolDecl"],
Not (Implies (eventually_addObserver, eventually_removeObserver))) in Not (Implies (eventually_addObserver, eventually_removeObserver))) in
let issue_desc = { let issue_desc = {
CIssue.issue = CIssue.Registered_observer_being_deallocated; CIssue.name = Localise.to_string Localise.registered_observer_being_deallocated;
CIssue.description = severity = Exceptions.Kwarning;
description =
"Object self is registered in a notification center but not being removed before deallocation"; "Object self is registered in a notification center but not being removed before deallocation";
CIssue.suggestion = suggestion =
Some "Consider removing the object from the notification center before its deallocation."; Some "Consider removing the object from the notification center before its deallocation.";
CIssue.loc = location_from_an lctx an; loc = location_from_an lctx an;
} in } in
condition, Some issue_desc condition, Some issue_desc
@ -152,7 +153,8 @@ let ctl_bad_pointer_comparison_warning lctx an =
let condition = InNode (["IfStmt"; "ForStmt"; "WhileStmt"; "ConditionalOperator"], etx) in let condition = InNode (["IfStmt"; "ForStmt"; "WhileStmt"; "ConditionalOperator"], etx) in
let issue_desc = let issue_desc =
{ CIssue. { CIssue.
issue = CIssue.Bad_pointer_comparison; name = Localise.to_string Localise.bad_pointer_comparison;
severity = Exceptions.Kwarning;
description = "Implicitly checking whether NSNumber pointer is nil"; description = "Implicitly checking whether NSNumber pointer is nil";
suggestion = suggestion =
Some ("Did you mean to compare against the unboxed value instead? " ^ Some ("Did you mean to compare against the unboxed value instead? " ^
@ -175,11 +177,12 @@ let ctl_strong_delegate_warning lctx an =
And (name_does_not_contains_queue, And (name_does_not_contains_queue,
is_strong_property))) in is_strong_property))) in
let issue_desc = { let issue_desc = {
CIssue.issue = CIssue.Strong_delegate_warning; CIssue.name = Localise.to_string Localise.strong_delegate_warning;
CIssue.description = severity = Exceptions.Kwarning;
description =
"Property or ivar %decl_name% declared strong"; "Property or ivar %decl_name% declared strong";
CIssue.suggestion = Some "In general delegates should be declared weak or assign"; suggestion = Some "In general delegates should be declared weak or assign";
CIssue.loc = location_from_an lctx an loc = location_from_an lctx an
} in } in
condition, Some issue_desc condition, Some issue_desc
@ -196,12 +199,14 @@ let ctl_global_var_init_with_calls_warning lctx an =
let condition = let condition =
InNode (["VarDecl"], And (ctl_is_global_var, ctl_is_initialized_with_expensive_call)) in InNode (["VarDecl"], And (ctl_is_global_var, ctl_is_initialized_with_expensive_call)) in
let issue_desc = { let issue_desc = {
CIssue.issue = CIssue.Global_variable_initialized_with_function_or_method_call; CIssue.name =
CIssue.description = Localise.to_string Localise.global_variable_initialized_with_function_or_method_call;
severity = Exceptions.Kwarning;
description =
"Global variable %decl_name% is initialized using a function or method call"; "Global variable %decl_name% is initialized using a function or method call";
CIssue.suggestion = Some suggestion = Some
"If the function/method call is expensive, it can affect the starting time of the app."; "If the function/method call is expensive, it can affect the starting time of the app.";
CIssue.loc = location_from_an lctx an loc = location_from_an lctx an
} in } in
condition, Some issue_desc condition, Some issue_desc
@ -212,11 +217,12 @@ let ctl_assign_pointer_warning lctx an =
And (Atomic ("is_assign_property", []), And (Atomic ("is_assign_property", []),
Atomic ("is_property_pointer_type", []))) in Atomic ("is_property_pointer_type", []))) in
let issue_desc = let issue_desc =
{ CIssue.issue = CIssue.Assign_pointer_warning; { CIssue.name = Localise.to_string Localise.assign_pointer_warning;
CIssue.description = severity = Exceptions.Kwarning;
description =
"Property `%decl_name%` is a pointer type marked with the `assign` attribute"; "Property `%decl_name%` is a pointer type marked with the `assign` attribute";
CIssue.suggestion = Some "Use a different attribute like `strong` or `weak`."; suggestion = Some "Use a different attribute like `strong` or `weak`.";
CIssue.loc = location_from_an lctx an loc = location_from_an lctx an
} in } in
condition, Some issue_desc condition, Some issue_desc
@ -233,11 +239,12 @@ let ctl_direct_atomic_property_access_warning lctx an =
Not (Atomic ("is_objc_constructor", []))), Not (Atomic ("is_objc_constructor", []))),
Not (Atomic ("is_objc_dealloc", [])))) in Not (Atomic ("is_objc_dealloc", [])))) in
let issue_desc = { let issue_desc = {
CIssue.issue = CIssue.Direct_atomic_property_access; CIssue.name = Localise.to_string Localise.direct_atomic_property_access;
CIssue.description = "Direct access to ivar %ivar_name% of an atomic property"; severity = Exceptions.Kwarning;
CIssue.suggestion = description = "Direct access to ivar %ivar_name% of an atomic property";
suggestion =
Some "Accessing an ivar of an atomic property makes the property nonatomic"; Some "Accessing an ivar of an atomic property makes the property nonatomic";
CIssue.loc = location_from_an lctx an loc = location_from_an lctx an
} in } in
condition, Some issue_desc condition, Some issue_desc
@ -246,12 +253,13 @@ let ctl_captured_cxx_ref_in_objc_block_warning lctx an =
let open CTL in let open CTL in
let condition = InNode (["BlockDecl"], Atomic ("captures_cxx_references", [])) in let condition = InNode (["BlockDecl"], Atomic ("captures_cxx_references", [])) in
let issue_desc = { let issue_desc = {
CIssue.issue = CIssue.Cxx_reference_captured_in_objc_block; CIssue.name = Localise.to_string Localise.cxx_reference_captured_in_objc_block;
CIssue.description = severity = Exceptions.Kwarning;
description =
"C++ Reference variable(s) %var_name% captured by Objective-C block"; "C++ Reference variable(s) %var_name% captured by Objective-C block";
CIssue.suggestion = Some ("C++ References are unmanaged and may be invalid " ^ suggestion = Some ("C++ References are unmanaged and may be invalid " ^
"by the time the block executes."); "by the time the block executes.");
CIssue.loc = match an with loc = match an with
| Stmt (Clang_ast_t.BlockExpr (_, _ , _, decl)) -> location_from_an lctx (Decl decl) | Stmt (Clang_ast_t.BlockExpr (_, _ , _, decl)) -> location_from_an lctx (Decl decl)
| _ -> location_from_an lctx an; | _ -> location_from_an lctx an;
} in } in

@ -120,15 +120,14 @@ let get_err_log translation_unit_context method_decl_opt =
(* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *) (* Add a frontend warning with a description desc at location loc to the errlog of a proc desc *)
let log_frontend_issue translation_unit_context method_decl_opt key issue_desc = let log_frontend_issue translation_unit_context method_decl_opt key issue_desc =
let issue = issue_desc.CIssue.issue in let name = issue_desc.CIssue.name in
let loc = issue_desc.CIssue.loc in let loc = issue_desc.CIssue.loc in
let errlog = get_err_log translation_unit_context method_decl_opt in let errlog = get_err_log translation_unit_context method_decl_opt in
let err_desc = Errdesc.explain_frontend_warning issue_desc.CIssue.description let err_desc = Errdesc.explain_frontend_warning issue_desc.CIssue.description
issue_desc.CIssue.suggestion loc in issue_desc.CIssue.suggestion loc in
let name = CIssue.to_string issue in
let exn = Exceptions.Frontend_warning (name, err_desc, __POS__) in let exn = Exceptions.Frontend_warning (name, err_desc, __POS__) in
let trace = [ Errlog.make_trace_element 0 issue_desc.CIssue.loc "" [] ] in let trace = [ Errlog.make_trace_element 0 issue_desc.CIssue.loc "" [] ] in
let err_kind = CIssue.severity_of_issue issue in let err_kind = issue_desc.CIssue.severity in
let method_name = Ast_utils.full_name_of_decl_opt method_decl_opt in let method_name = Ast_utils.full_name_of_decl_opt method_decl_opt in
let key = Hashtbl.hash (key ^ method_name) in let key = Hashtbl.hash (key ^ method_name) in
Reporting.log_issue_from_errlog err_kind errlog exn ~loc ~ltr:trace Reporting.log_issue_from_errlog err_kind errlog exn ~loc ~ltr:trace

@ -9,75 +9,9 @@
open! IStd open! IStd
type issue =
| Assign_pointer_warning
| Bad_pointer_comparison
| Component_factory_function
| Component_file_cyclomatic_complexity
| Component_file_line_count
| Component_initializer_with_side_effects
| Component_with_multiple_factory_methods
| Component_with_unconventional_superclass
| Cxx_reference_captured_in_objc_block
| Direct_atomic_property_access
| Global_variable_initialized_with_function_or_method_call
| Mutable_local_variable_in_component_file
| Registered_observer_being_deallocated
| Strong_delegate_warning
let to_string issue =
Localise.to_string
(match issue with
| Assign_pointer_warning ->
Localise.assign_pointer_warning
| Bad_pointer_comparison ->
Localise.bad_pointer_comparison
| Component_factory_function ->
Localise.component_factory_function
| Component_file_cyclomatic_complexity ->
Localise.component_file_cyclomatic_complexity
| Component_file_line_count ->
Localise.component_file_line_count
| Component_initializer_with_side_effects ->
Localise.component_initializer_with_side_effects
| Component_with_multiple_factory_methods ->
Localise.component_with_multiple_factory_methods
| 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 ->
Localise.direct_atomic_property_access
| Global_variable_initialized_with_function_or_method_call ->
Localise.global_variable_initialized_with_function_or_method_call
| Mutable_local_variable_in_component_file ->
Localise.mutable_local_variable_in_component_file
| Registered_observer_being_deallocated ->
Localise.registered_observer_being_deallocated
| Strong_delegate_warning ->
Localise.strong_delegate_warning
)
let severity_of_issue issue =
match issue with
| Assign_pointer_warning
| Bad_pointer_comparison
| Cxx_reference_captured_in_objc_block
| Direct_atomic_property_access
| Global_variable_initialized_with_function_or_method_call
| Registered_observer_being_deallocated
| Strong_delegate_warning -> Exceptions.Kwarning
| Component_factory_function
| Component_initializer_with_side_effects
| Component_with_multiple_factory_methods
| Component_with_unconventional_superclass
| Mutable_local_variable_in_component_file -> Exceptions.Kadvice
| Component_file_cyclomatic_complexity
| Component_file_line_count -> Exceptions.Kinfo
type issue_desc = { type issue_desc = {
issue : issue; (* issue *) name : string; (* issue name *)
severity: Exceptions.err_kind;
description : string; (* Description in the error message *) description : string; (* Description in the error message *)
suggestion : string option; (* an optional suggestion or correction *) suggestion : string option; (* an optional suggestion or correction *)
loc : Location.t; (* location in the code *) loc : Location.t; (* location in the code *)

@ -9,28 +9,9 @@
open! IStd open! IStd
type issue =
| Assign_pointer_warning
| Bad_pointer_comparison
| Component_factory_function
| Component_file_cyclomatic_complexity
| Component_file_line_count
| Component_initializer_with_side_effects
| Component_with_multiple_factory_methods
| Component_with_unconventional_superclass
| Cxx_reference_captured_in_objc_block
| Direct_atomic_property_access
| Global_variable_initialized_with_function_or_method_call
| Mutable_local_variable_in_component_file
| Registered_observer_being_deallocated
| Strong_delegate_warning
val to_string : issue -> string
val severity_of_issue : issue -> Exceptions.err_kind
type issue_desc = { type issue_desc = {
issue : issue; (* issue *) name : string; (* issue name *)
severity : Exceptions.err_kind;
description : string; (* Description in the error message *) description : string; (* Description in the error message *)
suggestion : string option; (* an optional suggestion or correction *) suggestion : string option; (* an optional suggestion or correction *)
loc : Location.t; (* location in the code *) loc : Location.t; (* location in the code *)

Loading…
Cancel
Save