mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
fix: C# and PHP return type inference — scanner fixes, method signature extraction, and cross-file resolution
Addresses code review findings on PR #284: C# scanner (csharp.ts): - Fix type node lookup: iterate children instead of childForFieldName('type') which returns undefined in tree-sitter-c-sharp - Fix initializer lookup: handle direct invocation_expression children (no equals_value_clause wrapper in tree-sitter-c-sharp) C# return type extraction (utils.ts): - Add 'returns' field check to extractMethodSignature — tree-sitter-c-sharp uses 'returns', not 'type', for method return types C# cross-file resolution (call-processor.ts + fixture): - Add constructor binding verification to sequential processCalls path (was only in the worker processCallsFromExtracted path) - Add ReturnType.csproj to csharp-return-type fixture - Update fixture namespaces to use ReturnType.Models/ReturnType.Services prefix (matches real C# project conventions) PHP scanner (php.ts): - Extend scanConstructorBinding to handle member_call_expression ($this->getUser() patterns), not just function_call_expression Shared (shared.ts): - Add member_access_expression to extractSimpleTypeName qualified-names block (C# method calls like svc.GetUser()) Tests: - Add Repo.cs/Repo.php disambiguation fixtures (two Save methods) - Strengthen C# and PHP return type tests with hard disambiguation assertions - Add C# scanner unit tests and return type extraction test
This commit is contained in:
parent
6f416410f9
commit
292cb8e642
14 changed files with 168 additions and 36 deletions
|
|
@ -110,6 +110,30 @@ export const processCalls = async (
|
|||
const typeEnv = lang ? buildTypeEnv(tree, lang, ctx.symbols) : null;
|
||||
const callRouter = callRouters[language];
|
||||
|
||||
// Verify constructor bindings against SymbolTable for return type inference.
|
||||
// In the worker path, this happens in processCallsFromExtracted. In the
|
||||
// sequential path, we must do it here before resolving calls.
|
||||
const verifiedReceivers = new Map<string, string>();
|
||||
if (typeEnv && typeEnv.constructorBindings.length > 0) {
|
||||
for (const { scope, varName, calleeName } of typeEnv.constructorBindings) {
|
||||
const tiered = ctx.resolve(calleeName, file.path);
|
||||
const isClass = tiered?.candidates.some(def => def.type === 'Class') ?? false;
|
||||
if (isClass) {
|
||||
verifiedReceivers.set(receiverKey(extractFuncNameFromScope(scope), varName), calleeName);
|
||||
} else {
|
||||
const callableDefs = tiered?.candidates.filter(d =>
|
||||
d.type === 'Function' || d.type === 'Method'
|
||||
);
|
||||
if (callableDefs && callableDefs.length === 1 && callableDefs[0].returnType) {
|
||||
const typeName = extractReturnTypeName(callableDefs[0].returnType);
|
||||
if (typeName) {
|
||||
verifiedReceivers.set(receiverKey(extractFuncNameFromScope(scope), varName), typeName);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
ctx.enableCache(file.path);
|
||||
|
||||
matches.forEach(match => {
|
||||
|
|
@ -184,7 +208,14 @@ export const processCalls = async (
|
|||
const callNode = captureMap['call'];
|
||||
const callForm = inferCallForm(callNode, nameNode);
|
||||
const receiverName = callForm === 'member' ? extractReceiverName(nameNode) : undefined;
|
||||
const receiverTypeName = receiverName && typeEnv ? typeEnv.lookup(receiverName, callNode) : undefined;
|
||||
let receiverTypeName = receiverName && typeEnv ? typeEnv.lookup(receiverName, callNode) : undefined;
|
||||
// Fall back to verified constructor bindings for return type inference
|
||||
if (!receiverTypeName && receiverName && verifiedReceivers.size > 0) {
|
||||
const enclosingFunc = findEnclosingFunction(callNode, file.path, ctx);
|
||||
const funcName = enclosingFunc ? extractFuncNameFromSourceId(enclosingFunc) : '';
|
||||
receiverTypeName = verifiedReceivers.get(receiverKey(funcName, receiverName))
|
||||
?? verifiedReceivers.get(receiverKey('', receiverName));
|
||||
}
|
||||
|
||||
const resolved = resolveCallTarget({
|
||||
calledName,
|
||||
|
|
|
|||
|
|
@ -106,26 +106,29 @@ const extractParameter: ParameterExtractor = (node: SyntaxNode, env: Map<string,
|
|||
/** C#: var x = SomeFactory(...) → bind x to SomeFactory (constructor-like call) */
|
||||
const scanConstructorBinding: ConstructorBindingScanner = (node) => {
|
||||
if (node.type !== 'variable_declaration') return undefined;
|
||||
const typeNode = node.childForFieldName('type');
|
||||
// Only handle implicit_type (var) — explicit types handled by extractDeclaration
|
||||
if (!typeNode || typeNode.type !== 'implicit_type') return undefined;
|
||||
// Find first variable_declarator child
|
||||
// Find type and declarator children by iterating (C# grammar doesn't expose 'type' as a named field)
|
||||
let typeNode: SyntaxNode | null = null;
|
||||
let declarator: SyntaxNode | null = null;
|
||||
for (let i = 0; i < node.namedChildCount; i++) {
|
||||
const child = node.namedChild(i);
|
||||
if (child?.type === 'variable_declarator') { declarator = child; break; }
|
||||
if (!child) continue;
|
||||
if (child.type === 'variable_declarator') { if (!declarator) declarator = child; }
|
||||
else if (!typeNode) { typeNode = child; }
|
||||
}
|
||||
// Only handle implicit_type (var) — explicit types handled by extractDeclaration
|
||||
if (!typeNode || typeNode.type !== 'implicit_type') return undefined;
|
||||
if (!declarator) return undefined;
|
||||
const nameNode = declarator.childForFieldName('name') ?? declarator.firstNamedChild;
|
||||
if (!nameNode || nameNode.type !== 'identifier') return undefined;
|
||||
// Find equals_value_clause
|
||||
let eqClause: SyntaxNode | null = null;
|
||||
// Find the initializer value: either inside equals_value_clause or as a direct child
|
||||
// (tree-sitter-c-sharp puts invocation_expression directly inside variable_declarator)
|
||||
let value: SyntaxNode | null = null;
|
||||
for (let i = 0; i < declarator.namedChildCount; i++) {
|
||||
const child = declarator.namedChild(i);
|
||||
if (child?.type === 'equals_value_clause') { eqClause = child; break; }
|
||||
if (!child) continue;
|
||||
if (child.type === 'equals_value_clause') { value = child.firstNamedChild; break; }
|
||||
if (child.type === 'invocation_expression' || child.type === 'object_creation_expression') { value = child; break; }
|
||||
}
|
||||
if (!eqClause) return undefined;
|
||||
const value = eqClause.firstNamedChild;
|
||||
if (!value) return undefined;
|
||||
// Skip object_creation_expression (new User()) — handled by extractInitializer
|
||||
if (value.type === 'object_creation_expression') return undefined;
|
||||
|
|
|
|||
|
|
@ -110,7 +110,7 @@ const extractParameter: ParameterExtractor = (node: SyntaxNode, env: Map<string,
|
|||
if (varName && typeName) env.set(varName, typeName);
|
||||
};
|
||||
|
||||
/** PHP: $x = SomeFactory() — bind variable to factory/static call return type */
|
||||
/** PHP: $x = SomeFactory() or $x = $this->getUser() — bind variable to call return type */
|
||||
const scanConstructorBinding: ConstructorBindingScanner = (node) => {
|
||||
if (node.type !== 'assignment_expression') return undefined;
|
||||
const left = node.childForFieldName('left');
|
||||
|
|
@ -119,13 +119,18 @@ const scanConstructorBinding: ConstructorBindingScanner = (node) => {
|
|||
if (left.type !== 'variable_name') return undefined;
|
||||
// Skip object_creation_expression (new User()) — handled by extractInitializer
|
||||
if (right.type === 'object_creation_expression') return undefined;
|
||||
if (right.type !== 'function_call_expression') return undefined;
|
||||
const calleeName = extractCalleeName(right);
|
||||
if (!calleeName) return undefined;
|
||||
// Keep the $ sigil — PHP env keys are stored with $ (e.g. "$user") by extractVarName
|
||||
const varName = left.text;
|
||||
if (!varName) return undefined;
|
||||
return { varName, calleeName };
|
||||
// Handle both standalone function calls and method calls ($this->getUser())
|
||||
if (right.type === 'function_call_expression') {
|
||||
const calleeName = extractCalleeName(right);
|
||||
if (!calleeName) return undefined;
|
||||
return { varName: left.text, calleeName };
|
||||
}
|
||||
if (right.type === 'member_call_expression') {
|
||||
const methodName = right.childForFieldName('name');
|
||||
if (!methodName) return undefined;
|
||||
return { varName: left.text, calleeName: methodName.text };
|
||||
}
|
||||
return undefined;
|
||||
};
|
||||
|
||||
export const typeConfig: LanguageTypeConfig = {
|
||||
|
|
|
|||
|
|
@ -17,7 +17,8 @@ export const extractSimpleTypeName = (typeNode: SyntaxNode): string | undefined
|
|||
if (typeNode.type === 'scoped_identifier' || typeNode.type === 'qualified_identifier'
|
||||
|| typeNode.type === 'scoped_type_identifier' || typeNode.type === 'qualified_name'
|
||||
|| typeNode.type === 'qualified_type'
|
||||
|| typeNode.type === 'member_expression' || typeNode.type === 'attribute'
|
||||
|| typeNode.type === 'member_expression' || typeNode.type === 'member_access_expression'
|
||||
|| typeNode.type === 'attribute'
|
||||
|| typeNode.type === 'scope_resolution') {
|
||||
const last = typeNode.lastNamedChild;
|
||||
if (last && (last.type === 'type_identifier' || last.type === 'identifier'
|
||||
|
|
|
|||
|
|
@ -640,6 +640,14 @@ export const extractMethodSignature = (node: SyntaxNode | null | undefined): Met
|
|||
}
|
||||
}
|
||||
|
||||
// C#: 'returns' field on method_declaration
|
||||
if (!returnType) {
|
||||
const csReturn = node.childForFieldName?.('returns');
|
||||
if (csReturn && csReturn.text !== 'void') {
|
||||
returnType = csReturn.text;
|
||||
}
|
||||
}
|
||||
|
||||
// TS/Rust/Python/C#/Kotlin: type_annotation or return_type child
|
||||
if (!returnType) {
|
||||
for (const child of node.children) {
|
||||
|
|
|
|||
9
gitnexus/test/fixtures/lang-resolution/csharp-return-type/Models/Repo.cs
vendored
Normal file
9
gitnexus/test/fixtures/lang-resolution/csharp-return-type/Models/Repo.cs
vendored
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
namespace ReturnType.Models;
|
||||
|
||||
public class Repo
|
||||
{
|
||||
public bool Save()
|
||||
{
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
namespace Models;
|
||||
namespace ReturnType.Models;
|
||||
|
||||
public class User
|
||||
{
|
||||
|
|
|
|||
5
gitnexus/test/fixtures/lang-resolution/csharp-return-type/ReturnType.csproj
vendored
Normal file
5
gitnexus/test/fixtures/lang-resolution/csharp-return-type/ReturnType.csproj
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
<PropertyGroup>
|
||||
<TargetFramework>net8.0</TargetFramework>
|
||||
</PropertyGroup>
|
||||
</Project>
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
using Models;
|
||||
using ReturnType.Models;
|
||||
|
||||
namespace Services;
|
||||
namespace ReturnType.Services;
|
||||
|
||||
public class App
|
||||
{
|
||||
|
|
|
|||
9
gitnexus/test/fixtures/lang-resolution/php-return-type/app/Models/Repo.php
vendored
Normal file
9
gitnexus/test/fixtures/lang-resolution/php-return-type/app/Models/Repo.php
vendored
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
<?php
|
||||
|
||||
namespace App\Models;
|
||||
|
||||
class Repo {
|
||||
public function save(): bool {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -575,25 +575,34 @@ describe('C# return type inference via var + invocation', () => {
|
|||
);
|
||||
}, 60000);
|
||||
|
||||
it('detects User and UserService classes', () => {
|
||||
it('detects User, UserService, and Repo classes', () => {
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('User');
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('UserService');
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('Repo');
|
||||
});
|
||||
|
||||
it('detects Save and GetUser methods', () => {
|
||||
it('detects Save on both User and Repo, plus GetUser', () => {
|
||||
const methods = getNodesByLabel(result, 'Method');
|
||||
expect(methods).toContain('Save');
|
||||
expect(methods).toContain('GetUser');
|
||||
// Repo.Save is also detected, proving the disambiguation test is meaningful
|
||||
expect(methods.filter((m: string) => m === 'Save').length).toBe(2);
|
||||
});
|
||||
|
||||
it('resolves user.Save() to User#Save via return type of GetUser(): User', () => {
|
||||
// C#'s CONSTRUCTOR_BINDING_SCANNER binds `var user = svc.GetUser()` to the
|
||||
// return type of GetUser (User), so the subsequent user.Save() call resolves
|
||||
// to User#Save rather than an unresolved target.
|
||||
it('resolves user.Save() to User#Save (not Repo#Save) via return type of GetUser(): User', () => {
|
||||
// scanConstructorBinding binds `var user = svc.GetUser()` → calleeName "GetUser".
|
||||
// processCallsFromExtracted verifies GetUser's returnType is "User" via
|
||||
// PackageMap resolution of `using ReturnType.Models;`, then receiver filtering
|
||||
// resolves user.Save() to User#Save (not Repo#Save).
|
||||
const calls = getRelationships(result, 'CALLS');
|
||||
const saveCall = calls.find(c =>
|
||||
c.target === 'Save' && c.source === 'Run' && c.targetFilePath.includes('Models'),
|
||||
c.target === 'Save' && c.source === 'Run' && c.targetFilePath.includes('User.cs'),
|
||||
);
|
||||
expect(saveCall).toBeDefined();
|
||||
// Must NOT resolve to Repo.Save — that would mean disambiguation failed
|
||||
const repoSave = calls.find(c =>
|
||||
c.target === 'Save' && c.source === 'Run' && c.targetFilePath.includes('Repo.cs'),
|
||||
);
|
||||
expect(repoSave).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -698,11 +698,12 @@ describe('PHP typed class property resolution', () => {
|
|||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Return type inference: $user = $this->getUser("alice"); $user->save()
|
||||
// PHP's CONSTRUCTOR_BINDING_SCANNER captures assignment_expression with
|
||||
// function_call_expression values, enabling return type inference.
|
||||
// PHP's scanConstructorBinding captures assignment_expression with both
|
||||
// function_call_expression and member_call_expression values, enabling
|
||||
// return type inference for method calls on objects.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('PHP return type inference via function call', () => {
|
||||
describe('PHP return type inference via member call', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
|
|
@ -712,22 +713,30 @@ describe('PHP return type inference via function call', () => {
|
|||
);
|
||||
}, 60000);
|
||||
|
||||
it('detects User and UserService classes', () => {
|
||||
it('detects User, UserService, and Repo classes', () => {
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('User');
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('UserService');
|
||||
expect(getNodesByLabel(result, 'Class')).toContain('Repo');
|
||||
});
|
||||
|
||||
it('detects save and getUser methods', () => {
|
||||
it('detects save on both User and Repo, and getUser method', () => {
|
||||
const methods = getNodesByLabel(result, 'Method');
|
||||
expect(methods).toContain('save');
|
||||
expect(methods).toContain('getUser');
|
||||
// save exists on both User and Repo — disambiguation required
|
||||
expect(methods.filter((m: string) => m === 'save').length).toBe(2);
|
||||
});
|
||||
|
||||
it('resolves $user->save() to User#save via return type of getUser(): User', () => {
|
||||
it('resolves $user->save() to User#save (not Repo#save) via return type of getUser(): User', () => {
|
||||
const calls = getRelationships(result, 'CALLS');
|
||||
const saveCall = calls.find(c =>
|
||||
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('User.php'),
|
||||
);
|
||||
expect(saveCall).toBeDefined();
|
||||
// Must NOT resolve to Repo.save — that would mean disambiguation failed
|
||||
const repoSave = calls.find(c =>
|
||||
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('Repo.php'),
|
||||
);
|
||||
expect(repoSave).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -230,6 +230,20 @@ describe('extractMethodSignature', () => {
|
|||
const sig = extractMethodSignature(methodNode);
|
||||
expect(sig.parameterCount).toBe(0);
|
||||
});
|
||||
|
||||
it('extracts return type from C# method', () => {
|
||||
parser.setLanguage(CSharp);
|
||||
const code = `class Svc {
|
||||
public User GetUser(string name) { return null; }
|
||||
}`;
|
||||
const tree = parser.parse(code);
|
||||
const classNode = tree.rootNode.child(0)!;
|
||||
const classBody = classNode.childForFieldName('body')!;
|
||||
const methodNode = classBody.namedChild(0)!;
|
||||
|
||||
const sig = extractMethodSignature(methodNode);
|
||||
expect(sig.returnType).toBe('User');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Go', () => {
|
||||
|
|
|
|||
|
|
@ -1794,5 +1794,34 @@ svc = App::Models::Service.new
|
|||
expect(flatGet(env, 'user')).toBe('User');
|
||||
expect(constructorBindings.find(b => b.varName === 'user')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('returns constructor binding for C# var user = svc.GetUser()', () => {
|
||||
const tree = parse(`
|
||||
class App {
|
||||
void Run() {
|
||||
var svc = new UserService();
|
||||
var user = svc.GetUser("alice");
|
||||
}
|
||||
}
|
||||
`, CSharp);
|
||||
const { constructorBindings } = buildTypeEnv(tree, 'csharp');
|
||||
const binding = constructorBindings.find(b => b.varName === 'user');
|
||||
expect(binding).toBeDefined();
|
||||
expect(binding!.calleeName).toBe('GetUser');
|
||||
});
|
||||
|
||||
it('returns constructor binding for C# var user = GetUser() (standalone call)', () => {
|
||||
const tree = parse(`
|
||||
class App {
|
||||
void Run() {
|
||||
var user = GetUser("alice");
|
||||
}
|
||||
}
|
||||
`, CSharp);
|
||||
const { constructorBindings } = buildTypeEnv(tree, 'csharp');
|
||||
const binding = constructorBindings.find(b => b.varName === 'user');
|
||||
expect(binding).toBeDefined();
|
||||
expect(binding!.calleeName).toBe('GetUser');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue