After implementing the CodePal checks they don't produce any results (but should)

[BaerbelW]

Check Name

All the checks in CodePal

Actual Behavior

None of the checks produce a result when they are used to check a complex program

Expected Behavior

The checked program is old and contains many statements which don't follow the Clean-ABAP rules so many statements should have been flagged (for example there are many MOVE-statements in the code). When I run other checks, like DEFAULT, they do produce results as expected.

I already tried to see what's happening via SAT and debugging while running a check, but hat no luck spotting a potential issue. There's more information and some screenshots in the comments to Olga's helpful blog post:
https://blogs.sap.com/2022/05/05/how-to-enable-clean-code-checks-for-abap/comment-page-1/#comment-621870

Please let me know what kind of additional information you need and/or what I can try myself come Monday (I won't have access to the sandbox system over the weekend).

Thanks much and cheers
Bärbel

[bjoern-jueliger-sap]

The checked program is old

The CodePal has a configuration setting - both in the individual checks (behind the attributes button for the check in SCI when you view the check variant) and in the profile management tool - that sets a "creation data threshold": Objects/includes older than this setting will not be checked. Did you perhaps leave this at its default and your checked program is older than that default?

[BaerbelW]

@D072189
Hi Björn,
thanks for your comment! Yes, I just figured this out and was going to post this update:

I think I narrowed down the reason why the checks don't find anything: it seems to have to do with when a program was created. Here is why I think that: the program I tried with was created several years ago and even though the checks seem to get accessed when I run it in debugging, no issues are found. I just now copied the whole program into a new local copy and ran the checks again and now it produces the expected long list of issues.

This "cut-off" date seems to be definied in the select-options for each check:

So, it can be manipulated there for each check which is just a wee bit tedious. Some questions:

• is a cut-off date like this really needed?
• instead of querying the creation date of a program, how about making it dependent on last update?
• If the date is needed, is there a central point somewhere in the check-code where it can be updated?

I'll have to check the profile management tool which I hadn't noticed thus far to see how involved tweaking this will be - right now, I'd really just need an option to do a "bulk" or central change for the creation date as we'll have many old programs where the checks should be run for.

Cheers
Bärbel

[bjoern-jueliger-sap]

The idea behind this feature is that since CleanABAP is mostly about style, most users will not want to invest into aligning code they already have ("legacy code") with this style. I.e. the "default use case" imagined here is a group of developers deciding that they will follow CleanABAP from a certain point onwards, but will not invest into retroactively applying this style to old code.

This explains the necessity of the cut-off date: They need to exclude objects that already existed before they commited to the new style from their check runs for the check runs to be useful.

However, they will still occasionally need to fix bugs in the legacy code: If the update of an include triggered the checks, then fixing a bug would suddenly commit you to also fixing everything else with the include. Besides, the functionality of "only report these findings when the location where they're at is touched" is effectively already covered by the ATC baseline - no need to configure this for checks individually.

Lastly, I think the profile management tool should effectively allow you to apply this creation date threshold to all checks more easily than going through them one by one, but I'm not really familiar with the tool.

[BaerbelW]

I may have a slightly different use case for these checks in mind: namely to get developers to take a close(r) look at what current recommendations for coding are and to become more familiar with CleanABAP. We have a central system running the ATC-checks with an active baseline. These checks get executed on task-release. As the code-pal checks (currently) are not RFC-enabled I cannot add them to these central checks for the time being. The check-variant for CodePal would exist locally in the relevant dev-systems and developers would be "strongly encouraged" to run their code against them manually. For that to work out, the initial creation date of a piece of code wouldn't be really relevant. I'd include CodePal within the larger context to raise awareness about CleanABAP.

[BaerbelW]

Thanks for changing the label from bug to discussion! I was wondering how to do that as this is obviously not a bug.

I went to Y_CODE_PAL_PROFILE and it looks as if the cut-off date needs to be updated for all 59 checks individually as these are values in table YTAB_CHECKS and field OBJECTS_CREATED_ON. A central default value would be nice to have which could then bset once and could then be tweaked for each check individually if needed.

[bjoern-jueliger-sap]

No worries - I had to create the label in order to tag this with it, so you couldn't have done this ;)

Hm, in that case I don't think there's currently any UI that allows to change this default centrally, but if you need a short-term solution you could just modify the corresponding line in class Y_CHECK_BASE: in your local copy of the checks - just change the date literal there to an older date.

[BaerbelW]

Thanks for pointing me at the code where I updated the date. Unfortunately, this doesn't seem to have the desired effect as the date shown in the checks as well as the entries in table YTAB_CHECKS sticks with 20190101 even though I changed the date to 20010101 in the code. I haven't yet found a place where the date gets persisted in the table even though I see that the assignement gets accessed when I set a breakpoint there.

[bjoern-jueliger-sap]

Did you try to create a new check variant? If you just tried it with a check variant created before the change then the old date is persisted as a setting in the check variant and changing the default achieves nothing. Either create a new check variant or deselect the checks in your current checkvariant, save, exit the SCI transaction, display the variant again and add the checks again.

[BaerbelW]

Thanks! Deselecting the checks briefly, saving the variant with another set of checks and then checking the CodePal checks again did the trick:

And the old program now also produces the expected long list of issues.

Now I can tweak the checks to eliminate the ones which don't make sense in our situation because we for example have a rule to add inline comments for change annotations, thus run afoul of this check a lot!

Thanks for your help figuring things out! Is there a means and good place to add the gist of this discussion to the CodePal documentation?

[bjoern-jueliger-sap]

I don't think we should recommend that users change the code manually to achieve this because it could result in merge conflicts when they try to update their installations to newer CodePal versions, so I'm reluctant to add this to the documentation.

What we really should do is offer a simple way to change this default across all checks. (That this isn't currently possible is really a disadvantage of the design choice to make each little check its own ATC check instead of having a single check that has a lot of message codes but that you only need to configure once.)

[BaerbelW]

Yes, I agree that a "hack" is not a good option outside of a sandbox system for prototyping like I'm doing right now. A fairly simple solution could be to get rid of the hard-coded default "20190101" and get the value from a table instead with an option to override it as needed (and currently already possible) in the individual checks.

What could however be added to the documentation is a mention that this restriction exists - just to avoid others going on wild-goose-chases like I did.