integrated-application-development / integrated-application-development/sonar-delphi

Detect use of uninitialised memory in `finally` blocks

Open
#216 1 comment 1 reaction 0 assignees View on GitHub
enhancement rule
Dominant language
Java
Stars
159
Forks
31
Avg merge
5d 4h
Merged PRs (30d)
4

Description

### Prerequisites

- [X] This improvement has not already been suggested.
- [X] This improvement should not be implemented as a separate rule.

### Rule to improve

VariableInitialization

### Improvement description

An anti-pattern in Delphi is
```delphi
try
Foo := TFoo.Create;
{more code}
finally
Foo.Free;
end;
```
which is problematic because if `TFoo.Create` throws an exception then `Foo` will be uninitialised when the `finally` block is executed.

The correct version of that code is
```delphi
Foo := TFoo.Create;
try
{more code}
finally
Foo.Free;
end;
```
but another correct version would be
```delphi
Foo := nil;
try
Foo := TFoo.Create;
{more code}
finally
Foo.Free;
end;
```

In order to catch the issue in the first example, without creating a false positive in the last example, the rule would need to have a better understanding of control flow.

There is an endless supply of similar cases that would benefit from enhanced control flow analysis; this rule improvement isn't really about the try-finally anti-pattern specifically.

### Rationale

The current variable initialisation rule is quite rudimentary and fails to catch many common cases of uninitialised memory.
It's too permissive, because it has little-to-no understanding about code structure and control flow.

One of the real benefits of static analysis tools is that they can detect issues that are subtle, and only become apparent after excruciatingly evaluating all the logical paths.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.