#501 Formatter adds spurious spaces around generic brackets containing reserved-word type names

[Luka Havrlišan]

Summary

The code formatter inserts spaces around the < / > of a generic instantiation when a reserved word used as a type name (e.g. string) appears nested inside the generic argument list. Ordinary identifiers (e.g. Integer) are handled correctly.

Actual vs. expected

// actual
TSynMahTypeConverterRegistry.Register < TList<string> > (TSynMahListStringConverter);
Bar := TDictionary < string, TList<string> > .Create;

// expected
TSynMahTypeConverterRegistry.Register<TList<string>>(TSynMahListStringConverter);
Bar := TDictionary<string, TList<string>>.Create;

These already format correctly, which is the clue to the cause:

TSynMahTypeConverterRegistry.Register<TList<Integer>>(TSynMahListIntConverter); // ok: Integer is a plain identifier
Foo := TDictionary<string, Integer>.Create;                                     // ok: 'string' is the FIRST generic arg

Cause

In TCodeFormatterFormatter.DetectGenericStart (Source/Formatter/Engine/GX_CodeFormatterFormatter.pas), the heuristic that classifies < as a generic-open vs. a less-than operator scans the tokens between < and the matching >. string is a reserved word. The first-token check after < already accepts rtReserved, but the forward scan over the remaining tokens did not — any rtReserved token there caused the scan to bail out with Result := False, so the brackets were treated as operators and spaced.

Result: in Register<TList<string>> the inner < is detected (its first token is string), but the outer < fails when its scan reaches the nested string.

Fix

Make the forward scan accept rtReserved word tokens, symmetric with the existing first-token check (covers string, array, set, file). The wtWord guard keeps it safe — word-shaped operators such as and/or/xor are rtOper, not rtReserved, and still correctly reject a non-generic <...>.

       // also allowed:

-      rtNothing: begin
-          // but only an identifier (correct?)
+      rtNothing,
+      rtReserved: begin
+          // an identifier, or a reserved word used as a type name (e.g.
+          // 'string', 'array', 'set', 'file') nested in the generic args
           if Next.WordType <> wtWord then
             Exit; //==>
         end;

Test

Added UnitTestsFormatter/Testcases/input/testfile_GenericReservedWord.pas. It is a regular test file, so it is validated against all six formatter configurations (borland, default, delforex, headwork, special, twm) — all produce identical output.

Input:

unit testfile_GenericReservedWord;

interface

implementation

procedure Test;
begin
  TSynMahTypeConverterRegistry.Register < TList<string> > (TSynMahListStringConverter);
  TSynMahTypeConverterRegistry.Register<TList<Integer>>(TSynMahListIntConverter);
  Foo := TDictionary<string, Integer>.Create;
  Bar := TDictionary < string, TList<string> > .Create;
end;

end.

Expected output (identical for every configuration):

unit testfile_GenericReservedWord;

interface

implementation

procedure Test;
begin
  TSynMahTypeConverterRegistry.Register<TList<string>>(TSynMahListStringConverter);
  TSynMahTypeConverterRegistry.Register<TList<Integer>>(TSynMahListIntConverter);
  Foo := TDictionary<string, Integer>.Create;
  Bar := TDictionary<string, TList<string>>.Create;
end;

end.

Full formatter test suite passes after the fix (1503 tests, no regressions).

[Thomas Mueller]

fixed in revision #5488