From 1b13452de10d681787c76fd04d09f44e168abdb2 Mon Sep 17 00:00:00 2001 From: ConjuringCoffee <72548231+ConjuringCoffee@users.noreply.github.com> Date: Wed, 10 Jan 2024 16:51:16 +0100 Subject: [PATCH] Improve check for `SY-SYSID` usage (#1126) Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com> --- docs/_checks/102.md | 4 +- src/checks/zcl_aoc_check_102.clas.abap | 129 ++++++++++----- .../zcl_aoc_check_102.clas.locals_def.abap | 31 ---- .../zcl_aoc_check_102.clas.locals_imp.abap | 82 ---------- .../zcl_aoc_check_102.clas.testclasses.abap | 19 ++- src/checks/zcl_aoc_check_102.clas.xml | 2 +- .../zcl_aoc_sy_variable_analyzer.clas.abap | 148 ++++++++++++++++++ .../zcl_aoc_sy_variable_analyzer.clas.xml | 16 ++ 8 files changed, 271 insertions(+), 160 deletions(-) delete mode 100644 src/checks/zcl_aoc_check_102.clas.locals_imp.abap create mode 100644 src/utils/zcl_aoc_sy_variable_analyzer.clas.abap create mode 100644 src/utils/zcl_aoc_sy_variable_analyzer.clas.xml diff --git a/docs/_checks/102.md b/docs/_checks/102.md index 82a15832..905ecaf4 100644 --- a/docs/_checks/102.md +++ b/docs/_checks/102.md @@ -1,8 +1,8 @@ --- -title: Use of system ID +title: Use of SY-SYSID cNumber: CHECK_102 rfc: true index: 102 --- -Coupling your logic to the name of the SAP system (`SY-SYSID`) is generally a bad idea. Use this check to find all uses of the system ID. \ No newline at end of file +Coupling your logic to the ID of the SAP system (`SY-SYSID`) is generally a bad idea. Use this check to find all uses of the system ID. \ No newline at end of file diff --git a/src/checks/zcl_aoc_check_102.clas.abap b/src/checks/zcl_aoc_check_102.clas.abap index 250999db..5d81b7c6 100644 --- a/src/checks/zcl_aoc_check_102.clas.abap +++ b/src/checks/zcl_aoc_check_102.clas.abap @@ -1,4 +1,4 @@ -"!

102 - Use of system ID

+"!

102 - Use of SY-SYSID

CLASS zcl_aoc_check_102 DEFINITION PUBLIC INHERITING FROM zcl_aoc_super @@ -8,6 +8,22 @@ CLASS zcl_aoc_check_102 DEFINITION METHODS constructor. METHODS check REDEFINITION. + + PRIVATE SECTION. + TYPES: + BEGIN OF ty_s_error_code_map, + usage_kind TYPE zcl_aoc_sy_variable_analyzer=>ty_v_usage_kind, + error_code TYPE sci_errc, + text TYPE ty_scimessage_text, + END OF ty_s_error_code_map. + + DATA mt_error_code_map TYPE TABLE OF ty_s_error_code_map. + + METHODS is_using_only_first_letter + IMPORTING + is_token TYPE stokesx + RETURNING + VALUE(rv_result) TYPE abap_bool. ENDCLASS. @@ -15,7 +31,7 @@ CLASS zcl_aoc_check_102 IMPLEMENTATION. METHOD constructor. super->constructor( ). - version = '001'. + version = '002'. position = '102'. has_attributes = abap_true. @@ -23,28 +39,44 @@ CLASS zcl_aoc_check_102 IMPLEMENTATION. enable_rfc( ). - insert_scimessage( iv_code = gc_code-usage_uncategorized - iv_text = TEXT-001 ). - insert_scimessage( iv_code = gc_code-in_condition - iv_text = TEXT-002 ). + mt_error_code_map = VALUE #( ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-usage_uncategorized + error_code = gc_code-usage_uncategorized + text = TEXT-001 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-in_condition + error_code = gc_code-in_condition + text = TEXT-002 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-as_default_value + error_code = gc_code-as_default_value + text = TEXT-004 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-in_concatenate + error_code = gc_code-in_concatenate + text = TEXT-005 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-overridden + error_code = gc_code-overridden + text = TEXT-006 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-assigned_to_variable + error_code = gc_code-assigned_to_variable + text = TEXT-007 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-in_database_select + error_code = gc_code-in_database_select + text = TEXT-008 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-in_write + error_code = gc_code-in_write + text = TEXT-009 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-in_message + error_code = gc_code-in_message + text = TEXT-010 ) + ( usage_kind = zcl_aoc_sy_variable_analyzer=>gc_usage_kind-within_macro + error_code = gc_code-within_macro + text = TEXT-011 ) ). + + LOOP AT mt_error_code_map ASSIGNING FIELD-SYMBOL(). + insert_scimessage( iv_code = -error_code + iv_text = -text ). + ENDLOOP. + insert_scimessage( iv_code = gc_code-first_letter_used iv_text = TEXT-003 ). - insert_scimessage( iv_code = gc_code-as_default_value - iv_text = TEXT-004 ). - insert_scimessage( iv_code = gc_code-in_concatenate - iv_text = TEXT-005 ). - insert_scimessage( iv_code = gc_code-overridden - iv_text = TEXT-006 ). - insert_scimessage( iv_code = gc_code-assigned_to_variable - iv_text = TEXT-007 ). - insert_scimessage( iv_code = gc_code-in_database_select - iv_text = TEXT-008 ). - insert_scimessage( iv_code = gc_code-in_write - iv_text = TEXT-009 ). - insert_scimessage( iv_code = gc_code-in_message - iv_text = TEXT-010 ). - insert_scimessage( iv_code = gc_code-within_macro - iv_text = TEXT-011 ). ENDMETHOD. METHOD check. @@ -52,33 +84,44 @@ CLASS zcl_aoc_check_102 IMPLEMENTATION. " https://github.com/larshp/abapOpenChecks " MIT License - DATA(lo_helper) = NEW lcl_check_helper( io_scan ). + DATA(lo_variable_analyzer) = NEW zcl_aoc_sy_variable_analyzer( io_scan ). + + DATA(lt_variable_usage) = lo_variable_analyzer->analyze_variable_usage( 'SYSID' ). + + LOOP AT lt_variable_usage ASSIGNING FIELD-SYMBOL(). + DATA(lv_error_code) = VALUE sci_errc( ). - LOOP AT io_scan->statements ASSIGNING FIELD-SYMBOL(). - LOOP AT io_scan->tokens ASSIGNING FIELD-SYMBOL() - FROM -from TO -to - WHERE str CP 'SY-SYSID*' - OR str CP '@SY-SYSID*'. + ASSIGN mt_error_code_map[ usage_kind = -usage_kind ] TO FIELD-SYMBOL(). - DATA(lv_index_token) = sy-tabix. + IF sy-subrc <> 0. + " Ignore this usage kind + CONTINUE. + ENDIF. - DATA(lv_error_code) = lo_helper->determine_error_code( is_token = - iv_index_token = lv_index_token - is_statement = ). + lv_error_code = -error_code. - IF lv_error_code IS INITIAL. - " No error - CONTINUE. - ENDIF. + ASSIGN io_scan->tokens[ -token_index ] TO FIELD-SYMBOL(). - DATA(lv_include) = io_scan->get_include( -level ). + IF is_using_only_first_letter( ) = abap_true. + " This is more interesting than where it is used + lv_error_code = gc_code-first_letter_used. + ENDIF. + + DATA(lv_include) = io_scan->get_include( -statement_level ). + + inform( p_sub_obj_name = lv_include + p_line = -row + p_kind = mv_errty + p_test = myname + p_code = lv_error_code ). - inform( p_sub_obj_name = lv_include - p_line = -row - p_kind = mv_errty - p_test = myname - p_code = lv_error_code ). - ENDLOOP. ENDLOOP. ENDMETHOD. + + METHOD is_using_only_first_letter. + rv_result = SWITCH #( is_token-str + WHEN `SY-SYSID+0(1)` OR `SY-SYSID(1)` OR `@SY-SYSID+0(1)` OR `@SY-SYSID(1)` + THEN abap_true + ELSE abap_false ). + ENDMETHOD. ENDCLASS. diff --git a/src/checks/zcl_aoc_check_102.clas.locals_def.abap b/src/checks/zcl_aoc_check_102.clas.locals_def.abap index dd350435..777b1cb6 100644 --- a/src/checks/zcl_aoc_check_102.clas.locals_def.abap +++ b/src/checks/zcl_aoc_check_102.clas.locals_def.abap @@ -12,34 +12,3 @@ CONSTANTS: in_message TYPE sci_errc VALUE '010', within_macro TYPE sci_errc VALUE '011', END OF gc_code. - -CLASS lcl_check_helper DEFINITION. - - PUBLIC SECTION. - METHODS constructor - IMPORTING - io_scan TYPE REF TO zcl_aoc_scan. - - METHODS determine_error_code - IMPORTING - is_token TYPE stokesx - iv_index_token TYPE sy-tabix - is_statement TYPE sstmnt - RETURNING - VALUE(rv_error_code) TYPE sci_errc. - - PRIVATE SECTION. - DATA mo_scan TYPE REF TO zcl_aoc_scan. - - METHODS is_using_only_first_letter - IMPORTING - is_token TYPE stokesx - RETURNING - VALUE(rv_result) TYPE abap_bool. - - METHODS is_used_in_macro - IMPORTING - is_statement TYPE sstmnt - RETURNING - VALUE(rv_result) TYPE abap_bool. -ENDCLASS. diff --git a/src/checks/zcl_aoc_check_102.clas.locals_imp.abap b/src/checks/zcl_aoc_check_102.clas.locals_imp.abap deleted file mode 100644 index f40f5890..00000000 --- a/src/checks/zcl_aoc_check_102.clas.locals_imp.abap +++ /dev/null @@ -1,82 +0,0 @@ -CLASS lcl_check_helper IMPLEMENTATION. - METHOD constructor. - mo_scan = io_scan. - ENDMETHOD. - - METHOD determine_error_code. - IF is_using_only_first_letter( is_token ) = abap_true. - rv_error_code = gc_code-first_letter_used. - RETURN. - ENDIF. - - IF is_used_in_macro( is_statement ) = abap_true. - rv_error_code = gc_code-within_macro. - RETURN. - ENDIF. - - ASSIGN mo_scan->tokens[ is_statement-from ] TO FIELD-SYMBOL(). - - CASE -str. - WHEN zcl_aoc_scan=>gc_keyword-types - OR zcl_aoc_scan=>gc_keyword-ranges. - " Ignore - RETURN. - WHEN zcl_aoc_scan=>gc_keyword-concatenate. - rv_error_code = gc_code-in_concatenate. - WHEN is_token-str. - rv_error_code = gc_code-overridden. - WHEN zcl_aoc_scan=>gc_keyword-select. - rv_error_code = gc_code-in_database_select. - WHEN zcl_aoc_scan=>gc_keyword-write. - rv_error_code = gc_code-in_write. - WHEN zcl_aoc_scan=>gc_keyword-message. - rv_error_code = gc_code-in_message. - WHEN zcl_aoc_scan=>gc_keyword-if - OR zcl_aoc_scan=>gc_keyword-elseif - OR zcl_aoc_scan=>gc_keyword-case - OR zcl_aoc_scan=>gc_keyword-when - OR zcl_aoc_scan=>gc_keyword-check - OR zcl_aoc_scan=>gc_keyword-assert. - rv_error_code = gc_code-in_condition. - WHEN zcl_aoc_scan=>gc_keyword-methods - OR zcl_aoc_scan=>gc_keyword-class_methods - OR zcl_aoc_scan=>gc_keyword-data - OR zcl_aoc_scan=>gc_keyword-class_data. - ASSIGN mo_scan->tokens[ iv_index_token - 1 ] TO FIELD-SYMBOL(). - - CASE -str. - WHEN zcl_aoc_scan=>gc_keyword-default. - rv_error_code = gc_code-as_default_value. - WHEN zcl_aoc_scan=>gc_keyword-like - OR zcl_aoc_scan=>gc_keyword-type. - " Ignore - RETURN. - ENDCASE. - WHEN OTHERS. - READ TABLE mo_scan->tokens ASSIGNING INDEX iv_index_token - 1. - - IF -str = '=' - AND iv_index_token - is_statement-from = 2. - rv_error_code = gc_code-assigned_to_variable. - ELSE. - rv_error_code = gc_code-usage_uncategorized. - ENDIF. - ENDCASE. - ENDMETHOD. - - METHOD is_using_only_first_letter. - CASE is_token-str. - WHEN 'SY-SYSID+0(1)' OR 'SY-SYSID(1)'. - rv_result = abap_true. - ENDCASE. - ENDMETHOD. - - METHOD is_used_in_macro. - ASSIGN mo_scan->tokens[ is_statement-to + 1 ] TO FIELD-SYMBOL(). - - IF sy-subrc = 0 - AND -str = zcl_aoc_scan=>gc_keyword-end_of_definition. - rv_result = abap_true. - ENDIF. - ENDMETHOD. -ENDCLASS. diff --git a/src/checks/zcl_aoc_check_102.clas.testclasses.abap b/src/checks/zcl_aoc_check_102.clas.testclasses.abap index 636ebb9c..daa5f985 100644 --- a/src/checks/zcl_aoc_check_102.clas.testclasses.abap +++ b/src/checks/zcl_aoc_check_102.clas.testclasses.abap @@ -22,6 +22,7 @@ CLASS ltcl_test DEFINITION METHODS if_condition_01 FOR TESTING. METHODS first_letter_01 FOR TESTING. METHODS first_letter_02 FOR TESTING. + METHODS first_letter_03 FOR TESTING. METHODS if_condition_02 FOR TESTING. METHODS if_condition_03 FOR TESTING. METHODS case_condition_01 FOR TESTING. @@ -52,7 +53,6 @@ CLASS ltcl_test DEFINITION METHODS message FOR TESTING. METHODS check_condition FOR TESTING. METHODS assert_condition FOR TESTING. - ENDCLASS. @@ -119,6 +119,23 @@ CLASS ltcl_test IMPLEMENTATION. assert_error_code( gc_code-first_letter_used ). ENDMETHOD. + METHOD first_letter_03. + " Given + _code `SELECT host`. + _code ` FROM ztable`. + _code ` UP TO 1 ROWS`. + _code ` INTO @DATA(lv_host)`. + _code ` WHERE sysname = @sy-sysid+0(1)`. + _code ` ORDER BY PRIMARY KEY.`. + _code `ENDSELECT.`. + + " When + execute_check( ). + + " Then + assert_error_code( gc_code-first_letter_used ). + ENDMETHOD. + METHOD if_condition_02. " Given _code `IF 1 = 2 OR sy-sysid = 'PRD'.`. diff --git a/src/checks/zcl_aoc_check_102.clas.xml b/src/checks/zcl_aoc_check_102.clas.xml index ea1f751c..b9e8c7b6 100644 --- a/src/checks/zcl_aoc_check_102.clas.xml +++ b/src/checks/zcl_aoc_check_102.clas.xml @@ -5,7 +5,7 @@ ZCL_AOC_CHECK_102 E - 102 - Use of system ID + 102 - Use of SY-SYSID 1 X X diff --git a/src/utils/zcl_aoc_sy_variable_analyzer.clas.abap b/src/utils/zcl_aoc_sy_variable_analyzer.clas.abap new file mode 100644 index 00000000..ec159628 --- /dev/null +++ b/src/utils/zcl_aoc_sy_variable_analyzer.clas.abap @@ -0,0 +1,148 @@ +"!

Analyzer for system variables (SY structure)

+CLASS zcl_aoc_sy_variable_analyzer DEFINITION + PUBLIC + FINAL + CREATE PUBLIC. + + PUBLIC SECTION. + TYPES ty_v_usage_kind TYPE i. + + TYPES: BEGIN OF ty_s_result, + statement_level TYPE stmnt_levl, + token_index TYPE sy-tabix, + usage_kind TYPE ty_v_usage_kind, + END OF ty_s_result. + + TYPES ty_t_result TYPE TABLE OF ty_s_result WITH EMPTY KEY. + + CONSTANTS: + BEGIN OF gc_usage_kind, + usage_uncategorized TYPE ty_v_usage_kind VALUE 1, + in_condition TYPE ty_v_usage_kind VALUE 2, + type_definition TYPE ty_v_usage_kind VALUE 3, + as_default_value TYPE ty_v_usage_kind VALUE 4, + in_concatenate TYPE ty_v_usage_kind VALUE 5, + overridden TYPE ty_v_usage_kind VALUE 6, + assigned_to_variable TYPE ty_v_usage_kind VALUE 7, + in_database_select TYPE ty_v_usage_kind VALUE 8, + in_write TYPE ty_v_usage_kind VALUE 9, + in_message TYPE ty_v_usage_kind VALUE 10, + within_macro TYPE ty_v_usage_kind VALUE 11, + END OF gc_usage_kind. + + METHODS constructor + IMPORTING + io_scan TYPE REF TO zcl_aoc_scan. + + METHODS analyze_variable_usage + IMPORTING + iv_sy_variable_name TYPE stokesx-str + RETURNING + VALUE(rt_result) TYPE ty_t_result. + + PRIVATE SECTION. + DATA mo_scan TYPE REF TO zcl_aoc_scan. + + METHODS determine_usage_kind + IMPORTING + is_token TYPE stokesx + iv_index_token TYPE sy-tabix + is_statement TYPE sstmnt + RETURNING + VALUE(rv_usage_kind) TYPE ty_v_usage_kind. + + METHODS is_used_in_macro + IMPORTING + is_statement TYPE sstmnt + RETURNING + VALUE(rv_result) TYPE abap_bool. +ENDCLASS. + + +CLASS zcl_aoc_sy_variable_analyzer IMPLEMENTATION. + METHOD constructor. + mo_scan = io_scan. + ENDMETHOD. + + METHOD analyze_variable_usage. + LOOP AT mo_scan->statements ASSIGNING FIELD-SYMBOL(). + LOOP AT mo_scan->tokens ASSIGNING FIELD-SYMBOL() + FROM -from TO -to + WHERE str CP |SY-{ iv_sy_variable_name }*| + OR str CP |@SY-{ iv_sy_variable_name }*|. + + DATA(lv_token_index) = sy-tabix. + + DATA(lv_usage_kind) = determine_usage_kind( is_token = + iv_index_token = lv_token_index + is_statement = ). + + INSERT VALUE #( statement_level = -level + token_index = lv_token_index + usage_kind = lv_usage_kind ) INTO TABLE rt_result. + ENDLOOP. + ENDLOOP. + ENDMETHOD. + + METHOD determine_usage_kind. + IF is_used_in_macro( is_statement ) = abap_true. + " In macros, all actual statements are detected as a single statement. + " That's way too complicated, so we just leave it at this... + rv_usage_kind = gc_usage_kind-within_macro. + RETURN. + ENDIF. + + ASSIGN mo_scan->tokens[ iv_index_token - 1 ] TO FIELD-SYMBOL(). + ASSIGN mo_scan->tokens[ is_statement-from ] TO FIELD-SYMBOL(). + + CASE -str. + WHEN zcl_aoc_scan=>gc_keyword-types + OR zcl_aoc_scan=>gc_keyword-ranges. + rv_usage_kind = gc_usage_kind-type_definition. + WHEN zcl_aoc_scan=>gc_keyword-concatenate. + rv_usage_kind = gc_usage_kind-in_concatenate. + WHEN is_token-str. + rv_usage_kind = gc_usage_kind-overridden. + WHEN zcl_aoc_scan=>gc_keyword-select. + rv_usage_kind = gc_usage_kind-in_database_select. + WHEN zcl_aoc_scan=>gc_keyword-write. + rv_usage_kind = gc_usage_kind-in_write. + WHEN zcl_aoc_scan=>gc_keyword-message. + rv_usage_kind = gc_usage_kind-in_message. + WHEN zcl_aoc_scan=>gc_keyword-if + OR zcl_aoc_scan=>gc_keyword-elseif + OR zcl_aoc_scan=>gc_keyword-case + OR zcl_aoc_scan=>gc_keyword-when + OR zcl_aoc_scan=>gc_keyword-check + OR zcl_aoc_scan=>gc_keyword-assert. + rv_usage_kind = gc_usage_kind-in_condition. + WHEN zcl_aoc_scan=>gc_keyword-methods + OR zcl_aoc_scan=>gc_keyword-class_methods + OR zcl_aoc_scan=>gc_keyword-data + OR zcl_aoc_scan=>gc_keyword-class_data. + + CASE -str. + WHEN zcl_aoc_scan=>gc_keyword-default. + rv_usage_kind = gc_usage_kind-as_default_value. + WHEN OTHERS. + rv_usage_kind = gc_usage_kind-type_definition. + ENDCASE. + WHEN OTHERS. + IF -str = '=' + AND iv_index_token - is_statement-from = 2. + rv_usage_kind = gc_usage_kind-assigned_to_variable. + ELSE. + rv_usage_kind = gc_usage_kind-usage_uncategorized. + ENDIF. + ENDCASE. + ENDMETHOD. + + METHOD is_used_in_macro. + ASSIGN mo_scan->tokens[ is_statement-to + 1 ] TO FIELD-SYMBOL(). + + IF sy-subrc = 0 + AND -str = zcl_aoc_scan=>gc_keyword-end_of_definition. + rv_result = abap_true. + ENDIF. + ENDMETHOD. +ENDCLASS. diff --git a/src/utils/zcl_aoc_sy_variable_analyzer.clas.xml b/src/utils/zcl_aoc_sy_variable_analyzer.clas.xml new file mode 100644 index 00000000..cf452447 --- /dev/null +++ b/src/utils/zcl_aoc_sy_variable_analyzer.clas.xml @@ -0,0 +1,16 @@ + + + + + + ZCL_AOC_SY_VARIABLE_ANALYZER + E + Analyzer for system variables (SY structure) + 1 + X + X + X + + + +