Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow CHECK keyword in begin of routines #1097

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Allow CHECK keyword in begin of routines #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
96dd95c
feat():Add logic to allow CHECK in begin of routines
alezhu Jan 17, 2023
9954a80
test():Add test for CHECK in begin of routine
alezhu Jan 17, 2023
1f9fb50
docs():Change documentation about check_02 exception
alezhu Jan 17, 2023
95d397f
Update src/checks/zcl_aoc_check_02.clas.abap
alezhu Jan 17, 2023
b352814
refactor():Tabs to spaces
alezhu Jan 17, 2023
aeae0d0
Merge branch 'check_02_smarter' of github.com:alezhu/abapOpenChecks i…
alezhu Jan 17, 2023
36ce9d1
Update src/checks/zcl_aoc_check_02.clas.abap
alezhu Jan 17, 2023
58d51c9
refactor():Fix abaplint problems
alezhu Jan 26, 2023
2f9aa15
Merge branch 'main' into check_02_smarter
larshp Apr 22, 2023
bb60f2b
Merge branch 'main' into check_02_smarter
alezhu Dec 7, 2023
448004c
Merge branch 'main' into check_02_smarter
alezhu Feb 27, 2024
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
11 changes: 11 additions & 0 deletions docs/_checks/02.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,19 @@ Note: this check is also part of SAP standard in "Extended Program Check" -> "Pr
### CHECK
See [https://help.sap.com/doc/abapdocu_750_index_htm/7.50/en-US/abapcheck_processing_blocks.htm](https://help.sap.com/doc/abapdocu_750_index_htm/7.50/en-US/abapcheck_processing_blocks.htm)


> #### **Rule** ([see](https://help.sap.com/doc/abapdocu_750_index_htm/7.50/en-US/abenexit_procedure_guidl.htm#@@ITOC@@ABENEXIT_PROCEDURE_GUIDL_2))
> Only use RETURN to exit procedures
>
> ##### **Exception**
> An exception to the rule to only use RETURN to exit procedures are CHECK statements that are located at the beginning of a procedure and that check the prerequisites for the execution of the procedure there. Using the CHECK statement in such a way does not impair the legibility and is thus allowed. However, this exception does not apply to other positions within a procedure and outside loops.

[https://github.com/SAP/styleguides/blob/master/clean-abap/CleanABAP.md#check-vs-return](https://github.com/SAP/styleguides/blob/master/clean-abap/CleanABAP.md#check-vs-return)





Note: this check is also part of SAP standard in "Extended Program Check" -> "Programming Guidelines".

### Configuration
Expand Down
114 changes: 113 additions & 1 deletion src/checks/zcl_aoc_check_02.clas.abap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
PROTECTED SECTION.
DATA mv_check TYPE flag.
DATA mv_exit TYPE flag.

METHODS _is_check_allow
IMPORTING
!io_scan TYPE REF TO zcl_aoc_scan
!iv_statement_index TYPE i
RETURNING
VALUE(rv_result) TYPE abap_bool .
PRIVATE SECTION.
ENDCLASS.

Expand Down Expand Up @@ -65,6 +72,12 @@
EXIT. " current loop
ENDLOOP.
IF sy-subrc <> 0.
IF lv_error = '002' AND _is_check_allow(
io_scan = io_scan
iv_statement_index = lv_index
) IS NOT INITIAL.
CONTINUE.
ENDIF.
lv_line = io_scan->statement_row( lv_index ).

lv_include = io_scan->get_include( <ls_statement>-level ).
Expand Down Expand Up @@ -142,4 +155,103 @@
ASSERT sy-subrc = 0.

ENDMETHOD.
ENDCLASS.


METHOD _is_check_allow.
************************************************************************
* Copyright (c) 2023 by Alexandr Zhuravlev
* MIT License
* https://github.com/alezhu/abapOpenChecks/
Comment on lines +161 to +164
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git keeps the history

you can choose to contribute under the license terms, I dont want to add multiple license parts

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse me, but I do not understand. Do you mean that I should remove those 3 lines?


*Rule
*
*Only use RETURN to exit procedures

*Exception !!!!!!
*
*An exception to the rule to only use RETURN to exit procedures are
*CHECK statements that are located at the beginning of a procedure and
*that check the prerequisites for the execution of the procedure there.
*
*Using the CHECK statement in such a way does not impair the legibility
*and is thus allowed.
************************************************************************

DATA(lp_s_check) = REF #( io_scan->statements[ iv_statement_index ] ).
DATA(lv_struct_index) = lp_s_check->struc.
"Search Routine parent
WHILE lv_struct_index > 0.
DATA(lp_s_struct) = REF #( io_scan->structures[ lv_struct_index ] ).
IF lp_s_struct->type = scan_struc_type-routine.
EXIT.
ENDIF.
lv_struct_index = lp_s_struct->back.
ENDWHILE.
IF lp_s_struct IS NOT BOUND.
RETURN.
ENDIF.

"Check all statements from routine start to current CHECK
"and skip available statements
DATA(lv_from) = SWITCH i( lp_s_struct->type
WHEN scan_struc_type-routine THEN lp_s_struct->stmnt_from + 1 " +1 skips METHOD/FORM/FUNCTION etc
ELSE lp_s_struct->stmnt_from ).
LOOP AT io_scan->statements REFERENCE INTO DATA(lp_s_statement)
FROM lv_from
TO iv_statement_index - 1. " -1 skips current CHECK

CASE lp_s_statement->type.
WHEN scan_stmnt_type-include
OR scan_stmnt_type-include_miss
OR scan_stmnt_type-type_pools
OR scan_stmnt_type-type_pools_miss
OR scan_stmnt_type-comment
OR scan_stmnt_type-comment_in_stmnt
OR scan_stmnt_type-pragma
OR scan_stmnt_type-abap_doc
OR scan_stmnt_type-macro_definition
OR scan_stmnt_type-empty.
"Skip allow
CONTINUE.
WHEN scan_stmnt_type-standard
OR scan_stmnt_type-unknown.
DATA(lv_keyword) = io_scan->statement_keyword( sy-tabix ).
CASE lv_keyword.
WHEN 'TYPES'

Check failure on line 220 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'DATA'
OR 'STATICS'
OR 'TABLES'
OR 'CONSTANTS'
OR 'FIELD-SYMBOLS'.
WHEN 'CLEAR'

Check failure on line 226 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'FREE'
OR 'REFRESH'.
Comment on lines +226 to +228
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WHEN 'CLEAR'
OR 'FREE'
OR 'REFRESH'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually clear the exporting parameters anyway and then check the importing parameters .

WHEN 'ASSERT'

Check failure on line 229 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'CHECK'
OR 'LEAVE'
OR 'RAISE'
OR 'RETURN'
OR 'EXIT'.
WHEN 'BREAK-POINT'

Check failure on line 235 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'LOG-POINT'.
WHEN 'DESCRIBE'

Check failure on line 237 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'GET'
OR 'INCLUDE'
OR 'ASSIGN'.
Comment on lines +237 to +240
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WHEN 'DESCRIBE'
OR 'GET'
OR 'INCLUDE'
OR 'ASSIGN'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'DESCRIBE' - for describe lines or get field type and then check it
'GET' - for GET PARAMETER for example and then check it
'ASSIGN' - for assign importing parameter of generic type and cast them to concret type or dereference ref and then check assigned structure field or check assigned table lines or some thing
'INCLUDE' - sometimes common checks in common include

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with pcf0: In my opinion this goes against the spirit of only allowing CHECK at the start of routines. If you need to do things before you CHECK, then it's not the start of the routine anymore.

WHEN 'IF'

Check failure on line 241 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'ENDIF'.
WHEN 'DEFINE'

Check failure on line 243 in src/checks/zcl_aoc_check_02.clas.abap

View check run for this annotation

abaplint / abaplint

Empty block, add code: When 

https://rules.abaplint.org/empty_structure
OR 'END-OF-DEFINITION'.
WHEN OTHERS.
"CHECK not allow after others
RETURN.
ENDCASE.
WHEN OTHERS.
"CHECK not allow after such statement type
RETURN.
ENDCASE.
ENDLOOP.

rv_result = abap_true.
ENDMETHOD.
ENDCLASS.
58 changes: 55 additions & 3 deletions src/checks/zcl_aoc_check_02.clas.testclasses.abap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ CLASS ltcl_test DEFINITION FOR TESTING
test002_02 FOR TESTING,
test002_03 FOR TESTING,
test002_04 FOR TESTING.
METHODS test002_05 FOR TESTING.

ENDCLASS. "lcl_Test

Expand Down Expand Up @@ -104,13 +105,13 @@ CLASS ltcl_test IMPLEMENTATION.
METHOD test002_01.
* ===========

_code 'cl_class=>method( ).'.
_code 'CHECK 1 = 2.'.

ms_result = zcl_aoc_unit_test=>check( mt_code ).

cl_abap_unit_assert=>assert_equals( exp = '002'
act = ms_result-code ).

ENDMETHOD. "test1

METHOD test002_02.
Expand Down Expand Up @@ -142,7 +143,7 @@ CLASS ltcl_test IMPLEMENTATION.
METHOD test002_04.
* ===========

_code 'WHILE 1 = 2'.
_code 'WHILE 1 = 2.'.
_code ' CHECK 1 = 2. '.
_code 'ENDWHILE. '.

Expand All @@ -152,4 +153,55 @@ CLASS ltcl_test IMPLEMENTATION.

ENDMETHOD. "test001_04

ENDCLASS. "lcl_Test
METHOD test002_05.
* ===========

_code 'CLASS LCL_FOO DEFINITION.'.
_code ' PUBLIC SECTION.'.
_code ' METHODS test.'.
_code 'ENDCLASS.'.
_code 'CLASS LCL_FOO IMPLEMENTATION.'.
_code 'METHOD test.'.
_code ' TYPE-POOLS SCAN.'.
_code ' TYPES:'.
_code ' BEGIN OF ts_struct,'.
_code ' date TYPE d,'.
_code ' END OF ts_struct.'.
_code ' CONSTANTS cv_value type string value `test`.'.
_code ' STATICS lv_index type i.'.
_code ' DATA ls_struct TYPE ts_struct'.
_code ' DATA lt_table LIKE STANDARD TABLE OF LS_STRUCT'.
_code ' FIELD-SYMBOLS <ls_data> TYPE any.'.
_code ' TABLES t100.'.
_code ' DEFINE macro.'.
_code ' READ TABLE lt_table INTO &1 INDEX &2.'.
_code ' END-OF-DEFINITION.'.
_code '*******************************'.
_code '" Test comment'.
_code ' CLEAR lv_index.'.
_code ' FREE lt_table.'.
_code ' REFRESH lt_table.'.
_code ' ASSERT 1 = 2.'.
_code ' BREAK-POINT.'.
_code ' BREAK-POINT ID Z.'.
_code ' LOG-POINT ID Z.'.
_code ' DESCRIBE FIELD lv_index LENGTH DATA(lv_length) IN BYTE MODE.'.
_code ' GET TIME.'.
_code ' INCLUDE zdummy IF FOUND.'.
_code ' ASSIGN ls_struct TO <ls_data>.'.
_code ' IF <ls_data> IS NOT ASSIGNED.'.
_code ' RAISE EXCEPTION TYPE CX_STATIC_CHECK.'.
_code ' LEAVE PROGRAM.'.
_code ' RETURN.'.
_code ' ENDIF.'.
_code ''.
_code ' CHECK <ls_data> IS ASSIGNED.'.
_code ''.
_code 'ENDMETHOD.'.
_code 'ENDCLASS.'.

ms_result = zcl_aoc_unit_test=>check( mt_code ).
cl_abap_unit_assert=>assert_initial( ms_result ).
ENDMETHOD. "test002_05

ENDCLASS. "lcl_Test