Better documentation

README.md

@@ -4,8 +4,7 @@
 [![license](https://img.shields.io/github/license/SAP/code-pal-for-abap)](LICENSE)
 [![REUSE status](https://api.reuse.software/badge/github.com/SAP/code-pal-for-abap)](https://api.reuse.software/info/github.com/SAP/code-pal-for-abap)
 
-Based on the [Clean ABAP](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md), this tool contains a set of checks to guarantee the [Clean ABAP](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md) adherence.  
-Together, we both support you in writing a clean ABAP code. 
+This tool provides a set of checks to help adhering to the [Clean ABAP style guide](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md). While not all recommendations from the guide can be checked by static code analysis and in certain situations strict adherence to the guide might not be recommended, this provides robust automated support at least for a subset of recommendations.
 
 ❣️ It's **free** and **open-source**.
 

docs/checks/avoid-default-key.md

@@ -1,27 +1,26 @@
 [code pal for ABAP](../../README.md) > [Documentation](../check_documentation.md) > [Avoid DEFAULT KEY](avoid-default-key.md)
 
-## Avoid DEFAULT KEY
+## Avoid default table keys
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
-> Default keys are often only added to get the newer functional statements working. The keys themselves in fact are usually superfluous and waste resources for nothing. They can even lead to obscure mistakes because they ignore numeric data types. The `SORT` and `DELETE ADJACENT` statements without explicit field list will resort to the primary key of the internal table, which in case of usage of `DEFAULT KEY` can lead to very unexpected results when having e.g. numeric fields as component of the key, in particular in combination with `READ TABLE ... BINARY` etc.
+Default table keys (declared by the `WITH DEFAULT KEY` clause in a table definition) can lead to obscure mistakes because they ignore numeric data types and they make it the intent of the key unclear. `SORT` and `DELETE ADJACENT` statements without explicit field list will resort to the primary key of the internal table, which in the case of default keys can lead to unexpected results, in particular in combination with `READ TABLE ... BINARY SEARCH` statements.
 
-Source: [Clean ABAP - Avoid DEFAULT KEY](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-default-key).
-
-Therefore, this check searches for the internal table definitions which forces the use of the default key. 
+Therefore, this check searches for internal table definitions that declare a default table key. 
 
 ### How to solve the issue?
 
-> Either specify the key components explicitly  
-> ```abap
-> DATA itab1 TYPE STANDARD TABLE OF row_type WITH NON-UNIQUE KEY comp1 comp2.
-> ``` 
-> or resort to `EMPTY KEY` if you don't need a key at all.  
-> ```abap
-> DATA itab1 TYPE STANDARD TABLE OF row_type WITH EMPTY KEY.
-> ```
+Either specify the key components explicitly  
+```abap
+DATA itab1 TYPE STANDARD TABLE OF row_type WITH NON-UNIQUE KEY comp1 comp2.
+``` 
+or resort to `EMPTY KEY` if you don't need a key at all.  
+```abap
+DATA itab1 TYPE STANDARD TABLE OF row_type WITH EMPTY KEY.
+```
+If you declare an explicit key, also consider whether you actually should use a sorted or hashed key.
 
-Source: [Clean ABAP - Avoid DEFAULT KEY](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-default-key).
+See also: [Clean ABAP - Avoid DEFAULT KEY](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-default-key).
 
 ### What to do in case of exception?
 
@@ -34,7 +33,7 @@ CLASS-DATA itab1 TYPE STANDARD TABLE OF row_type WITH DEFAULT KEY. "#EC DEFAULT_
 DATA itab1 TYPE STANDARD TABLE OF row_type WITH DEFAULT KEY. "#EC DEFAULT_KEY
 ```
 ```abap
-TYPES: BEGIN OF type1, ' )
+TYPES: BEGIN OF type1,
          non_unique TYPE STANDARD TABLE OF row_type WITH NON-UNIQUE KEY object,
          default_key TYPE STANDARD TABLE OF row_type WITH DEFAULT KEY, "#EC DEFAULT_KEY'
          empty_key TYPE STANDARD TABLE OF row_type WITH EMPTY KEY,

docs/checks/boolean-input-parameter.md

@@ -2,11 +2,11 @@
 
 ## Boolean Input Parameter
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
-This check searches for the usage of boolean input parameters in a method signature. These parameters, could be an indicator of bad design where the Single Responsibility Principle is not followed (the method might be doing several things instead of a single thing).   
+This check searches for boolean input parameters in method signatures. These parameters could be an indicator that the [single-responsibility principle (SRP)](https://en.wikipedia.org/wiki/Single-responsibility_principle) is not followed since the method might be doing several things at once.   
 
-REMARK: Setter methods using boolean input variables are acceptable.
+Setter methods using boolean input variables are acceptable when the variable being set is a boolean.
 
 ### How to solve the issue?
 

docs/checks/call-method-usage.md

@@ -2,17 +2,17 @@
 
 ## CALL METHOD Usage Check
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
-This check verifies the usage of the `CALL METHOD` ABAP statement. It is preferable to call a method dynamically instead of via this statement: CALL METHOD. In other words, prefer a functional instead of a procedural call.
+This check searches for `CALL METHOD` statements. Static occurrences, i.e. those where the called method is known statically at compile-time, of this statement are obsolete and should be replaced by their functional equivalents.
 
 ### How does the check work?
 
-It checks the usage of `CALL METHOD` statement in the code.
+The check finds `CALL METHOD` statements that do not contain dynamic parts. Dynamic parts are indicated by the method being called being specified by a literal or character-like variable in parentheses or the usage of the additions `PARAMETER-TABLE` or `EXCEPTION-TABLE`.
 
 ### How to solve the issue?
 
-Change the long method calls using `CALL METHOD` statement to short method calls using parenthesis notation (dynamic call).
+Change the method calls using `CALL METHOD` to functional method calls.
 
 ### What to do in case of exception?
 
@@ -28,14 +28,15 @@ Before the check:
 
 ```abap
 DATA(class) = NEW object( ).
-CALL METHOD class->method.
+CALL METHOD class->method
+    EXPORTING param = var.
 ```
 
 After the check:
 
 ```abap
 DATA(class) = NEW object( ).
-class->method( ).
+class->method( var ).
 ```
 
 ### Further Readings & Knowledge

docs/checks/chain-declaration-usage.md

@@ -2,13 +2,13 @@
 
 ## Chain Declaration Usage
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
-This check verifies the usage of chain up-front declarations.
+This check searches for chained up-front declarations of variables. Chaining visually implies a logical grouping that is often not actually present and variables often should rather be declared inline at their first point of usage.
 
 ### How to solve the issue?
 
-Change the chain up-front declarations to inline declarations.
+Change the chained up-front declarations to inline declarations.
 
 ### What to do in case of exception?
 
@@ -18,15 +18,15 @@ In exceptional cases, you can suppress this finding by using the pseudo comment
   DATA: "#EC CHAIN_DECL_USAG
     string TYPE string,
     json TYPE REF TO cl_abap_json,
-    client LIKE sy-mandt.
+    client LIKE sy-mandt. 
 ```
 
 ```abap
   TYPES: "#EC CHAIN_DECL_USAG
     name TYPE string,
     json TYPE REF TO cl_abap_json.
 ```
-
+or 
 ```abap
   CONSTANTS: "#EC CHAIN_DECL_USAG
     min_age TYPE i VALUE 18,
@@ -42,23 +42,30 @@ Before the check:
     string TYPE string,
     json TYPE REF TO cl_abap_json,
     client LIKE sy-mandt.
+  string = `Hello world`.
+  create object json
+    exporting iv_json = string.
+  client = sy-mandt.     
 ```
 
 After the check:
 
 ```abap
-  DATA string TYPE string.
-  DATA json TYPE REF TO cl_abap_json.
-  DATA client LIKE sy-mandt.
+  data(string) = `Hello world`.
+  data(json) = new cl_abap_json( string ).
+  data(client) = sy-mandt. 
 ```
 
-Or even (which looks neat - but it won't be enforced):
+or 
 
 ```abap
-DATA var1         TYPE a.
-DATA var2         TYPE string.
-DATA my_var3      TYPE int.
-DATA a            TYPE c.
+  DATA string TYPE string.
+  DATA json TYPE REF TO cl_abap_json.
+  DATA client LIKE sy-mandt.
+  string = `Hello world`.
+  create object json
+    exporting iv_json = string.
+  client = sy-mandt.   
 ```
 
 ### Further Readings & Knowledge

docs/checks/check-in-loop.md

@@ -1,16 +1,21 @@
 [code pal for ABAP](../../README.md) > [Documentation](../check_documentation.md) > [CHECK in LOOP](check-in-loop.md)
 
-## CHECK in LOOP
+## Check in Loop
 
-### What is the Intent of the Check?
-This check verifies if a `CHECK` statement is being used inside of a `LOOP` structure. A CHECK within a LOOP, ends the current iteration and proceeds to the next one. This behavior might lead to some confusion like: Does it end the method processing or does it exit the loop?
+### What is the intent of the check?
+This check searches for `CHECK` statements inside iterations (such as `LOOP...ENDLOOP` or `DO...ENDDO`). A `CHECK` within a loop ends the current instance of the iteration and proceeds to the next one. Since `CHECK` statements that are not inside a loop leave the current modularization unit, e.g. inside a method act like a conditional `RETURN` statement, it can be confusing to determine the effect of any given `CHECK` statement.
 
 ### How to solve the issue?
-Prefer using `CONTINUE` (within an IF-Statement) instead of using the CHECK Statement in this case. Since the keyword `CONTINUE` can only be used in LOOPS, the intention is then automatic clear to everyone reading the code.
-Keep also in mind, that other Keywords like: `EXIT` or `RETURN` are also more explicit than `CHECK`.
+Replace the `CHECK` statement by its equivalent `IF` statement
+```abap
+IF not condition.
+  CONTINUE.
+ENDIF.
+```
+and take care to choose a form of the logical expression that is easy to interpret. Since the keyword `CONTINUE` can only be used in loops, the intent of continuing to the next instance of the iteration is now unambiguous.
 
 ### What to do in case of exception?
-In exceptional cases, you can suppress this finding by using the pseudo comment `"#EC CHECK_IN_LOOP` which should be placed after the `CHECK` Statement itself:
+In exceptional cases, you can suppress this finding by using the pseudo comment `"#EC CHECK_IN_LOOP` which should be placed after the `CHECK` statement itself:
 
 ```abap
 LOOP AT tadir ASSIGNING FIELD-SYMBOL(<tadir>).
@@ -23,6 +28,7 @@ Before the check:
 ```abap
 LOOP AT tadir ASSIGNING FIELD-SYMBOL(<tadir>).
   CHECK <tadir>-delflag = abap_true.
+  "some more code...
 ENDLOOP.
 ```
 
@@ -32,9 +38,18 @@ LOOP AT tadir ASSIGNING FIELD-SYMBOL(<tadir>).
   IF <tadir>-delflag = abap_false.
     CONTINUE.
   ENDIF.
+  "some more code...
+ENDLOOP.
+```
+
+Note that in this example, the cleanest way of expressing the intent behind the code would be to remove the statement inside the loop completely and replace it by a `WHERE` clause in the `LOOP` statement:
+
+```abap
+LOOP AT tadir ASSIGNING FIELD-SYMBOL(<tadir>) WHERE delflag = abap_false.
+  "some more code...
 ENDLOOP.
 ```
 
 ### Further Readings & Knowledge
 - [Clean ABAP: Avoid CHECK in other positions](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-check-in-other-positions)
-- [Exiting Loops -> Check](https://help.sap.com/doc/abapdocu_752_index_htm/7.52/en-US/abapcheck_loop.htm)
+- [Exiting Loops -> Check](https://help.sap.com/doc/abapdocu_756_index_htm/7.56/en-US/abapcheck_loop.htm)

docs/checks/check-statement-position.md

@@ -2,19 +2,20 @@
 
 ## CHECK Statement Position Check
 
-### What is the Intent of the Check?
-This check verifies whether the `CHECK` statement is the very first statement within a method, function-module or form-routine.  
+### What is the intent of the check?
+This check searches for `CHECK` statements that are not the first statement within a method, function module or form subroutine since the statement behaves differently in different positions and may lead to unclear, unexpected effects.
 
-The [Clean ABAP](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-check-in-other-positions) says:
-> Do not use `CHECK` outside of the initialization section of a method. The statement behaves differently in different positions and may lead to unclear, unexpected effects.
+### How does the check work
 
-REMARKS: 
-1. CHECK statement inside of LOOPs will be disregard by check (for that, refer to: CHECK_IN_LOOP). 
-2. The usage of CLEAR statement prior to CHECK statement is considered to be a bad coding practice! This is actually a workaround in bad designed code (against OO scope principles). 
-3. DATA declarations (DATA / FIELD-SYMBOLS when not dynamic declared – inline declarations), might also come before the keyword CHECK.
+When the check finds a `CHECK` statement, it will report a finding unless one of the following circumstances applies:
+
+1. The statement occurs within a loop - see [Check in Loop](check-in-loop.md) for a check dealing with these statements.
+2. The only statement before the `CHECK` statement are variable declarations. However, see the check for [chained declarations](chain-declaration-usage.md) for better practices when declaring variables.
+
+While it might sometimes seem necessary to have a `CLEAR` statement for exporting parameters in front of any `CHECK` statements, a need for both of these statements indicates a method that should be refactored to be less confusing, so the check still reports a finding for these cases.
 
 ### How to solve the issue?
-The `CHECK` statement shall be the first statement of a method. If it is not possible, try to substitute this keyword with an IF-statement instead.
+Either move the `CHECK` statement to be the first statement of the method or replace it with its equivalent `IF` statement.
 
 ### What to do in case of exception?
 In exceptional cases, you can suppress this finding by using the pseudo comment `"#EC CHECK_POSITION` which has to be placed after the `CHECK` statement:
@@ -45,7 +46,17 @@ METHOD example.
   ...
 ENDMETHOD.
 ```
-OR
+or
+
+```abap
+METHOD example.
+  ...
+  IF sy-mandt = 000.
+    ...
+  ENDIF.
+ENDMETHOD.
+```
+or
 
 ```abap
 METHOD example.
@@ -54,5 +65,6 @@ METHOD example.
 ENDMETHOD.
 ``` 
 
+Note how the second option expresses most clearly the intent both syntactically and visually - the rest of the method (the part indented inside the `IF` statement) is to be executed if the condition is true.
 ### Further Readings & Knowledge
 - [Clean ABAP: Avoid CHECK in other positions (Clean ABAP)](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#avoid-check-in-other-positions)

docs/checks/collect.md

@@ -2,31 +2,29 @@
 
 ## COLLECT restriction
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
 The [ABAP - Keyword Documentation](https://help.sap.com/doc/abapdocu_755_index_htm/7.55/en-US/abencollect_guidl.htm?file=abencollect_guidl.htm) says:
 > **Rule**
 >
 > Do not fill standard tables with collections of lines
 > 
-> Only use the statement COLLECT for hashed tables or sorted tables with a unique key. Do not use it any more for standard tables.
+> Only use the statement `COLLECT` for hashed tables or sorted tables with a unique key. Do not use it any more for standard tables.
 > 
 > **Details**
 >
-> The statement COLLECT is based on unique entries with respect to the primary key and stable key administration. This means that not all categories of internal tables are suitable for COLLECT:
+> The statement `COLLECT` is based on unique entries with respect to the primary key and stable key administration. This means that not all categories of internal tables are suitable for `COLLECT`:
 >
-> * If the statement COLLECT is applied to a standard table, this table first needs its own internal hash administration. Change operations on the table can invalidate this temporary hash administration. After a change operation of this type, the following COLLECT statements must resort to a linear search, which can affect performance considerably. The primary key of a standard table is also never unique.
-> * COLLECT can be used for sorted tables and hashed tables without any problems since these, unlike standard tables, always have a separate, stable key administration that can be utilized by COLLECT. COLLECT can work properly for sorted tables only if the primary key is unique. If a sorted table has a non-unique key, only COLLECT can be used to fill the table, which is difficult to guarantee. In hashed tables, the key values are always unique.
+> * If the statement `COLLECT` is applied to a standard table, this table first needs its own internal hash administration. Change operations on the table can invalidate this temporary hash administration. After a change operation of this type, the following COLLECT statements must resort to a linear search, which can affect performance considerably. The primary key of a standard table is also never unique.
+> * `COLLECT` can be used for sorted tables and hashed tables without any problems since these, unlike standard tables, always have a separate, stable key administration that can be utilized by COLLECT. `COLLECT` can work properly for sorted tables only if the primary key is unique. If a sorted table has a non-unique key, only COLLECT can be used to fill the table, which is difficult to guarantee. In hashed tables, the key values are always unique.
 
 ### How does the check work?
 
-It searches for `COLLECT` in:
-* Internal tables typed as `SORTED TABLE` with `NON-UNIQUE KEY`;
-* Internal tables typed as `STANDARD TABLE`;
+The check searches for `COLLECT` statements whose argument are internal tables typed as `SORTED TABLE ... WITH NON-UNIQUE KEY` or `STANDARD TABLE`.
 
 ### How to solve the issue?
 
-Change the internal table to `SORTED` or `HASHED` as recommended, or perform the collection manually.   
+Change the internal table to be a sorted or hashed table as recommended by the ABAP keyword documentation, or perform the collection manually.   
 
 ### What to do in case of exception?
 

docs/checks/comment-position.md

@@ -2,16 +2,18 @@
 
 ## Comment Position
 
-### What is the Intent of the Check?
+### What is the intent of the check?
 
-This check searches for "Quote comments" which are not indented along with the statements they belong to.
+This check finds comments starting with `"` that are not indented at the same level as the statements that follow them. Comments should generally refer to the code that follows them, and hence should be indented at the same level.
+
+The check does not report findings for comments that are in-line comments or are the only "code" inside an empty branch of a conditional statement.
 
 ### How to solve the issue?
 
-You should indent the comments along with the statements they are commenting.
+You should indent comments along with the statements they are commenting.
 
 ### What to do in case of exception?
-There is no exception for this check since it works as an indicator only. Thus, it is also not possible to suppress its findings.
+There are no pseudo comments for this check since you cannot put pseudo comments on the same line as an actual comment.
 
 ### Example
 
@@ -22,17 +24,17 @@ Before the check:
   output = calculate_result( input ).
 ```
 
-```abap
-  output = calculate_result( input ). " delegate pattern
-```
-
 After the check:
 
 ```abap
   " delegate pattern
   output = calculate_result( input ).
 ```
+or
+```abap
+  output = calculate_result( input ). " delegate pattern
+```
 
 ### Further Readings & Knowledge
 
-* [Clean ABAP](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#put-comments-before-the-statement-they-relate-to)
+* [Clean ABAP - Put comments before the statement they relate to](https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#put-comments-before-the-statement-they-relate-to)