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.

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

Include from 2015 is being checked and throwing errors #556

Open
pedrohaccorsi opened this issue Mar 15, 2022 · 6 comments
Open

Include from 2015 is being checked and throwing errors #556

pedrohaccorsi opened this issue Mar 15, 2022 · 6 comments
Labels
bug Something isn't working correctly

Comments

@pedrohaccorsi
Copy link

Check Name

Comment Usage

Error replication

Internal system EHR

Actual Behavior

This check is being thrown for report RP_HRCCO_NECOH_WRI which was automatically generated as part of the DPP (Data Privacy and Protection) initiative; however, the check is pointing to a specific line where it includes a very old (2015) code, hcmdp_parch_sel_screen_mod. Check below:

Program RP_HRCCO_NECOH_WRI Include RP_HRCCO_NECOH_WRI line 38 Column. 0

38. AT SELECTION-SCREEN OUTPUT.
39. 
40.  INCLUDE hcmdp_parch_sel_screen_mod.

This is unexpected because the only includes subject to analysis should be those with creation date >= 2018.

Expected Behavior

Include hcmdp_parch_sel_screen_mod should be ingnored from the analysis and report RP_HRCCO_NECOH_WRI should throw no errors at all.

@pedrohaccorsi pedrohaccorsi added the bug Something isn't working correctly label Mar 15, 2022
@bjoern-jueliger-sap
Copy link
Member

This is not a bug: The object creation date configuration refers to the creation date of the checked main object, i.e. in this case the creation date of RP_HRCCO_NECOH_WRI, not the creation date of HCMDP_PARCH_SEL_SCREEN_MOD. The creation dates of individual includes are entirely irrelevant.

In general, using the creation dates for individual includes rather than the creation date of the main program would lead to very confusing situations: If a global class is created before the threshold, but a new method is then created after the threshold, then we would get findings only for this method. In contrast, new methods in a local class wouldn't trigger findings because local classes don't have individual includes for each method on a technical level. Hence, only the creation date of the main object is relevant in order to avoid confusing situation.

In general, newly created objects shouldn't reuse old code via INCLUDE statements anyway - this is a clear anti-pattern, modern ABAP has much better reuse techniques.

@pedrohaccorsi
Copy link
Author

The creation dates of individual includes are entirely irrelevant.

Recently our team started a refactor in report HARCIMPU (system EHR). This report is from November 24, 2009. Inside of it, we created the following includes:

INCLUDE harcimpu_f01.
INCLUDE harcimpu_f02.
INCLUDE harcimpu_f03.
INCLUDE harcimpu_f04.
INCLUDE harcimpu_f05.
INCLUDE harcimpu_f06.
INCLUDE harcimpu_f07.
INCLUDE harcimpu_f08.
INCLUDE harcimpu_f09.
INCLUDE harcimpu_f10.

As the code from these includes is a cut and paste from the content of HARCIMPU, with very very old code, they all started throwing multiple code pal issues (this is the reason why I created issue #555), forcing us to either correct or use pragma.

--

This example goes agaisn't your words, or perhaps I missuderstood it. Can you please check? If you need a real example, I can check with my team to remove some of the pragmas so that you can check the errors.

Thanks!

@bjoern-jueliger-sap
Copy link
Member

Ah, this is an issue with "includes" in ABAP being treated inconsistently.

The correct statement is that the creation date of the smallest TADIR object enclosing the line in which a finding is found is relevant. For a finding in e.g. a class, this will always be the entire class, even if the individual methods exist as includes on a technical level.

But includes that are included via INCLUDE directives often have their own TADIR entry, which is what is determined here, so for these includes, the creation date of the include will matter.

It is unclear to me whether this is intentional or not, but at least this merits discussion - reopening.

@pedrohaccorsi
Copy link
Author

so I don't really understand...

  1. new report throwing errors in old include
    RP_HRCCO_NECOH_WRI from 2022 throwing errors in hcmdp_parch_sel_screen_mod from 2015

  2. old report throwing errors in new include
    HARCIMPU from 2009 throwing errors in harcimpu_f01 from 2022.

In both scenarios, the includes are added with the literal statement include. so, as per your comment "The correct statement is that the creation date of the smallest TADIR object enclosing the line in which a finding is found is relevant", I assume that the example 1 is wrong, since the oldest object added via include is from 2015.

Can you help me clarify?

@bjoern-jueliger-sap
Copy link
Member

bjoern-jueliger-sap commented Mar 18, 2022

Yeah, the first one shouldn't happen according to my current understanding of the code - which means either there's a subtle bug somewhere or I haven't actually understood what's going on ;)

(but also: you really shouldn't be producing situations like the first one - multiply-used includes are a much bigger code smell than anything the code pal checks might find inside that include...)

@pedrohaccorsi
Copy link
Author

you really shouldn't be producing situations like the first one

Unfortunately we don't have much power over that entire report, as it is automatically generated by the system. Down sides of working with such legacy systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

2 participants