Report (warn) on STATIC variables that hold P.L.Objects

[PeterJudgeZA]

The procedure editor explicitly cleans up (aka DELETE OBJECT) any objects creates during any code it runs (that it does not know about). This is for purposes of reducing memory leaks - it is done for certain handles too.

OOABL static members are initialised once - when a static member is accessed - and theoretically left alone. A static property or variable can be defined as having a type of PLO (or a derivative), and if those members are populated while being run from the procedure editor, they will be deleted by the procedure editor. If they are set in a static constructor, this will only ever be run once per AVM session.

It would be good to have a rule that warned about static properties and variables that are PLO, explicitly for this case. Not all shops use the procedure editor, but there are plenty of people who do (whether it's to dev or just run code). Mitigation of this is to use a property, with a getter that checks whether the property has a value, and creates a new object if not. This does have a performance impact, so should not be more than a warning. Having such warnings can drastically reduce the time taken to debug issues.

This is not an issue when code is run from , say, PDSOE or command-line directly.

[gquerret]

So your concern is about the high initialization cost when running directly from the Procedure Editor, for example when initializing large lists ?

[cverbiest]

It's related to https://openedge.ideas.aha.io/ideas/OPENEDGE-I-714.
I stopped using procedure editor because of this and wrote my own basic editor that runs code without clean-up

[PeterJudgeZA]

So your concern is about the high initialization cost when running directly from the Procedure Editor, for example when initializing large lists ?

Static members are typically assigned via the static constructor, or can get set in a property getter implementation. Going from the simple assignment to the property getter carries a cost.

But having things "mysteriously" disappear thanks to the ProcEd adds cognitive cost to developers - they may not understand why the object that they created/assigned , and which was there the first run, now has vanished like beer at a PUG.

The warning is just to alert the devs to the fact that this might happen if they use the ProcEd for testing.

[jurjendijkstra]

Yeah beer at a PUG vanishes but then appears again and again and again.

There are 2 problems I see with this warning: first is that the warning would be raised even if the programmer did nothing wrong: they wrote a class with static members and that is perfectly fine. The rule warns about an issue that can not be fixed, it can only be ignored.

The second problem I see is that the warning does NOT pop up for programmers who are just using the procedure editor to do something that has a dependency on that class with static members. A dependency that this programmer may not even be aware of. So the sonarqube rule does not help their WTF status.

The problem is not de sourcecode, but an incomplete cleanup built into the procedure editor. Why not improve there? The procedure editor apparently has a list of handles it created (and will kill), so it knows it created a static, so I guess it could warn that static values in class .... may be wrong (and advice to restart your session).

[PeterJudgeZA]

There are 2 problems I see with this warning: first is that the warning would be raised even if the programmer did nothing wrong: they wrote a class with static members and that is perfectly fine. The rule warns about an issue that can not be fixed, it can only be ignored.

Yes, hence this should be a low-severity message. And a rule that not all people will run, for the reasons you describe.

The issue can be fixed - change from variable to property, or make it externally settable. This issue will primarily be seen in PRIVATE static variables, since only the class that defines them can set such values.

The problem is not de sourcecode, but an incomplete cleanup built into the procedure editor. Why not improve there? The procedure editor apparently has a list of handles it created (and will kill), so it knows it created a static, so I guess it could warn that static values in class .... may be wrong (and advice to restart your session).

The cleanup code for persistent procedures looks for an internal procedure named ADEPersistent and uses that to decide what to clean up or not. See https://github.com/progress/ADE/blob/913533dd12f3c1944b13e126e647b07d4bc99274/adecomm/_runcode.p#L694 .

The potential exists to do something similar for objects - Progress adds an interface that it checks for, before deleting any objects. But this cannot solve all problems (since you may not have control over certain sources); an alternative might be to provide a set of interfaces/types to not clean up. But either way is messy/complex.