Fine-grained rule exceptions / pragma-like behaviour

[pokrakam]

It would be nice to have finer grained rule exemptions at code level rather than an entire file in abaplint.json.

Example use case: empty_structure is good, but is also a valid construct for checking complex conditions against a table

LOOP AT itab WHERE qty = 0 OR date > sy-datum.
ENDLOOP. "#lint empty_structure ok
result = xsdbool( sy-subrc = 0 ).

[larshp]

I'd rather add a feature in the rule: if the loop is empty and its a complex condition, and subrc is checked after => then ok

that way anything "pragma like" can be avoided altogether

[pokrakam]

That would be useful as the loop case is sensible in ABAP. Not sure if it's worth the effort or even sensible to distinguish that LOOP ... WHERE f1 = 1 should be refactored but ... WHERE f1 > 1 is OK?
I think it would be good enough just to check for a condition.
LOOP + WHERE + sy-subrc => valid.

The reason for pragma-like behaviour was more generic as I tend to bump into 'OK' exceptions here and there, this was just one of the more obvious examples that made me flag it up. Some more cases where I find the rule useful, but would like an exception:

commented_code is OK if providing sample code, or "Uncomment this if you need <somespecificrequirement> within a program.

line_break_multiple_parameters => in my opinion it's OK with some builtin functions if not too many variables are used, e.g. I prefer the first variant:

IF substring( val = mystring len = 3 ) = 'FOO'.
  RETURN.
ENDIF.

IF substring( val = mystring
              len = 3 ) = 'FOO'.
  RETURN.
ENDIF.

But then again adding a lint pragma would make a mess of that one. Another use case is visually list parameters of repeated method calls, more common in unit test code:

  list.add( f1 = 'FOO' f2 = 'B'   f3 = 2 ).
  list.add( f1 = 'ABC' f2 = 'BOO' f3 = 2 ).
  list.add( f1 = 'Z'   f2 = 'B'   f3 = 2 ).

I guess it's not that simple though, and there are ways to restructure it, but it feels wrong to build methods just to make the linter happy when the code is perfectly readable. Anyways, definitely a nice to have 🙂

[larshp]

commented_code: there is a trick to remove the "." and then the rule doesnt trigger

line_break_multiple_parameters: can add a "allow in condition if less than x parameters" setting

for unit tests, its possible to disable the check in all ".testclasses." via the "exclude" setting

the example looks like it could be replaced with a VALUE #(), depending on the release tho

I do see this pattern in unit tests tho, typically in a scenario where multiple things are tested in the same method, I'd recommend testing one thing per method, which might avoid the repeated method calls

empty_structure: loop and subrc check added in #3310

[pokrakam]

Cool, thanks for the quick update on the loop - this was my biggest bugbear and works well.

I like the line break rule, especially for code reviews. It's just my opinion of course, I usually don't like single line params for methods so would like to keep the rule, but for the builtins like repeat or substring it's fine. Maybe because I'm used to it elsewhere. Not a biggie though, more important things to do 🙂

Good idea on disabling the test in all unit test classes, didn't think of just disabling the one test. Test code should follow the rules too, but some circumstances are different (and yeah mine was a bad example but you know what I meant).

And nice tip for the . - it also works just adding an empty comment . " at the end.

[larshp]

And nice tip for the . - it also works just adding an empty comment . " at the end.

I do that quite often, not sure if I really like it, but having the rule does help me to remember temporarily commented code