mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-16 23:43:12 +00:00
fix(extractors): address PR review findings on UE macro preprocessor
Resolves three blocking issues raised by automated review: 1. Prettier format: ran prettier --write on call-processor.ts, heritage-processor.ts, import-processor.ts (the three sites where the cache-miss reparse hook insertion landed unformatted). 2. Byte-length contract narrowed: language-provider.ts docblock now states the contract precisely (UTF-16 .length + newline-position preservation, not UTF-8 byte length). Notes that startIndex byte offsets only match the original file when the elided range is pure ASCII -- which is the practical UE case (reflection macros and module-export tokens are ASCII-only). 3. Tree-sitter extraction tests added: new end-to-end tests parse the preprocessed source with tree-sitter-cpp and assert the captured class/struct name is the real UClass identifier (UMyClass, FMyData), never the MODULE_API export macro. Also asserts source positions (startPosition.row) survive the transform. Plus one moderate fix: 4. _API stripping is now scoped to UE files only. The HAS_UE_HINT guard previously included [A-Z]_API tokens, which would fire on non-UE codebases that use REST_API / HTTP_API / MY_LIB_API as constants or enum values, silently erasing them. The guard now requires a strong UE marker (UCLASS|UFUNCTION|UPROPERTY|USTRUCT|UENUM|UINTERFACE|GENERATED_BODY|UE_DEPRECATED|DECLARE_*_DELEGATE) to be present before any stripping runs. Two new tests confirm REST_API and DECLARE_HANDLER style identifiers in non-UE files are left untouched. Plus one minor fix: 5. stripUeMacros signature now accepts (source, _filePath?) to match the LanguageProvider.preprocessSource hook contract exactly. The filePath argument is unused; UE detection is purely content-based. Verification: 34/34 preprocessor tests pass (was 27, +7 new for non-ASCII preservation, REST_API safety, tree-sitter extraction, struct extraction, source position preservation). Full unit suite 5349 pass, 0 regressions. Typecheck clean. Prettier --check clean on all 9 changed files.
This commit is contained in:
parent
0ee01bcaa3
commit
bf9112d4f8
6 changed files with 152 additions and 23 deletions
|
|
@ -769,8 +769,7 @@ export const processCalls = async (
|
|||
|
||||
let tree = astCache.get(file.path);
|
||||
if (!tree) {
|
||||
const parseContent =
|
||||
provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
const parseContent = provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
try {
|
||||
tree = parser.parse(parseContent, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(parseContent),
|
||||
|
|
@ -3282,8 +3281,7 @@ export const extractFetchCallsFromFiles = async (
|
|||
|
||||
let tree = astCache.get(file.path);
|
||||
if (!tree) {
|
||||
const parseContent =
|
||||
provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
const parseContent = provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
try {
|
||||
tree = parser.parse(parseContent, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(parseContent),
|
||||
|
|
|
|||
|
|
@ -18,7 +18,18 @@
|
|||
*
|
||||
* Pure function — no tree-sitter dependency, safe for worker threads.
|
||||
*/
|
||||
const HAS_UE_HINT = /\b(?:UCLASS|UFUNCTION|UPROPERTY|USTRUCT|UENUM|UINTERFACE|GENERATED_BODY|GENERATED_[A-Z_]+_BODY|UE_DEPRECATED|DECLARE_(?:DYNAMIC_)?(?:MULTICAST_)?DELEGATE|[A-Z][A-Z0-9_]*_API)/;
|
||||
/**
|
||||
* Strong UE markers — reflection macros that only Unreal Engine projects use.
|
||||
* Presence of one of these is sufficient evidence that the file is a UE source
|
||||
* and that `MODULENAME_API` tokens in it are intended as export macros.
|
||||
*
|
||||
* Importantly, `_API` tokens are NOT in this guard — `REST_API`, `HTTP_API`,
|
||||
* `MY_LIB_API` and similar identifiers appear in plenty of non-UE C++ codebases
|
||||
* as constants/enums/parameter names. We must not erase them just because the
|
||||
* file mentions an `_API` token.
|
||||
*/
|
||||
const HAS_UE_HINT =
|
||||
/\b(?:UCLASS|UFUNCTION|UPROPERTY|USTRUCT|UENUM|UINTERFACE|GENERATED_BODY|GENERATED_[A-Z_]+_BODY|UE_DEPRECATED|DECLARE_(?:DYNAMIC_)?(?:MULTICAST_)?DELEGATE)/;
|
||||
|
||||
const SIMPLE_MACROS_NO_ARGS: readonly string[] = [
|
||||
'GENERATED_BODY',
|
||||
|
|
@ -185,10 +196,15 @@ function skipWhitespace(source: string, idx: number): number {
|
|||
/**
|
||||
* Strip Unreal Engine reflection macros from C++ source, length-preserving.
|
||||
*
|
||||
* Returns the original string unchanged if no UE markers are detected, so
|
||||
* non-UE C++ files incur only a single regex test.
|
||||
* Returns the original string unchanged if no strong UE marker is detected,
|
||||
* so non-UE C++ files (including ones that contain `*_API`-suffixed
|
||||
* identifiers like `REST_API` or `HTTP_API`) incur only a single regex test.
|
||||
*
|
||||
* The `_filePath` parameter is part of the `LanguageProvider.preprocessSource`
|
||||
* contract but is unused — UE detection is purely content-based. Accepted and
|
||||
* ignored here so the function matches the hook signature exactly.
|
||||
*/
|
||||
export function stripUeMacros(source: string): string {
|
||||
export function stripUeMacros(source: string, _filePath?: string): string {
|
||||
if (!HAS_UE_HINT.test(source)) return source;
|
||||
|
||||
const chars: string[] = source.split('');
|
||||
|
|
|
|||
|
|
@ -222,8 +222,7 @@ export const processHeritage = async (
|
|||
// Per-language source preprocessor (length-preserving, e.g. UE macro
|
||||
// stripping for C++). MUST mirror parsing-processor on cache miss so
|
||||
// re-parses see the same input as the cached AST.
|
||||
const parseContent =
|
||||
provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
const parseContent = provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
try {
|
||||
tree = parser.parse(parseContent, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(parseContent),
|
||||
|
|
@ -418,8 +417,7 @@ export async function extractExtractedHeritageFromFiles(
|
|||
|
||||
let tree = astCache.get(file.path);
|
||||
if (!tree) {
|
||||
const parseContent =
|
||||
provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
const parseContent = provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
try {
|
||||
tree = parser.parse(parseContent, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(parseContent),
|
||||
|
|
|
|||
|
|
@ -305,8 +305,7 @@ export const processImports = async (
|
|||
let wasReparsed = false;
|
||||
|
||||
if (!tree) {
|
||||
const parseContent =
|
||||
provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
const parseContent = provider.preprocessSource?.(file.content, file.path) ?? file.content;
|
||||
try {
|
||||
tree = parser.parse(parseContent, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(parseContent),
|
||||
|
|
|
|||
|
|
@ -124,11 +124,22 @@ interface LanguageProviderConfig {
|
|||
* `UFUNCTION`, `MODULENAME_API`) in C++ headers that prevent the parser from
|
||||
* recognising class/function names correctly.
|
||||
*
|
||||
* **Length-preserving requirement:** the returned string MUST have the same
|
||||
* byte length as the input. Implementations should replace removed text with
|
||||
* spaces, preserving newlines, so tree-sitter's reported byte offsets and
|
||||
* line/column positions still match the original file. Violating this will
|
||||
* silently corrupt symbol locations in the graph.
|
||||
* **Length / position preservation:** the returned string MUST have the same
|
||||
* JavaScript `.length` as the input AND preserve every newline (`\n`/`\r`)
|
||||
* position byte-for-byte. Implementations replace elided characters with
|
||||
* ASCII spaces while leaving newlines untouched. With this contract:
|
||||
*
|
||||
* - tree-sitter's reported `startPosition.row`/`startPosition.column`
|
||||
* match the original file exactly (line/column come from newline counts)
|
||||
* - `startIndex`/`endIndex` byte offsets match the original file exactly
|
||||
* **when the elided range is pure ASCII** (UTF-16 `.length` equals UTF-8
|
||||
* byte length only for ASCII).
|
||||
*
|
||||
* Implementations targeting languages where elided ranges may contain
|
||||
* non-ASCII content must therefore preserve byte length, not just `.length`,
|
||||
* if downstream code uses `startIndex` to slice the original UTF-8 bytes.
|
||||
* The current C++ UE-macro preprocessor relies on the practical fact that
|
||||
* UE reflection macros and module-export tokens are ASCII-only.
|
||||
*
|
||||
* Must be a pure function — same input always yields the same output. Called
|
||||
* once per file, on every code path that re-parses (parsing-processor, import
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import Parser from 'tree-sitter';
|
||||
import CPP from 'tree-sitter-cpp';
|
||||
import { stripUeMacros } from '../../src/core/ingestion/cpp-ue-preprocessor.js';
|
||||
|
||||
describe('stripUeMacros — detection guard', () => {
|
||||
|
|
@ -57,8 +59,8 @@ describe('stripUeMacros — macro removal', () => {
|
|||
expect(out).toContain('class UBar {};');
|
||||
});
|
||||
|
||||
it('elides MODULE_API export macros (BRAWLUI_API style)', () => {
|
||||
const src = `class BRAWLUI_API UMyClass : public UObject {};`;
|
||||
it('elides MODULE_API export macros (BRAWLUI_API style) when paired with a UE marker', () => {
|
||||
const src = `UCLASS()\nclass BRAWLUI_API UMyClass : public UObject {};`;
|
||||
const out = stripUeMacros(src);
|
||||
expect(out).not.toContain('BRAWLUI_API');
|
||||
expect(out).toContain('class');
|
||||
|
|
@ -66,8 +68,8 @@ describe('stripUeMacros — macro removal', () => {
|
|||
expect(out).toContain('public UObject');
|
||||
});
|
||||
|
||||
it('elides multiple distinct *_API tokens in same file', () => {
|
||||
const src = `class CORE_API A {};\nclass UMG_API B : public A {};`;
|
||||
it('elides multiple distinct *_API tokens in same file when UE marker is present', () => {
|
||||
const src = `UCLASS()\nclass CORE_API A {};\nUCLASS()\nclass UMG_API B : public A {};`;
|
||||
const out = stripUeMacros(src);
|
||||
expect(out).not.toContain('CORE_API');
|
||||
expect(out).not.toContain('UMG_API');
|
||||
|
|
@ -116,6 +118,43 @@ describe('stripUeMacros — macro removal', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('stripUeMacros — non-UE files left alone', () => {
|
||||
it('does NOT strip standalone *_API identifiers when no UE marker is present', () => {
|
||||
const src = `enum class Status { REST_API = 1, HTTP_API = 2, MY_LIB_API = 3 };\nvoid handle(REST_API status);`;
|
||||
expect(stripUeMacros(src)).toBe(src);
|
||||
});
|
||||
|
||||
it('does NOT strip _API tokens in a file that only mentions DECLARE_DELEGATE-like macros from non-UE codebases', () => {
|
||||
const src = `// Custom delegate framework, not UE\n#define DECLARE_HANDLER(x) void x()\nDECLARE_HANDLER(MyHandler);\nint REST_API = 0;`;
|
||||
expect(stripUeMacros(src)).toBe(src);
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripUeMacros — non-ASCII content preservation', () => {
|
||||
it('leaves non-ASCII content outside elided ranges intact and at the same .length offset', () => {
|
||||
const src = `// Comment with non-ASCII: café résumé naïve\nUCLASS()\nclass UMyClass : public UObject\n{\n GENERATED_BODY()\n // Trailing: 日本語 αβγ\n};`;
|
||||
const out = stripUeMacros(src);
|
||||
expect(out.length).toBe(src.length);
|
||||
expect(out).toContain('café résumé naïve');
|
||||
expect(out).toContain('日本語 αβγ');
|
||||
expect(out).toContain('class UMyClass : public UObject');
|
||||
expect(out).not.toContain('UCLASS');
|
||||
expect(out).not.toContain('GENERATED_BODY');
|
||||
});
|
||||
|
||||
it('preserves newline positions when the file contains non-ASCII characters', () => {
|
||||
const src = `// café\nUPROPERTY()\nint32 Health;\n// résumé\nUFUNCTION()\nvoid Run();`;
|
||||
const out = stripUeMacros(src);
|
||||
const inputNewlines: number[] = [];
|
||||
const outputNewlines: number[] = [];
|
||||
for (let i = 0; i < src.length; i++) {
|
||||
if (src.charCodeAt(i) === 0x0a) inputNewlines.push(i);
|
||||
if (out.charCodeAt(i) === 0x0a) outputNewlines.push(i);
|
||||
}
|
||||
expect(outputNewlines).toEqual(inputNewlines);
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripUeMacros — false-positive guards', () => {
|
||||
it('does NOT strip identifiers that merely contain UCLASS as a substring', () => {
|
||||
const src = `void NotUCLASSAtAll(); int MyUCLASS = 0;`;
|
||||
|
|
@ -163,3 +202,71 @@ describe('stripUeMacros — class-name extraction sanity', () => {
|
|||
expect(tail.startsWith('UMyClass')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripUeMacros — tree-sitter extraction (end-to-end)', () => {
|
||||
/**
|
||||
* Walk the parse tree and return the captured class name(s). Works against
|
||||
* the actual tree-sitter-cpp grammar so this is a true integration check
|
||||
* for the core PR claim: the indexer now sees `UMyClass`, not `BRAWLUI_API`.
|
||||
*/
|
||||
function extractClassNames(source: string): string[] {
|
||||
const parser = new Parser();
|
||||
parser.setLanguage(CPP as unknown as Parser.Language);
|
||||
const tree = parser.parse(source);
|
||||
const names: string[] = [];
|
||||
const stack: Parser.SyntaxNode[] = [tree.rootNode];
|
||||
while (stack.length > 0) {
|
||||
const node = stack.pop()!;
|
||||
if (node.type === 'class_specifier' || node.type === 'struct_specifier') {
|
||||
const nameNode = node.childForFieldName('name');
|
||||
if (nameNode) names.push(nameNode.text);
|
||||
}
|
||||
for (let i = node.namedChildCount - 1; i >= 0; i--) {
|
||||
const child = node.namedChild(i);
|
||||
if (child) stack.push(child);
|
||||
}
|
||||
}
|
||||
return names;
|
||||
}
|
||||
|
||||
it('tree-sitter-cpp captures UMyClass as the class name (not BRAWLUI_API)', () => {
|
||||
const src = `UCLASS(BlueprintType)\nclass BRAWLUI_API UMyClass : public UObject\n{\n GENERATED_BODY()\n public:\n UFUNCTION()\n void Run();\n};`;
|
||||
const out = stripUeMacros(src);
|
||||
const names = extractClassNames(out);
|
||||
expect(names).toContain('UMyClass');
|
||||
expect(names).not.toContain('BRAWLUI_API');
|
||||
});
|
||||
|
||||
it('tree-sitter-cpp captures struct name correctly through USTRUCT + MODULE_API', () => {
|
||||
const src = `USTRUCT(BlueprintType)\nstruct ENGINE_API FMyData : public FBase\n{\n GENERATED_BODY()\n float Value;\n};`;
|
||||
const out = stripUeMacros(src);
|
||||
const names = extractClassNames(out);
|
||||
expect(names).toContain('FMyData');
|
||||
expect(names).not.toContain('ENGINE_API');
|
||||
});
|
||||
|
||||
it('tree-sitter-cpp source positions are preserved across stripping (line numbers match)', () => {
|
||||
const src = `UCLASS()\nclass BRAWLUI_API UMyClass : public UObject\n{\n GENERATED_BODY()\n public:\n void Run();\n};`;
|
||||
const out = stripUeMacros(src);
|
||||
const parser = new Parser();
|
||||
parser.setLanguage(CPP as unknown as Parser.Language);
|
||||
const tree = parser.parse(out);
|
||||
const stack: Parser.SyntaxNode[] = [tree.rootNode];
|
||||
let runLine: number | undefined;
|
||||
while (stack.length > 0) {
|
||||
const node = stack.pop()!;
|
||||
if (node.type === 'function_declarator') {
|
||||
const declarator = node.childForFieldName('declarator');
|
||||
if (declarator?.text === 'Run') {
|
||||
runLine = node.startPosition.row;
|
||||
break;
|
||||
}
|
||||
}
|
||||
for (let i = node.namedChildCount - 1; i >= 0; i--) {
|
||||
const child = node.namedChild(i);
|
||||
if (child) stack.push(child);
|
||||
}
|
||||
}
|
||||
expect(runLine).toBe(5); // 0-indexed: "void Run();" is on line 6 (index 5)
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue