New rule: `out` arguments should not be assigned before the call

[Chadehoc]

Prerequisites

• [X] This rule has not already been suggested.
• [X] This should be a new rule, not an improvement to an existing rule.
• [X] This rule would be generally useful, not specific to my code or setup.

Suggested rule title

OutArgumentShouldNotBeCalledInitialized

Rule description

The discussion at the end of #127 leads me to propose this as a separate rule; but I would prefer it as a part of the rule proposed in #132, because it also has to do with the possible confusion between var and out arguments (an integrated rule could be called OutAndVarArgumentConfusion).

The Delphi compiler does not actually make a difference between out and var. This is confusing, and can lead to accepting var as initializing the variable passed to it (rule #132 proposed against this). Another confusion is passing an initialized variable as out argument, which means that the argument should actually be var. This rule aims to detect that case.

Should be warned against:

var LVal: Integer := Default(TMyRecord); //useless
FillMyRecord({out argument} LVal); // an out argument should come out initialized

An exception is when the variable is actually used before the call. Then it is just a reuse of an existing variable.

var LVal: Integer := 12;
UseTheValue(LVal);
FillMyRecord({out argument} LVal); // this is a variable being reused, ok

Rationale

Track the confusion between out and var arguments. While there is no difference for the compiler, there is a huge difference in intent and readability.

[cirras]

Makes sense to me - thanks!

For the rule title, I'm thinking: RedundantOutArgumentInitialization

[sglienke]

The Delphi compiler does not actually make a difference between out and var.

This is not true - when the type of the out parameter is managed the compiler clears the variable before the call. Also, Delphi allows reading an out parameter inside the routine before it has been assigned and it does not ensure that such a parameter is set. This means for this proposed rule to be correct there also would need to be rules that ensure that the parameter is not read and that it is being set on all code paths. That means if the other rules are not also executed this rule could actually lead to defect if it would warn about an unnecessary initialization which is actually needed because either the value is being read or not being set in some code path.

[Chadehoc]

This means for this proposed rule to be correct there also would need to be rules that ensure that the parameter is not read and that it is being set on all code paths.

I think it's what #134 is for (ensure that the parameter is not read), and that missing out results are detected by rule "Routine results should be assigned a value".

It means #134 should be finished before this new rule is created.

That means if the other rules are not also executed this rule could actually lead to defect if it would warn about an unnecessary initialization which is actually needed because either the value is being read or not being set in some code path.

Interesting point. I don't know what the policy is for a rule that should imply another one.

I would argue that if an out parameter is used as if it was var, I would prefer to be warned, rather than have it ignored by fear of an unappropriate correction.

In this particular case, the only way the unappropriate correction would not be detected by sonar-delphi would be because an essential rule has been explicitely disabled (detecting uninitialized variables or unassigned results), provided #134 is done.