sonar-openedge icon indicating copy to clipboard operation
sonar-openedge copied to clipboard

State not reset

Open stefandrissen opened this issue 5 years ago • 3 comments

From a real issue, a version of this was in a session super procedure:

def var p_lerror as logical no-undo.

function getError returns logical ():
   return p_lerror.
end function.

function setError returns logical ():
   p_lerror = true.
end function.

The effect being that once an error was flagged, it was never reset and any subsequent error checker was just failing.

The pieces that contribute to the issue:

  • a procedure scoped variable
  • with only one assign with a constant value
  • a procedure (or class) who's instance is reused

Especially the third piece may make it tricky to create a rule for this. Maybe it is not needed and any piece of code should have code to reset all procedure scoped variables.

stefandrissen avatar Sep 23 '20 05:09 stefandrissen

What should Lint look for: that there is no statement p_lerror = false anywhere? How about character variables, should there be a check to to see if it is assigned "" anywhere? And that temp-table records are deleted after they are created? And even if there is a statement that resets a variable to its default value: is it called from anywhere? Wouldn't those kinds of checks raise a lot of false positives?

I really think this issue looks like a code design flaw, not something that Lint can try to catch.

jurjendijkstra avatar Sep 23 '20 06:09 jurjendijkstra

It is a code design flaw. In the example it is an obvious flaw, although even there it could be overlooked. But it is a flaw that could be detected.

As to the reset, yes, there should be a statement to reset the variable to it's initial value.

If it is called or not is beside the point, 'when a variable is assigned but never used' can be tricked by using it in an unused code block.

Temp-tables deserve their own issue since there are some extra complexities.

stefandrissen avatar Sep 23 '20 07:09 stefandrissen

The problem with this kind of rule is that real issues may be lost in the noise. A common example is for late instantiation of procedures, with an initialized variable, initially set to false, and set to true when the init procedure is executed. No good reason to turn it back to false, so it would be flagged by the rule. It may be worth a try, but I'm bit worried about the quality of the outcome.

gquerret avatar Sep 23 '20 09:09 gquerret