Skip to content

Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT #355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions docs/check_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
57 changes: 57 additions & 0 deletions docs/checks/prefer-line-exists.md
Original file line number Diff line number Diff line change
@@ -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)
66 changes: 66 additions & 0 deletions src/checks/y_check_prefer_line_exists.clas.abap
Original file line number Diff line number Diff line change
@@ -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(<token>)
FROM statement-from TO statement-to.
result = |{ result } { <token>-str }|.
ENDLOOP.
ENDMETHOD.

ENDCLASS.
141 changes: 141 additions & 0 deletions src/checks/y_check_prefer_line_exists.clas.testclasses.abap
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 17 additions & 0 deletions src/checks/y_check_prefer_line_exists.clas.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<abapGit version="v1.0.0" serializer="LCL_OBJECT_CLAS" serializer_version="v1.0.0">
<asx:abap xmlns:asx="http://www.sap.com/abapxml" version="1.0">
<asx:values>
<VSEOCLASS>
<CLSNAME>Y_CHECK_PREFER_LINE_EXISTS</CLSNAME>
<LANGU>E</LANGU>
<DESCRIPT>Prefer LINE_EXISTS or LINE_INDEX to READ TABLE or LOOP AT</DESCRIPT>
<STATE>1</STATE>
<CLSCCINCL>X</CLSCCINCL>
<FIXPT>X</FIXPT>
<UNICODE>X</UNICODE>
<WITH_UNIT_TESTS>X</WITH_UNIT_TESTS>
</VSEOCLASS>
</asx:values>
</asx:abap>
</abapGit>
11 changes: 11 additions & 0 deletions src/examples/y_demo_failures.clas.abap
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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

Expand Down