diff --git a/changelog.txt b/changelog.txt index 0e45c0c7..c6329586 100644 --- a/changelog.txt +++ b/changelog.txt @@ -14,6 +14,7 @@ Whenever you upgrade code pal for ABAP, it is highly recommended to execute the 2021-04-** v.1.14.0 ------------------ ++ Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT (#355) + Additional option to disable exceptions/pragmas (#329) ! abapLint Rules * Diffs for TABL (#359) diff --git a/docs/check_documentation.md b/docs/check_documentation.md index 453ffcd9..07dad1c5 100644 --- a/docs/check_documentation.md +++ b/docs/check_documentation.md @@ -40,6 +40,7 @@ - [Number of Output Parameter](checks/number-output-parameter.md) - [Prefer CASE to ELSEIF](checks/prefer-case-to-elseif.md) - [Prefer IS NOT to NOT IS](checks/prefer-is-not-to-not-is.md) +- [Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT](checks/prefer-line-exists.md) - [Prefer NEW to CREATE OBJECT](checks/prefer-new-to-create-object.md) - [Pseudo Comment Usage](checks/pseudo-comment-usage.md) - [Omit Optional EXPORTING](checks/omit-optional-exporting.md) diff --git a/docs/checks/prefer-line-exists.md b/docs/checks/prefer-line-exists.md new file mode 100644 index 00000000..c85633f5 --- /dev/null +++ b/docs/checks/prefer-line-exists.md @@ -0,0 +1,57 @@ +[code pal for ABAP](../../README.md) > [Documentation](../check_documentation.md) > [Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT](prefer-line-exists.md) + +## Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT + +### What is the Intent of the Check? + +Prefer `LINE_EXISTS` or `LINE_INDEX` over `READ TABLE` or `LOOP AT` as they avoid needlessly longer statements. + +### How to solve the issue? + +Preferably, use `LINE_EXISTS` to check whether the row of an internal table exists, and `LINE_INDEX` to check the row index. + +### What to do in case of exception? + +In exceptional cases, you can suppress this finding by using the pseudo comment `"#EC PREF_LINE_EX`: + +```abap + READ TABLE my_table TRANSPORTING NO FIELDS WITH KEY key = 'A'. "#EC PREF_LINE_EX +``` + +```abap + LOOP AT my_table REFERENCE INTO DATA(line) WHERE key = 'A'. "#EC PREF_LINE_EX + ... + ENDLOOP. +``` + +### Example + +Before the check: + +```abap + READ TABLE my_table TRANSPORTING NO FIELDS WITH KEY key = 'A'. + + IF sy-subrc = 0. + line_index = sy-tabix. + line_exists = abap_true. + ENDIF. +``` + +```abap + LOOP AT my_table REFERENCE INTO DATA(line) WHERE key = 'A'. + line_index = sy-tabix. + line_exists = abap_true. + EXIT. + ENDLOOP. +``` + +After the check: + +```abap + DATA(index) = line_index( my_table[ key = 'A' ] ). + DATA(exists) = xsdbool( line_exists( my_table[ key = 'A' ] ) ). +``` + +### Further Readings & Knowledge + +* [ABAP Styleguides on Clean Code](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#prefer-line_exists-to-read-table-or-loop-at) diff --git a/src/checks/y_check_prefer_line_exists.clas.abap b/src/checks/y_check_prefer_line_exists.clas.abap new file mode 100644 index 00000000..ccdaf278 --- /dev/null +++ b/src/checks/y_check_prefer_line_exists.clas.abap @@ -0,0 +1,66 @@ +CLASS y_check_prefer_line_exists DEFINITION PUBLIC INHERITING FROM y_check_base CREATE PUBLIC. + PUBLIC SECTION. + METHODS constructor. + + PROTECTED SECTION. + METHODS inspect_tokens REDEFINITION. + + METHODS get_statement_inline IMPORTING statement TYPE sstmnt + RETURNING VALUE(result) TYPE string. + +ENDCLASS. + + + +CLASS y_check_prefer_line_exists IMPLEMENTATION. + + + METHOD constructor. + super->constructor( ). + + settings-pseudo_comment = '"#EC PREF_LINE_EX' ##NO_TEXT. + settings-disable_threshold_selection = abap_true. + settings-threshold = 0. + settings-prio = c_warning. + settings-documentation = |{ c_docs_path-checks }prefer-line-exists.md|. + + set_check_message( 'Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT!' ). + ENDMETHOD. + + + METHOD inspect_tokens. + CHECK get_token_abs( statement-from ) = 'READ' + OR get_token_abs( statement-from ) = 'LOOP'. + + DATA(inline) = get_statement_inline( statement ). + + IF inline NP '* TRANSPORTING NO FIELDS *'. + RETURN. + ENDIF. + + IF inline CP '* FROM *' + OR inline CP '* TO *'. + RETURN. + ENDIF. + + DATA(configuration) = detect_check_configuration( statement ). + + IF configuration IS INITIAL. + RETURN. + ENDIF. + + raise_error( statement_level = statement-level + statement_index = index + statement_from = statement-from + error_priority = configuration-prio ). + ENDMETHOD. + + + METHOD get_statement_inline. + LOOP AT ref_scan_manager->tokens ASSIGNING FIELD-SYMBOL() + FROM statement-from TO statement-to. + result = |{ result } { -str }|. + ENDLOOP. + ENDMETHOD. + +ENDCLASS. diff --git a/src/checks/y_check_prefer_line_exists.clas.testclasses.abap b/src/checks/y_check_prefer_line_exists.clas.testclasses.abap new file mode 100644 index 00000000..cc53e9f7 --- /dev/null +++ b/src/checks/y_check_prefer_line_exists.clas.testclasses.abap @@ -0,0 +1,141 @@ +*"* use this source file for your ABAP unit test classes +CLASS ltc_read_table DEFINITION INHERITING FROM y_unit_test_base FOR TESTING RISK LEVEL HARMLESS DURATION SHORT. + PROTECTED SECTION. + METHODS get_cut REDEFINITION. + METHODS get_code_with_issue REDEFINITION. + METHODS get_code_without_issue REDEFINITION. + METHODS get_code_with_exemption REDEFINITION. +ENDCLASS. + +CLASS ltc_read_table IMPLEMENTATION. + + METHOD get_cut. + result ?= NEW y_check_prefer_line_exists( ). + ENDMETHOD. + + METHOD get_code_with_issue. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( | READ TABLE tadir TRANSPORTING NO FIELDS WITH KEY object = 'y_check_prefer_line_exists'. | ) + + ( ' IF sy-subrc = 0. ' ) + ( ' exists = abap_true. ' ) + ( ' ENDIF. ' ) + ). + ENDMETHOD. + + METHOD get_code_without_issue. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( | DATA(exists) = xsdbool( line_exists( tadir[ object = 'y_check_prefer_line_exists' ] ) ). | ) + ( | DATA(index) = line_index( tadir[ object = 'y_check_prefer_line_exists' ] ). | ) + ). + ENDMETHOD. + + METHOD get_code_with_exemption. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( | READ TABLE tadir TRANSPORTING NO FIELDS WITH KEY object = 'y_check_prefer_line_exists'. "#EC PREF_LINE_EX | ) + + ( ' IF sy-subrc = 0. ' ) + ( ' exists = abap_true. ' ) + ( ' ENDIF. ' ) + ). + ENDMETHOD. + +ENDCLASS. + + + +CLASS ltc_loop_at DEFINITION INHERITING FROM ltc_read_table FOR TESTING RISK LEVEL HARMLESS DURATION SHORT. + PROTECTED SECTION. + METHODS get_code_with_issue REDEFINITION. + METHODS get_code_with_exemption REDEFINITION. +ENDCLASS. + +CLASS ltc_loop_at IMPLEMENTATION. + + METHOD get_code_with_issue. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( | LOOP AT tadir TRANSPORTING NO FIELDS WHERE object = 'y_check_prefer_line_exists'. | ) + ( ' exists = abap_true. ' ) + ( ' ENDLOOP. ' ) + ). + ENDMETHOD. + + METHOD get_code_with_exemption. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( | LOOP AT tadir TRANSPORTING NO FIELDS WHERE object = 'y_check_prefer_line_exists'. "#EC PREF_LINE_EX | ) + ( ' exists = abap_true. ' ) + ( ' ENDLOOP. ' ) + ). + ENDMETHOD. + +ENDCLASS. + + + +CLASS ltc_loop_at_from_to DEFINITION INHERITING FROM ltc_loop_at FOR TESTING RISK LEVEL HARMLESS DURATION SHORT. + PROTECTED SECTION. + METHODS get_code_with_issue REDEFINITION. + METHODS get_code_without_issue REDEFINITION. +ENDCLASS. + +CLASS ltc_loop_at_from_to IMPLEMENTATION. + + METHOD get_code_with_issue. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA from_list TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( | LOOP AT from_list TRANSPORTING NO FIELDS WHERE object = 'y_check_prefer_line_exists'. | ) + ( ' exists = abap_true. ' ) + ( ' ENDLOOP. ' ) + ). + ENDMETHOD. + + METHOD get_code_without_issue. + result = VALUE #( + ( 'REPORT y_example. ' ) + + ( ' START-OF-SELECTION. ' ) + ( ' DATA tadir TYPE TABLE OF tadir. ' ) + ( ' DATA exists TYPE abap_bool. ' ) + + ( ' LOOP AT tadir TRANSPORTING NO FIELDS ' ) + ( ' FROM 1 TO 50 ' ) + ( | WHERE object = 'y_check_prefer_line_exists'. | ) + ( ' exists = abap_true. ' ) + ( ' ENDLOOP. ' ) + ). + ENDMETHOD. + +ENDCLASS. diff --git a/src/checks/y_check_prefer_line_exists.clas.xml b/src/checks/y_check_prefer_line_exists.clas.xml new file mode 100644 index 00000000..86b7a26d --- /dev/null +++ b/src/checks/y_check_prefer_line_exists.clas.xml @@ -0,0 +1,17 @@ + + + + + + Y_CHECK_PREFER_LINE_EXISTS + E + Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT + 1 + X + X + X + X + + + + diff --git a/src/examples/y_demo_failures.clas.abap b/src/examples/y_demo_failures.clas.abap index 4bb4d4b1..4e2363e4 100644 --- a/src/examples/y_demo_failures.clas.abap +++ b/src/examples/y_demo_failures.clas.abap @@ -115,6 +115,7 @@ CLASS y_demo_failures DEFINITION PUBLIC FINAL CREATE PUBLIC. METHODS prefer_is_not_to_not_is. METHODS prefer_case_to_elseif. METHODS prefer_new_to_create_object. + METHODS prefer_line_exists. METHODS deprecated_classes. METHODS scope_of_variable. @@ -416,6 +417,16 @@ CLASS Y_DEMO_FAILURES IMPLEMENTATION. ENDMETHOD. + METHOD prefer_line_exists. + DATA tadir TYPE TABLE OF tadir. + DATA exists TYPE abap_bool. + READ TABLE tadir TRANSPORTING NO FIELDS WITH KEY object = 'y_demo_failures'. + IF sy-subrc = 0. + exists = abap_true. + ENDIF. + ENDMETHOD. + + METHOD pseudo_comment_usage. ENDMETHOD. "#EC EMPTY_PROCEDURE