From 28b741e62c9d1d6b80ca378460bd608e3280fafd Mon Sep 17 00:00:00 2001 From: Ryan Rhee Date: Wed, 23 Nov 2016 09:07:49 -0800 Subject: [PATCH] [componentkit] Compute Cyclomatic Complexity Reviewed By: dulmarod Differential Revision: D4203455 fbshipit-source-id: 9b19c25 --- infer/src/IR/Localise.ml | 1 + infer/src/IR/Localise.mli | 1 + infer/src/clang/ComponentKit.ml | 39 ++++++++++++ infer/src/clang/ComponentKit.mli | 3 + infer/src/clang/cFrontend_checkers_main.ml | 1 + infer/src/clang/cFrontend_errors.ml | 2 + infer/src/clang/cIssue.ml | 4 ++ infer/src/clang/cIssue.mli | 1 + .../componentkit/TestComponentKitAnalytics.mm | 9 +++ .../componentkit_analytics_report.json | 60 +++++++++++++++++++ 10 files changed, 121 insertions(+) diff --git a/infer/src/IR/Localise.ml b/infer/src/IR/Localise.ml index 042b95be8..f6f206229 100644 --- a/infer/src/IR/Localise.ml +++ b/infer/src/IR/Localise.ml @@ -42,6 +42,7 @@ let class_cast_exception = "CLASS_CAST_EXCEPTION" let comparing_floats_for_equality = "COMPARING_FLOAT_FOR_EQUALITY" let condition_is_assignment = "CONDITION_IS_ASSIGNMENT" let component_factory_function = "COMPONENT_FACTORY_FUNCTION" +let component_file_cyclomatic_complexity = "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY" let component_file_line_count = "COMPONENT_FILE_LINE_COUNT" let component_initializer_with_side_effects = "COMPONENT_INITIALIZER_WITH_SIDE_EFFECTS" let component_with_multiple_factory_methods = "COMPONENT_WITH_MULTIPLE_FACTORY_METHODS" diff --git a/infer/src/IR/Localise.mli b/infer/src/IR/Localise.mli index 51bbec9bb..1fcafa255 100644 --- a/infer/src/IR/Localise.mli +++ b/infer/src/IR/Localise.mli @@ -42,6 +42,7 @@ val condition_always_false : t val condition_always_true : t val context_leak : t val component_factory_function : t +val component_file_cyclomatic_complexity : t val component_file_line_count : t val component_initializer_with_side_effects : t val component_with_multiple_factory_methods : t diff --git a/infer/src/clang/ComponentKit.ml b/infer/src/clang/ComponentKit.ml index 9ab257149..a423d2b8e 100644 --- a/infer/src/clang/ComponentKit.ml +++ b/infer/src/clang/ComponentKit.ml @@ -329,3 +329,42 @@ let component_file_line_count_info (context: CLintersContext.context) dec = } ) (IList.range 1 line_count) | _ -> [] + +(** Computes a component file's cyclomatic complexity. + + Somewhat borrowed from + https://github.com/oclint/oclint/blob/5889b5ec168185513ba69ce83821ea1cc8e63fbe + /oclint-metrics/lib/CyclomaticComplexityMetric.cpp *) +let component_file_cyclomatic_complexity_info (context: CLintersContext.context) an = + let is_cyclo_stmt stmt = match stmt with + | Clang_ast_t.IfStmt _ + | Clang_ast_t.ForStmt _ + | Clang_ast_t.ObjCForCollectionStmt _ + | Clang_ast_t.CXXForRangeStmt _ + | Clang_ast_t.WhileStmt _ + | Clang_ast_t.DoStmt _ + | Clang_ast_t.CaseStmt _ + | Clang_ast_t.ObjCAtCatchStmt _ + | Clang_ast_t.CXXCatchStmt _ + | Clang_ast_t.ConditionalOperator _ -> true + | Clang_ast_t.BinaryOperator (_, _, _, boi) -> + IList.mem (=) boi.Clang_ast_t.boi_kind [`LAnd; `LOr] + | _ -> false in + let cyclo_loc_opt an = match an with + | CTL.Stmt stmt when (Config.compute_analytics + && is_cyclo_stmt stmt + && is_ck_context context an) -> + Some (CFrontend_checkers.location_from_stmt context stmt) + | CTL.Decl (Clang_ast_t.TranslationUnitDecl _ as d) + when Config.compute_analytics && context.is_ck_translation_unit -> + Some (CFrontend_checkers.location_from_decl context d) + | _ -> None in + match cyclo_loc_opt an with + | Some loc -> + CTL.True, Some { + CIssue.issue = CIssue.Component_file_cyclomatic_complexity; + CIssue.description = "Cyclomatic Complexity Incremental Marker"; + CIssue.suggestion = None; + CIssue.loc = loc + } + | _ -> CTL.False, None diff --git a/infer/src/clang/ComponentKit.mli b/infer/src/clang/ComponentKit.mli index 8deaa2827..2bdd2443b 100644 --- a/infer/src/clang/ComponentKit.mli +++ b/infer/src/clang/ComponentKit.mli @@ -31,3 +31,6 @@ val component_initializer_with_side_effects_advice : val component_file_line_count_info : CLintersContext.context -> Clang_ast_t.decl -> CIssue.issue_desc list + +val component_file_cyclomatic_complexity_info : + CLintersContext.context -> CTL.ast_node -> CTL.t * CIssue.issue_desc option diff --git a/infer/src/clang/cFrontend_checkers_main.ml b/infer/src/clang/cFrontend_checkers_main.ml index 4213fb9b8..54ac2dbb2 100644 --- a/infer/src/clang/cFrontend_checkers_main.ml +++ b/infer/src/clang/cFrontend_checkers_main.ml @@ -135,6 +135,7 @@ let do_frontend_checks trans_unit_ctx ast = let context = context_with_ck_set (CLintersContext.empty trans_unit_ctx) decl_list in ignore (CFrontend_errors.run_translation_unit_checker context ast); + ignore (CFrontend_errors.run_frontend_checkers_on_an context (CTL.Decl ast)); let is_decl_allowed decl = let decl_info = Clang_ast_proj.get_decl_tuple decl in CLocation.should_do_frontend_check trans_unit_ctx decl_info.Clang_ast_t.di_source_range in diff --git a/infer/src/clang/cFrontend_errors.ml b/infer/src/clang/cFrontend_errors.ml index da79766a6..a50ab38f8 100644 --- a/infer/src/clang/cFrontend_errors.ml +++ b/infer/src/clang/cFrontend_errors.ml @@ -19,12 +19,14 @@ let decl_checkers_list = [CFrontend_checkers.ctl_strong_delegate_warning; ComponentKit.component_with_unconventional_superclass_advice; ComponentKit.mutable_local_vars_advice; ComponentKit.component_factory_function_advice; + ComponentKit.component_file_cyclomatic_complexity_info; ComponentKit.component_with_multiple_factory_methods_advice;] (* List of checkers on ivar access *) let stmt_checkers_list = [CFrontend_checkers.ctl_direct_atomic_property_access_warning; CFrontend_checkers.ctl_captured_cxx_ref_in_objc_block_warning; CFrontend_checkers.ctl_bad_pointer_comparison_warning; + ComponentKit.component_file_cyclomatic_complexity_info; ComponentKit.component_initializer_with_side_effects_advice;] (* List of checkers on translation unit that potentially output multiple issues *) diff --git a/infer/src/clang/cIssue.ml b/infer/src/clang/cIssue.ml index 494704e9e..a60d2c387 100644 --- a/infer/src/clang/cIssue.ml +++ b/infer/src/clang/cIssue.ml @@ -11,6 +11,7 @@ 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 @@ -31,6 +32,8 @@ let to_string issue = 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 -> @@ -67,6 +70,7 @@ let severity_of_issue issue = | 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 diff --git a/infer/src/clang/cIssue.mli b/infer/src/clang/cIssue.mli index ce0bfe315..d9185d429 100644 --- a/infer/src/clang/cIssue.mli +++ b/infer/src/clang/cIssue.mli @@ -11,6 +11,7 @@ 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 diff --git a/infer/tests/build_systems/codetoanalyze/componentkit/TestComponentKitAnalytics.mm b/infer/tests/build_systems/codetoanalyze/componentkit/TestComponentKitAnalytics.mm index 89abd2a02..e3186254f 100644 --- a/infer/tests/build_systems/codetoanalyze/componentkit/TestComponentKitAnalytics.mm +++ b/infer/tests/build_systems/codetoanalyze/componentkit/TestComponentKitAnalytics.mm @@ -12,4 +12,13 @@ @interface SomeClass : CKCompositeComponent @end @implementation SomeClass ++ (instancetype) new { + if (1 == 3) { + return nil; + } else if (2 == 4) { + return nil; + } else { + return nil; + } +} @end diff --git a/infer/tests/build_systems/expected_outputs/componentkit_analytics_report.json b/infer/tests/build_systems/expected_outputs/componentkit_analytics_report.json index d91327dc7..b7a829210 100644 --- a/infer/tests/build_systems/expected_outputs/componentkit_analytics_report.json +++ b/infer/tests/build_systems/expected_outputs/componentkit_analytics_report.json @@ -1,4 +1,64 @@ [ + { + "bug_type": "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY", + "file": "TestComponentKitAnalytics.mm", + "procedure": "SomeClass_new" + }, + { + "bug_type": "COMPONENT_FILE_CYCLOMATIC_COMPLEXITY", + "file": "TestComponentKitAnalytics.mm", + "procedure": "SomeClass_new" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, + { + "bug_type": "COMPONENT_FILE_LINE_COUNT", + "file": "TestComponentKitAnalytics.mm", + "procedure": "Linters_dummy_method" + }, { "bug_type": "COMPONENT_FILE_LINE_COUNT", "file": "TestComponentKitAnalytics.mm",