Feature request: Remove end-of comments

[sap]

Feature request: Remove end-of comments

[jmgrassau]

Hi ConjuringCoffee,

oh yes, that's a nice one! I just checked a few MB of code and found countless examples where such end-of-comments could be removed after ENDCLASS, ENDINTERFACE, ENDMETHOD, ENDFORM, ENDCASE, ENDLOOP, ENDIF etc.

However, some cases did actually contain relevant information, so the rule would have to check whether the comment simply repeats parts of the corresponding CLASS, INTERFACE, METHOD etc. statement and only remove those. And it should allow for some flexibility, e.g. sometimes ABAP words are repeated (and prefixes / underscores are omitted), as in

CLASS lcl_any_class IMPLEMENTATION.
  METHOD handle_errors.
    LOOP AT lts_error_contract ...
    ENDLOOP. " AT error contract
  ENDMETHOD. " handle errors
ENDCLASS. " lcl_any_class IMPLEMENTATION

What we won't be able to eliminate this way, regrettably, are copy errors such as:

METHOD other_method.
  ...
ENDMETHOD.      " any_method

Kind regards,
Jörg-Michael

[fabianlupa]

Similar thing with these generated headers no one ever adjusts and is too lazy to delete by the way.

[jmgrassau]

Hi @ConjuringCoffee, @jelliottp and @fabianlupa,

here's what the upcoming "Remove end-of comments" rule does :-)

If in the options …

… you select "always remove", you could even get rid of the comments in line 24 and 38. Maybe you'd only want that on ENDMETHOD and ENDCLASS level, where you'd frequently find auto-generated end-of comments.

With "remove if redundant" (= default), such comments are only removed if they more or less repeat the (start of) the opening command – "more or less" meaning that ABAP words could be skipped, underscores could be spaces, prefixes like lts_ could be omitted, upper/lower case is ignored etc. (see examples above).

Generated headers should be removed by a different cleanup rule, I think.

Kind regards,
Jörg-Michael

[jmgrassau]

Hi Josh,

exactly – if the opening command reads LOOP AT lts_any_table INTO DATA(ls_struc), then any of the following comments would be regarded as redundant:

ENDLOOP. " lts_any_table
ENDLOOP. " LTS_ANY_TABLE
ENDLOOP. " any table
ENDLOOP. " AT any table
ENDLOOP. " LOOP AT any table
ENDLOOP. " LOOP lts_any_table
ENDLOOP. " lts_any_table INTO DATA(ls_struc)
...

So the rule tries to 'match' the whole comment, but tolerates any skipped ABAP words. Identifiers must not be skipped, but their underscores may be spaces, and the bit before the first underscore may be skipped (assuming it could be a prefix). As soon as the whole comment is consumed, it is regarded a match (regardless of whether or not the opening command continues).

This approach removes most redundant end-of comments from the code samples that I looked at for this cleanup rule. If an end-of comment is NOT removed, it is usually because it was copied and not adjusted, e.g.

LOOP AT lts_other_table ...
ENDLOOP. " any table

Kind regards,
Jörg-Michael

[jelliottp]

I support this as a feature as well. I rarely, if ever, have seen a useful comment on the end of a statement. Perhaps this could be an option of the rule to either remove all, or only “repeated” information.

[fabianlupa]

I suppose you can always remove them if the comments are exactly as generated by the IDE. I doubt anyone adjusted the generation patterns in the last two decades ;)

[se38]

What will be the default?
Suggestion: "always keep" inside methods and "remove If redundant" at ENDMETHOD/ENDCLASS

[jelliottp]

With "remove if redundant" (= default), such comments are only removed if they more or less repeat the (start of) the opening command – "more or less" meaning that ABAP words could be skipped, underscores could be spaces, prefixes like lts_ could be omitted, upper/lower case is ignored etc. (see examples above).

To clarify: does this mean for example, if line 19 contained " LOOP at ls_item-inner_item it would also get removed?

[jmgrassau]

Hi Uwe,

so far, I'd have the rule activated by default and use "remove if redundant" in both contexts, because the style guide explicitly mentions examples inside methods, too. Removing redundant comments only, we would be sure to not lose relevant information. What scenario would you think of when you suggest "always keep" inside methods?

A possible option would be to introduce a line count limit – e.g. you could say, end-of comments are kept if the opening command is more than 30 lines away (and therefore out of sight).

Kind regards,
Jörg-Michael

[se38]

The comments inside a method are all "hand made", so someone thought, this comment would be helpful and should be kept by default.
The ENDMETHOD/ENDCLASS/ENDPERFORM are generated in GUI-mode and are useless (imho)

[jelliottp]

Thanks for clarifying. I think this looks great and helps reduce manual cleanup time.

[jmgrassau]

Hi Uwe,

yes, I see your point, it would definitely be the more careful default. On the other hand, even 'hand made' comments of this sort are advised against by the style guide:

" anti-pattern
METHOD get_kpi_calc.
  IF has_entries = abap_false.
    result = 42.
  ENDIF.  " IF has_entries = abap_false
ENDMETHOD.   " get_kpi_calc

From what I've seen, you usually find them in veeeery long methods with breathtaking nesting depth and a strict 'structured programming' approach (where methods should always have exactly one entry point and one exit point, and it is therefore seen as good style to make an IF its_contract IS NOT INITIAL ... ENDIF span the entire 400 lines of the method), e.g.

* the opening statements for the following are 400 lines further up the method

            ENDLOOP. " ls_item-subitem
          ENDIF. " ls_item-validaty < sy-datum
        ENDIF. " ls_item-processable = 'X'
      ENDLOOP. " item
    ENDLOOP. " contract
  ENDIF. " contract IS NOT INITIAL
ENDMETHOD. " process_items

It could be argued, of course, that the line-end comments are the least of your worries (and maybe indeed helpful) if you have to deal with code like that… So maybe an "line limit" option would make sense?

Kind regards,
Jörg-Michael

[se38]

I know exactly these kind of programs 🤬
Line limit would be nice, but low prio from my side