diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c4a7c0f..08981f81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 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. diff --git a/__tests__/pascal-for-in-var.test.ts b/__tests__/pascal-for-in-var.test.ts new file mode 100644 index 00000000..0d782bdb --- /dev/null +++ b/__tests__/pascal-for-in-var.test.ts @@ -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); +procedure WithDecl(const AList: TArray); + +implementation + +function TItem.DropIfEmpty: Boolean; +begin + Result := True; +end; + +procedure WithVar(const AList: TArray); +begin + for var lItem in AList do + lItem.DropIfEmpty; +end; + +procedure WithDecl(const AList: TArray); +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, check: (cg: CodeGraph) => void): Promise { + 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); + +implementation + +procedure THandleBox.Close; +begin +end; + +procedure TDecoy.Close; +begin +end; + +procedure UseTyped(const AHandles: TArray); +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>; const L: TArray); +begin + for var P: TPair> in D do + P.Value.Clear; + for var Q: System.Generics.Collections.TPair 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>', + ' 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>'), + 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); +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); + }); +}); diff --git a/src/extraction/languages/pascal-preparse.ts b/src/extraction/languages/pascal-preparse.ts index 8b7c5fb1..5d907f3e 100644 --- a/src/extraction/languages/pascal-preparse.ts +++ b/src/extraction/languages/pascal-preparse.ts @@ -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 = new Set(['const', 'var', 'out', 'constref']); const ROUTINE_KEYWORDS: ReadonlySet = 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 = 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>`) 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) {