integrated-application-development / sonar-delphi

Delphi language plugin for SonarQube
GNU Lesser General Public License v3.0
104 stars 15 forks source link

False positives in `VariableInitialization` around `SetLength` intrinsic #250

Closed zedxxx closed 4 months ago

zedxxx commented 4 months ago

Delphi IDE version: Delphi 12.1

DelphiLint version: 1.1.0

SonarDelphi version: 1.6.0

1

unit Unit1;

interface

uses
  GR32,
  GR32_Polygons,
  GR32_VectorUtils;

procedure DrawThickLine(ABitmap: TBitmap32; const AStart, AEnd: TFloatPoint; const AColor: TColor32;
  const AWidth: Integer); inline;

procedure DrawThickPolyLine(ABitmap: TBitmap32; const APoints: TArrayOfFloatPoint; const AColor: TColor32;
  const AWidth: Integer);

implementation

procedure DrawThickLine(ABitmap: TBitmap32; const AStart, AEnd: TFloatPoint; const AColor: TColor32;
  const AWidth: Integer);
var
  VLine: TArrayOfFloatPoint;
begin
  SetLength(VLine, 2);

  VLine[0] := AStart;
  VLine[1] := AEnd;

  DrawThickPolyLine(ABitmap, VLine, AColor, AWidth);
end;

procedure DrawThickPolyLine(ABitmap: TBitmap32; const APoints: TArrayOfFloatPoint;
  const AColor: TColor32; const AWidth: Integer);
var
  VPoly: TArrayOfFloatPoint;
begin
  VPoly := BuildPolyLine(APoints, AWidth);

  if not ABitmap.MeasuringMode then begin
    PolygonFS(ABitmap, VPoly, AColor);
  end;

  ABitmap.Changed(MakeRect(PolygonBounds(VPoly), rrOutside));
end;

end.
Cirras commented 4 months ago

Hi @zedxxx, thanks for reporting these!

The first issue (uninitialized VLine) might be a mistake in the way we're modeling the SetLength intrinsic. Definitely a problem - I'll have a look at fixing it.

The second issue (unused VPoly) looks like a name resolution issue on the PolygonFS invocation - I'm interested in chasing it, but can you make a new GitHub issue to track that distinct bug separately?

zedxxx commented 4 months ago

The second issue (unused VPoly) looks like a name resolution issue on the PolygonFS invocation - I'm interested in chasing it, but can you make a new GitHub issue to track that distinct bug separately ?

Sure: https://github.com/integrated-application-development/sonar-delphi/issues/251

Cirras commented 4 months ago

Cheers, I'll chase up these issues. Thanks for reporting them. 👍

Cirras commented 4 months ago

Hi @zedxxx,

Looks like SetLength is modeled correctly - and I can't seem to reproduce this VariableInitialization FP. (See image below)

Here's what I did:

The analyzer requires source code to perform type & invocation resolution, and things can go quite haywire if dependency source code isn't in the search path (or debugger source path).

One potential thing - do you only have the Graphics32 DCUs on your search path, and not the source files?

zedxxx commented 4 months ago

In my case, the path to the sources was added through the Delphi settings, not the project settings. Also, the IDE environment variable is used to specify the path (if it matters).

1

Cirras commented 4 months ago

Ah, there's the problem.

The analyzer doesn't currently read the library path. Adding support hasn't been seriously looked at, but it would certainly improve the accuracy of our import resolution (as evidenced by your case).

In your case, for an accurate analysis you'll have to add your dependencies to the search path or debug source path.

We could look into adding support for the library path, and you're welcome to open a feature request.