fix(pascal): calls inside a Delphi for var x in loop are indexed (#2781)

The Pascal grammar's for-in rule takes a plain variable, so a loop that
declares its variable inline (Delphi 10.3 and later), like
`for var lItem in AList do lItem.DropIfEmpty;` or
`for var lHandle: THandle in lHandles do`, became an ERROR node. A
paren-less call in its body lost its statement shape and reached no
graph, so the routine was missing from the callee's callers.

The Pascal preParse added for issue #2545 now also blanks the inline
`var` and the loop variable's `: Type`, offsets kept, so the loop parses
as the grammar's `for x in` form. The type runs to the `in` at nesting
depth 0, so generic and dotted types are whole; `for var i := 0 to`,
which the grammar already parses, and `for var` inside comments or
strings are left alone. The resolver still types the loop variable: it
reads a receiver's declaration from the source as written, so
`lHandle.Close` reaches THandle's Close, not a namesake.

Validation:
- New __tests__/pascal-for-in-var.test.ts (6 tests, the issue's unit
  included): 5 fail with main's pascal-preparse.ts, all pass with it.
- Every textual `for var x in` in the corpus is blanked except 9 that sit
  in comments or strings (DelphiMVCFramework 47 of 49, DWScript 89 of
  96), no other text is touched, and no file parses worse.
- A/B against main 929c65c8, both arms built from their own sources:
  SynEdit, HeidiSQL and mORMot2 are byte-identical. DelphiMVCFramework:
  errors in the 20 changed files drop from 57 to 10, edges are
  identical (its loop bodies hold parenthesized calls, which main's
  error recovery already kept), +1 unresolved ref for the VCL's own
  GetControls in a loop's iterator. DWScript (2,497 files, mostly
  DWScript-language test scripts): edges -41 +30, every one triaged.
  The two real-library files gain correct calls; one paren-less
  right-hand-side read main kept only through error recovery
  (`var v := FIter.CollectInteger`) is gone until issue #2514's
  paren-less calls land. The rest are script files: 7 new calls inside
  the loops, one call moved from the file node to its routine, one stray
  top-level statement no longer credited to the routine error recovery
  had stretched over it, 21 file-level guesses error recovery made on one
  script's stray top-level statements, and 8 garbage nodes per
  enumerations script whose multi-line names now hold the blanked text.
- Controls leveldb (C++, kernel) and SDWebImage (Objective-C, wasm) are
  byte-identical in dump-graph.
- Cost: no measurable change; index time medians of 3 interleaved
  rounds: HeidiSQL 5.50 to 5.49 s, mORMot2 13.96 to 13.94 s, DWScript
  13.98 to 13.94 s wall.

Fixes #2546

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-22 04:23:40 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 929c65c83a
commit 88ebefa135
3 changed files with 312 additions and 2 deletions
+1
View File
@@ -25,6 +25,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` no longer fails with "FOREIGN KEY constraint failed" in a project whose code lazy-loads a workspace package, like Supabase Studio's `dynamic(() => import('icons').then((mod) => mod.EdgeFunctions))` or vben's `() => import('@vben/layouts').then((m) => m.IFrameView)`. Before, every sync after an edit in such a project rolled back, so the index stopped following your changes until a full re-index. Thanks @bompus for the report and @inth3shadows. (#2565)
- In VB.NET, every member of a `Structure` is now indexed, including its fields, properties, methods, constructors and nested enums. Before, only the first member was, so the rest could not be found and their callers looked empty.
- In Delphi, a routine with an attribute on one of its parameters, like `const [ref] AValue: Integer` or DelphiMVCFramework's `const [MVCFromBody] Person: TPerson`, is now indexed with its calls, and so is every class member declared after it. Before, the routine was lost or its calls were credited to the file, the parameter showed up as a field of the class, and the members after it were missing. Thanks @stalexteam for the report. (#2545)
- In Delphi, a `for … in` loop that declares its variable inline, like `for var lItem in AList do` or `for var lHandle: THandle in lHandles do`, now has its calls indexed, and a call on a typed loop variable reaches that type's method. Before, the whole loop was skipped, so a routine calling a method only from such a loop was missing from that method's callers. Thanks @stalexteam for the report. (#2546)
- In VB.NET and C#, what a property's `Get` and `Set` code calls, creates and reads now belongs to that property, as do C#'s `get => …` accessors and `=> …` property bodies. Before, it was dropped, so a method used only from a property looked unused.
- In VB.NET, a field or property initializer like `= Compute()` or `As New List(Of Order)` now links what it calls and creates, and so do a `Custom Event`'s `AddHandler`, `RemoveHandler` and `RaiseEvent` blocks. Re-index VB.NET and C# projects after upgrading.
- In C#, a field or property initializer like `private readonly ILogger _log = LogManager.GetLogger(typeof(X));` or `public List<Foo> Items { get; } = new();` now links what it calls, creates and reads, including inside a lambda, and a target-typed `new()` there counts as creating the declared type. Before, initializers were skipped, so a method or class used only from one — like a converter created in a static list — looked unused, and a method passed as a value there was credited to the whole class instead of the field or property.
+237
View File
@@ -0,0 +1,237 @@
/**
* A Delphi `for … in` loop that declares its variable inline (#2546):
* `for var lItem in AList do`, `for var lHandle: THandle in lHandles do`
* (Delphi 10.3 and later).
*
* The Pascal grammar's for-in rule takes a plain variable, so the whole loop
* was an ERROR node and no call in its body reached the graph. The extractor's
* preParse now blanks the inline `var` and its type, offsets kept, so the loop
* parses the way the grammar knows it. The resolver still finds the type: it
* reads a receiver's declaration from the source as written.
*/
import { describe, it, expect, beforeAll } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { CodeGraph } from '../src';
import { extractFromSource } from '../src/extraction';
import { getParser, initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars';
import { preParsePascal } from '../src/extraction/languages/pascal-preparse';
beforeAll(async () => {
await initGrammars();
await loadGrammarsForLanguages(['pascal']);
});
// The issue's own unit.
const LOOP_PAS = `unit Loop;
interface
type
TItem = class
public
function DropIfEmpty: Boolean;
end;
procedure WithVar(const AList: TArray<TItem>);
procedure WithDecl(const AList: TArray<TItem>);
implementation
function TItem.DropIfEmpty: Boolean;
begin
Result := True;
end;
procedure WithVar(const AList: TArray<TItem>);
begin
for var lItem in AList do
lItem.DropIfEmpty;
end;
procedure WithDecl(const AList: TArray<TItem>);
var
lItem: TItem;
begin
for lItem in AList do
lItem.DropIfEmpty;
end;
end.
`;
function callsFrom(code: string, qualifiedName: string, file = 'Loop.pas'): string[] {
const result = extractFromSource(file, code);
const from = result.nodes.find((n) => n.qualifiedName === qualifiedName);
return result.unresolvedReferences
.filter((r) => r.referenceKind === 'calls' && r.fromNodeId === from?.id)
.map((r) => `${r.referenceName}:${r.line}`);
}
async function withIndex(files: Record<string, string>, check: (cg: CodeGraph) => void): Promise<void> {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-pascal-for-var-'));
try {
for (const [name, body] of Object.entries(files)) fs.writeFileSync(path.join(dir, name), body);
const cg = await CodeGraph.init(dir, { index: true });
try {
check(cg);
} finally {
cg.close();
}
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
}
function callerNames(cg: CodeGraph, qualifiedName: string): string[] {
const target = cg.getNodesByKind('method').find((n) => n.qualifiedName === qualifiedName);
if (!target) return [];
return cg
.getIncomingEdges(target.id)
.filter((e) => e.kind === 'calls')
.map((e) => cg.getNode(e.source)?.name ?? '?')
.sort();
}
describe('Delphi `for var … in` loops (#2546)', () => {
it('reads the call in the body of a `for var x in` loop, as for a declared loop variable', () => {
expect(callsFrom(LOOP_PAS, 'WithVar')).toEqual(['lItem.DropIfEmpty:24']);
expect(callsFrom(LOOP_PAS, 'WithDecl')).toEqual(['lItem.DropIfEmpty:32']);
});
it('links both routines to the method the issue names', async () => {
await withIndex({ 'Loop.pas': LOOP_PAS }, (cg) => {
expect(callerNames(cg, 'TItem::DropIfEmpty')).toEqual(['WithDecl', 'WithVar']);
});
});
it('resolves a typed loop variable on its declared type, not on a namesake', async () => {
const code = `unit Recv;
interface
type
THandleBox = class
public
procedure Close;
end;
TDecoy = class
public
procedure Close;
end;
procedure UseTyped(const AHandles: TArray<THandleBox>);
implementation
procedure THandleBox.Close;
begin
end;
procedure TDecoy.Close;
begin
end;
procedure UseTyped(const AHandles: TArray<THandleBox>);
begin
for var lHandle: THandleBox in AHandles do
lHandle.Close;
end;
end.
`;
await withIndex({ 'Recv.pas': code }, (cg) => {
expect(callerNames(cg, 'THandleBox::Close')).toEqual(['UseTyped']);
expect(callerNames(cg, 'TDecoy::Close')).toEqual([]);
});
});
it('parses generic and dotted loop types, loops inside anonymous methods and one-line loops', () => {
const code = `unit Forms;
interface
implementation
procedure Run(const D: TDictionary<string, TList<Integer>>; const L: TArray<TItem>);
begin
for var P: TPair<string, TList<Integer>> in D do
P.Value.Clear;
for var Q: System.Generics.Collections.TPair<string, Integer> in Pairs do Q.Free;
TParallel.For(0, 9, procedure(I: Integer)
begin
for var X in L do X.Reset;
end);
if Assigned(L) then for var Y in L do Y.Done;
end;
end.
`;
const tree = getParser('pascal')!.parse(preParsePascal(code))!;
try {
expect(tree.rootNode.hasError).toBe(false);
} finally {
tree.delete();
}
// `P.Value.Clear` keeps the bare method name the extractor gives a chain
// on a variable; the anonymous method's call belongs to `Run`.
expect(callsFrom(code, 'Run', 'Forms.pas')).toEqual([
'Clear:10',
'Q.Free:11',
'TParallel.For:12',
'X.Reset:14',
'Assigned:16',
'Y.Done:16',
]);
});
it('blanks only the inline `var` and its type, keeping every offset', () => {
const blank = (line: string, ...parts: string[]) =>
parts.reduce((s, part) => s.replace(part, ' '.repeat(part.length)), line);
const lines = [
'for var lItem in AList do',
'for var lHandle: THandle in lHandles do',
'for var P: TPair<string, TList<Integer>>',
' in D do',
];
const out = preParsePascal(lines.join('\r\n'));
expect(out.split('\r\n')).toEqual([
blank(lines[0]!, 'var'),
blank(lines[1]!, 'var', ': THandle'),
blank(lines[2]!, 'var', ': TPair<string, TList<Integer>>'),
lines[3],
]);
});
it('leaves every other `for` alone', () => {
const code = `unit Keep;
interface
type
TStringHelper = record helper for string
function Upper: string;
end;
implementation
procedure Run(const L: TArray<Integer>);
var
N: Integer;
begin
for var I := 0 to 9 do Use(I);
for var J: Integer := 0 to 9 do Use(J);
for N in L do Use(N);
for N := High(L) downto 0 do Use(N);
// for var Old in L do Use(Old);
{ for var Older in L do }
WriteLn('for var Text in L do');
end;
end.
`;
expect(preParsePascal(code)).toBe(code);
});
});
+74 -2
View File
@@ -11,16 +11,29 @@
* at the `[` and every member declared after it in the class was lost.
* The extractor reads nothing from an attribute, and a signature still
* shows it: Pascal signatures read the source as written.
* - The inline variable of a `for … in` loop (Delphi 10.3): `for var lItem in
* AList do`, `for var lHandle: THandle in lHandles do`. The grammar's for-in
* rule takes a plain variable, so the whole loop became an error and no
* call in its body was read. Blanking `var` and the type leaves the loop
* the grammar knows. The resolver still finds the type: it reads a
* receiver's declaration from the source as written.
*/
const MODIFIERS: ReadonlySet<string> = new Set(['const', 'var', 'out', 'constref']);
const ROUTINE_KEYWORDS: ReadonlySet<string> = new Set(['procedure', 'function', 'constructor', 'destructor', 'operator']);
/** An attribute group longer than this is not one (and keeps the scan linear). */
const MAX_ATTRIBUTE_LENGTH = 2000;
/** A for-in loop variable's type longer than this is not one. */
const MAX_LOOP_TYPE_LENGTH = 500;
/** Words that end a for-in variable's type without an `in`: not the for-in form. */
const NOT_A_FOR_IN: ReadonlySet<string> = new Set(['do', 'to', 'downto', 'begin', 'end']);
/** Whether a file can hold a `for var … in` loop at all (the scan below decides). */
const FOR_VAR = /\bfor\s+var\b/i;
const CH_TAB = 9, CH_LF = 10, CH_CR = 13, CH_SPACE = 32, CH_AMP = 38, CH_QUOTE = 39;
const CH_LPAREN = 40, CH_RPAREN = 41, CH_STAR = 42, CH_DOT = 46, CH_SLASH = 47;
const CH_SEMI = 59, CH_LT = 60, CH_GT = 62, CH_LBRACKET = 91, CH_RBRACKET = 93, CH_LBRACE = 123;
const CH_COLON = 58, CH_SEMI = 59, CH_LT = 60, CH_EQ = 61, CH_GT = 62;
const CH_LBRACKET = 91, CH_RBRACKET = 93, CH_LBRACE = 123;
/** Identifier characters, `&` (Delphi's keyword escape) and any non-ASCII letter. */
function isIdentStart(c: number): boolean {
@@ -199,6 +212,62 @@ function scanParameterList(s: string, open: number, ranges: number[]): number {
return i;
}
/** The word starting at `i`, lowercased, or '' when no word starts there. */
function lowerWordAt(s: string, i: number): string {
return isIdentStart(s.charCodeAt(i)) ? s.slice(i, wordEnd(s, i + 1)).toLowerCase() : '';
}
/**
* After a `for` keyword ending at `forEnd`, recognize `var Name in` or
* `var Name: Type in` and push the `var` range and the `: Type` range onto
* `ranges`. The type runs to the `in` at nesting depth 0, so generic types
* (`TPair<string, TList<Integer>>`) and dotted names are whole. Any other
* shape (`for var i := 0 to …`, which the grammar parses, or no `in`) adds
* nothing. Returns the index to resume scanning from.
*/
function scanForInVariable(s: string, forEnd: number, ranges: number[]): number {
const varStart = skipTrivia(s, forEnd);
if (lowerWordAt(s, varStart) !== 'var') return forEnd;
const varEnd = varStart + 3;
const nameStart = skipTrivia(s, varEnd);
if (!isIdentStart(s.charCodeAt(nameStart))) return forEnd;
let i = skipTrivia(s, wordEnd(s, nameStart + 1));
let typeStart = -1;
if (s.charCodeAt(i) === CH_COLON && s.charCodeAt(i + 1) !== CH_EQ) {
typeStart = i;
const limit = Math.min(s.length, i + MAX_LOOP_TYPE_LENGTH);
let depth = 0;
i++;
for (;;) {
if (i >= limit) return forEnd;
const k = commentOrStringEnd(s, i);
if (k >= 0) {
i = k;
continue;
}
const c = s.charCodeAt(i);
if (c === CH_LT || c === CH_LPAREN || c === CH_LBRACKET) {
depth++;
} else if (c === CH_GT || c === CH_RPAREN || c === CH_RBRACKET) {
if (--depth < 0) return forEnd;
} else if (c === CH_SEMI || c === CH_EQ) {
return forEnd; // `for var i: Integer := 0 to …`, or no loop at all
} else if (isIdentStart(c)) {
const word = lowerWordAt(s, i);
if (depth === 0 && word === 'in') break;
if (NOT_A_FOR_IN.has(word)) return forEnd;
i = wordEnd(s, i + 1);
continue;
}
i++;
}
}
if (lowerWordAt(s, i) !== 'in') return forEnd;
ranges.push(varStart, varEnd);
if (typeStart >= 0) ranges.push(typeStart, i);
return i;
}
/** `s` with every `[from, to)` range in `ranges` blanked to spaces, newlines kept. */
function blankRanges(s: string, ranges: number[]): string {
if (ranges.length === 0) return s;
@@ -225,7 +294,7 @@ function followsClass(s: string, i: number): boolean {
* the source unchanged when there is nothing to blank.
*/
export function preParsePascal(source: string): string {
if (source.indexOf('[') < 0) return source;
if (source.indexOf('[') < 0 && !FOR_VAR.test(source)) return source;
const ranges: number[] = [];
const n = source.length;
let i = 0;
@@ -248,6 +317,9 @@ export function preParsePascal(source: string): string {
i = scanParameterList(source, open, ranges);
continue;
}
} else if (len === 3 && source.slice(i, e).toLowerCase() === 'for') {
i = scanForInVariable(source, e, ranges);
continue;
}
i = e;
} else if (c >= 48 && c <= 57) {