integrated-application-development / sonar-delphi

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

Apply directives to the closest context instead of to subsequent code #63

Open fourls opened 1 year ago

fourls commented 1 year ago

Prerequisites

Engine area

Delphi language support

Improvement description

Currently, SonarDelphi "applies" a compiler directive when it is encountered. In Delphi, however, a compiler directive is applied at the beginning of whatever context it is in (which is not well defined). For example, the following prints "1":

S := '01';
WriteLn(S[1 {$ZEROBASEDSTRINGS ON}]);

Rationale

SonarDelphi is handling Delphi's semantics incorrectly.

Cirras commented 1 year ago

This seems pretty tricky, but we should absolutely do it for correctness.

Paging @zaneduffield: any more information on what constitutes a "context"?

zaneduffield commented 1 year ago

The following program demonstrates the basics of how it works. It runs to completion with no uncaught exceptions.

program CompilerSwitchContextExamples;

{$APPTYPE CONSOLE}

{$R *.res}

uses
  System.SysUtils;

function Echo(I: Integer): Integer;
begin
  Result := I;
end;

function MaxInt: Integer;
begin
  Result := System.MaxInt;
end;

procedure AssertRaises(proc: TProc; Expected: ExceptClass);
begin
  try
    proc;
    Assert(false, 'Expected proc to raise an exception');
  except
    on E: Exception do
      Assert(E.ClassType = Expected, 'Expected class ' + Expected.ClassName + ', Actual class ' + E.ClassName);
  end;
end;

var
  S: String;
begin
  S := '01';

  {$ZEROBASEDSTRINGS OFF}
  // active from the start of the []
  Assert(S[1 {$ZEROBASEDSTRINGS ON}] = '1');

  {$ZEROBASEDSTRINGS OFF}
  // even when wrapped in parentheses, it's active in the []
  Assert(S[1 + (0 {$ZEROBASEDSTRINGS ON})] = '1');

  {$ZEROBASEDSTRINGS OFF}
  var A := [0];
  // even when wrapped in another [], it's active in the outer []
  Assert(S[1 + A[0 {$ZEROBASEDSTRINGS ON}]] = '1');

  {$ZEROBASEDSTRINGS OFF}
  // even when wrapped in a function call, it's active in the []
  Assert(S[Echo(1 {$ZEROBASEDSTRINGS ON})] = '1');

  {$ZEROBASEDSTRINGS OFF}
  // however, when put to the right of the [], it's not active inside []
  Assert(S[1] {$ZEROBASEDSTRINGS ON} = '0');
  {$ZEROBASEDSTRINGS OFF}
  Assert(S[1] = '0' {$ZEROBASEDSTRINGS ON});
  {$ZEROBASEDSTRINGS OFF}
  Assert(S[1] = '0') {$ZEROBASEDSTRINGS ON};
  {$ZEROBASEDSTRINGS OFF}
  Assert(S[1] = '0'); {$ZEROBASEDSTRINGS ON}

  {$ZEROBASEDSTRINGS OFF}
  // it doesn't become active across the assignment operator
  S[1] := {$ZEROBASEDSTRINGS ON} 'a';
  Assert(S = 'a1');

  {$Q-}
  AssertRaises(procedure begin Echo(MaxInt + 1 {$Q+}) end, EIntOverflow);

  {$Q-}
  AssertRaises(procedure begin var I := MaxInt + 1 {$Q+} end, EIntOverflow);

  {$Q-}
  AssertRaises(procedure begin var A := [MaxInt + 1 {$Q+}] end, EIntOverflow);

  {$Q-}
  // no exception, the directive doesn't 'jump' the comma
  var A2 := [MaxInt + 1, 0 {$Q+}];
end.