fix: JSDoc async return type, PHP attribute walkers, and $this receiver disambiguation

Three fixes from fourth-pass code review on PR #284:

1. JSDoc `@returns {Promise<User>}` no longer stripped to `Promise` — extractReturnType
   now uses sanitizeReturnType (preserves generics) instead of normalizeJsDocType
   (which stripped them before extractReturnTypeName could unwrap WRAPPER_GENERICS).

2. PHP 8+ `#[Attribute]` and JS `@decorator` nodes no longer break doc-comment walkers.
   Both extractReturnType and collect*Params functions now skip attribute_list/decorator
   nodes instead of breaking on them as named siblings.

3. PHP `$this->method()` now provides receiverClassName for disambiguation.
   When two classes define the same method, the enclosing class narrows candidates
   via ownerId matching in call-processor, preventing false no-binding results.
This commit is contained in:
Gergo Magyar 2026-03-15 12:33:36 +00:00
parent 36ca3a3d54
commit fda19a915c
16 changed files with 323 additions and 52 deletions

View file

@ -1,7 +1,7 @@
<!-- gitnexus:start -->
# GitNexus — Code Intelligence
This project is indexed by GitNexus as **GitNexus** (1927 symbols, 4372 relationships, 143 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely.
This project is indexed by GitNexus as **GitNexus** (1996 symbols, 4656 relationships, 149 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely.
> If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first.
@ -97,25 +97,5 @@ To check whether embeddings exist, inspect `.gitnexus/meta.json` — the `stats.
| Rename / extract / split / refactor | `.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md` |
| Tools, resources, schema reference | `.claude/skills/gitnexus/gitnexus-guide/SKILL.md` |
| Index, status, clean, wiki CLI commands | `.claude/skills/gitnexus/gitnexus-cli/SKILL.md` |
| Work in the Ingestion area (184 symbols) | `.claude/skills/generated/ingestion/SKILL.md` |
| Work in the Cli area (71 symbols) | `.claude/skills/generated/cli/SKILL.md` |
| Work in the Workers area (58 symbols) | `.claude/skills/generated/workers/SKILL.md` |
| Work in the Wiki area (50 symbols) | `.claude/skills/generated/wiki/SKILL.md` |
| Work in the Kuzu area (45 symbols) | `.claude/skills/generated/kuzu/SKILL.md` |
| Work in the Components area (40 symbols) | `.claude/skills/generated/components/SKILL.md` |
| Work in the Embeddings area (32 symbols) | `.claude/skills/generated/embeddings/SKILL.md` |
| Work in the Local area (31 symbols) | `.claude/skills/generated/local/SKILL.md` |
| Work in the Mcp area (31 symbols) | `.claude/skills/generated/mcp/SKILL.md` |
| Work in the Services area (25 symbols) | `.claude/skills/generated/services/SKILL.md` |
| Work in the Resolvers area (15 symbols) | `.claude/skills/generated/resolvers/SKILL.md` |
| Work in the Eval area (15 symbols) | `.claude/skills/generated/eval/SKILL.md` |
| Work in the Llm area (14 symbols) | `.claude/skills/generated/llm/SKILL.md` |
| Work in the Hooks area (14 symbols) | `.claude/skills/generated/hooks/SKILL.md` |
| Work in the Bridge area (13 symbols) | `.claude/skills/generated/bridge/SKILL.md` |
| Work in the Environments area (11 symbols) | `.claude/skills/generated/environments/SKILL.md` |
| Work in the Analysis area (10 symbols) | `.claude/skills/generated/analysis/SKILL.md` |
| Work in the Server area (9 symbols) | `.claude/skills/generated/server/SKILL.md` |
| Work in the Type-extractors area (8 symbols) | `.claude/skills/generated/type-extractors/SKILL.md` |
| Work in the Unit area (7 symbols) | `.claude/skills/generated/unit/SKILL.md` |
<!-- gitnexus:end -->

View file

@ -1,7 +1,7 @@
<!-- gitnexus:start -->
# GitNexus — Code Intelligence
This project is indexed by GitNexus as **GitNexus** (1927 symbols, 4372 relationships, 143 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely.
This project is indexed by GitNexus as **GitNexus** (1996 symbols, 4656 relationships, 149 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely.
> If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first.
@ -97,25 +97,5 @@ To check whether embeddings exist, inspect `.gitnexus/meta.json` — the `stats.
| Rename / extract / split / refactor | `.claude/skills/gitnexus/gitnexus-refactoring/SKILL.md` |
| Tools, resources, schema reference | `.claude/skills/gitnexus/gitnexus-guide/SKILL.md` |
| Index, status, clean, wiki CLI commands | `.claude/skills/gitnexus/gitnexus-cli/SKILL.md` |
| Work in the Ingestion area (184 symbols) | `.claude/skills/generated/ingestion/SKILL.md` |
| Work in the Cli area (71 symbols) | `.claude/skills/generated/cli/SKILL.md` |
| Work in the Workers area (58 symbols) | `.claude/skills/generated/workers/SKILL.md` |
| Work in the Wiki area (50 symbols) | `.claude/skills/generated/wiki/SKILL.md` |
| Work in the Kuzu area (45 symbols) | `.claude/skills/generated/kuzu/SKILL.md` |
| Work in the Components area (40 symbols) | `.claude/skills/generated/components/SKILL.md` |
| Work in the Embeddings area (32 symbols) | `.claude/skills/generated/embeddings/SKILL.md` |
| Work in the Local area (31 symbols) | `.claude/skills/generated/local/SKILL.md` |
| Work in the Mcp area (31 symbols) | `.claude/skills/generated/mcp/SKILL.md` |
| Work in the Services area (25 symbols) | `.claude/skills/generated/services/SKILL.md` |
| Work in the Resolvers area (15 symbols) | `.claude/skills/generated/resolvers/SKILL.md` |
| Work in the Eval area (15 symbols) | `.claude/skills/generated/eval/SKILL.md` |
| Work in the Llm area (14 symbols) | `.claude/skills/generated/llm/SKILL.md` |
| Work in the Hooks area (14 symbols) | `.claude/skills/generated/hooks/SKILL.md` |
| Work in the Bridge area (13 symbols) | `.claude/skills/generated/bridge/SKILL.md` |
| Work in the Environments area (11 symbols) | `.claude/skills/generated/environments/SKILL.md` |
| Work in the Analysis area (10 symbols) | `.claude/skills/generated/analysis/SKILL.md` |
| Work in the Server area (9 symbols) | `.claude/skills/generated/server/SKILL.md` |
| Work in the Type-extractors area (8 symbols) | `.claude/skills/generated/type-extractors/SKILL.md` |
| Work in the Unit area (7 symbols) | `.claude/skills/generated/unit/SKILL.md` |
<!-- gitnexus:end -->

View file

@ -492,7 +492,7 @@ export const processCallsFromExtracted = async (
const fileReceiverTypes = new Map<string, Map<string, string>>();
if (constructorBindings) {
for (const { filePath, bindings } of constructorBindings) {
for (const { scope, varName, calleeName } of bindings) {
for (const { scope, varName, calleeName, receiverClassName } of bindings) {
const tiered = ctx.resolve(calleeName, filePath);
const isClass = tiered?.candidates.some(def => def.type === 'Class') ?? false;
if (isClass) {
@ -501,9 +501,19 @@ export const processCallsFromExtracted = async (
} else {
// Return type inference: if the callee is a function/method with a known
// return type, bind the variable to that return type.
const callableDefs = tiered?.candidates.filter(d =>
let callableDefs = tiered?.candidates.filter(d =>
d.type === 'Function' || d.type === 'Method'
);
// When receiver class is known (e.g. $this->method() in PHP), narrow
// candidates to methods owned by that class to avoid false disambiguation failures.
if (callableDefs && callableDefs.length > 1 && receiverClassName) {
const narrowed = callableDefs.filter(d => {
if (!d.ownerId) return false;
const owner = graph.getNode(d.ownerId);
return owner?.properties.name === receiverClassName;
});
if (narrowed.length > 0) callableDefs = narrowed;
}
if (callableDefs && callableDefs.length === 1 && callableDefs[0].returnType) {
const typeName = extractReturnTypeName(callableDefs[0].returnType);
if (typeName) {

View file

@ -374,6 +374,8 @@ export interface ConstructorBinding {
varName: string;
/** Name of the callee (potential class constructor) */
calleeName: string;
/** Enclosing class name when callee is a method on a known receiver (e.g. $this) */
receiverClassName?: string;
}

View file

@ -65,6 +65,10 @@ const normalizePhpType = (raw: string): string | undefined => {
return undefined;
};
/** Node types to skip when walking backwards to find doc-comments.
* PHP 8+ attributes (#[Route(...)]) appear as named siblings between PHPDoc and method. */
const SKIP_NODE_TYPES: ReadonlySet<string> = new Set(['attribute_list', 'attribute']);
/** Regex to extract PHPDoc @param annotations: `@param Type $name` */
const PHPDOC_PARAM_RE = /@param\s+(\S+)\s+\$(\w+)/g;
@ -78,7 +82,7 @@ const collectPhpDocParams = (methodNode: SyntaxNode): Map<string, string> => {
while (sibling) {
if (sibling.type === 'comment') {
commentTexts.unshift(sibling.text);
} else if (sibling.isNamed) {
} else if (sibling.isNamed && !SKIP_NODE_TYPES.has(sibling.type)) {
break;
}
sibling = sibling.previousSibling;
@ -195,7 +199,16 @@ const scanConstructorBinding: ConstructorBindingScanner = (node) => {
if (right.type === 'member_call_expression') {
const methodName = right.childForFieldName('name');
if (!methodName) return undefined;
return { varName: left.text, calleeName: methodName.text };
// When receiver is $this/self/static, qualify with enclosing class for disambiguation
const receiver = right.childForFieldName('object');
const receiverText = receiver?.text;
let receiverClassName: string | undefined;
if (receiverText === '$this' || receiverText === 'self' || receiverText === 'static') {
const cls = findEnclosingClass(node);
const clsName = cls?.childForFieldName('name');
if (clsName) receiverClassName = clsName.text;
}
return { varName: left.text, calleeName: methodName.text, receiverClassName };
}
return undefined;
};
@ -213,7 +226,7 @@ const extractReturnType: ReturnTypeExtractor = (node) => {
if (sibling.type === 'comment') {
const match = PHPDOC_RETURN_RE.exec(sibling.text);
if (match) return normalizePhpType(match[1]);
} else if (sibling.isNamed) break;
} else if (sibling.isNamed && !SKIP_NODE_TYPES.has(sibling.type)) break;
sibling = sibling.previousSibling;
}
return undefined;

View file

@ -14,8 +14,10 @@ export type ClassNameLookup = { has(name: string): boolean };
export type InitializerExtractor = (node: SyntaxNode, env: Map<string, string>, classNames: ClassNameLookup) => void;
/** Scans an AST node for untyped `var = callee()` patterns for return-type inference.
* Returns { varName, calleeName } if the node matches, undefined otherwise. */
export type ConstructorBindingScanner = (node: SyntaxNode) => { varName: string; calleeName: string } | undefined;
* Returns { varName, calleeName } if the node matches, undefined otherwise.
* `receiverClassName` — optional hint for method calls on known receivers
* (e.g. $this->getUser() in PHP provides the enclosing class name). */
export type ConstructorBindingScanner = (node: SyntaxNode) => { varName: string; calleeName: string; receiverClassName?: string } | undefined;
/** Extracts a return type string from a method/function definition node.
* Used for languages where return types are expressed in comments (e.g. YARD @return [Type])

View file

@ -45,7 +45,7 @@ const collectJsDocParams = (funcNode: SyntaxNode): Map<string, string> => {
while (sibling) {
if (sibling.type === 'comment') {
commentTexts.unshift(sibling.text);
} else if (sibling.isNamed) {
} else if (sibling.isNamed && sibling.type !== 'decorator') {
break;
}
sibling = sibling.previousSibling;
@ -156,6 +156,27 @@ const scanConstructorBinding: ConstructorBindingScanner = (node) => {
/** Regex to extract @returns or @return from JSDoc comments: `@returns {Type}` */
const JSDOC_RETURN_RE = /@returns?\s*\{([^}]+)\}/;
/**
* Minimal sanitization for JSDoc return types — preserves generic wrappers
* (e.g. `Promise<User>`) so that extractReturnTypeName in call-processor
* can apply WRAPPER_GENERICS unwrapping. Unlike normalizeJsDocType (which
* strips generics), this only strips JSDoc-specific syntax markers.
*/
const sanitizeReturnType = (raw: string): string | undefined => {
let type = raw.trim();
// Strip JSDoc nullable/non-nullable prefixes: ?User → User, !User → User
if (type.startsWith('?') || type.startsWith('!')) type = type.slice(1);
// Strip module: prefix — module:models.User → models.User
if (type.startsWith('module:')) type = type.slice(7);
// Take last segment of dotted path: models.User → User
const dotIdx = type.lastIndexOf('.');
if (dotIdx >= 0) type = type.slice(dotIdx + 1);
// Reject unions (ambiguous)
if (type.includes('|')) return undefined;
if (!type) return undefined;
return type;
};
/**
* Extract return type from JSDoc `@returns {Type}` or `@return {Type}` annotation
* preceding a function/method definition. Walks backwards through preceding siblings
@ -166,8 +187,8 @@ const extractReturnType: ReturnTypeExtractor = (node) => {
while (sibling) {
if (sibling.type === 'comment') {
const match = JSDOC_RETURN_RE.exec(sibling.text);
if (match) return normalizeJsDocType(match[1]);
} else if (sibling.isNamed) break;
if (match) return sanitizeReturnType(match[1]);
} else if (sibling.isNamed && sibling.type !== 'decorator') break;
sibling = sibling.previousSibling;
}
return undefined;

View file

@ -0,0 +1,25 @@
const { User, Repo } = require('./models');
/**
* @returns {Promise<User>}
*/
async function fetchUser(name) {
return new User(name);
}
/**
* @returns {Promise<Repo>}
*/
async function fetchRepo(path) {
return new Repo(path);
}
async function processUser() {
const user = await fetchUser('alice');
user.save();
}
async function processRepo() {
const repo = await fetchRepo('/data');
repo.save();
}

View file

@ -0,0 +1,21 @@
class User {
constructor(name) {
this.name = name;
}
save() {
return true;
}
}
class Repo {
constructor(path) {
this.path = path;
}
save() {
return true;
}
}
module.exports = { User, Repo };

View file

@ -0,0 +1,8 @@
<?php
class User {
public function save() { return true; }
}
class Repo {
public function save() { return true; }
}

View file

@ -0,0 +1,46 @@
<?php
require_once 'Models.php';
class UserService {
/**
* @return User
*/
#[Route('/user')]
public function getUser(string $name) {
return new User();
}
/**
* @return Repo
*/
#[Route('/repo')]
public function getRepo(string $path) {
return new Repo();
}
public function processUser() {
$user = $this->getUser("alice");
$user->save();
}
public function processRepo() {
$repo = $this->getRepo("/data");
$repo->save();
}
/**
* @param User $user the user to handle
*/
#[Validate]
public function handleUser($user) {
$user->save();
}
/**
* @param Repo $repo the repo to handle
*/
#[Validate]
public function handleRepo($repo) {
$repo->save();
}
}

View file

@ -0,0 +1,14 @@
<?php
require_once 'Models.php';
class AdminService {
/** @return Repo */
public function getUser(string $name) {
return new Repo();
}
public function processAdmin() {
$repo = $this->getUser("admin");
$repo->save();
}
}

View file

@ -0,0 +1,8 @@
<?php
class User {
public function save() { return true; }
}
class Repo {
public function save() { return true; }
}

View file

@ -0,0 +1,14 @@
<?php
require_once 'Models.php';
class UserService {
/** @return User */
public function getUser(string $name) {
return new User();
}
public function processUser() {
$user = $this->getUser("alice");
$user->save();
}
}

View file

@ -792,3 +792,93 @@ describe('PHP return type inference via PHPDoc @return annotation', () => {
expect(saveCall).toBeDefined();
});
});
// ---------------------------------------------------------------------------
// PHPDoc @return with PHP 8+ attributes (#[Route]) between doc-comment and method
// ---------------------------------------------------------------------------
describe('PHP PHPDoc @return with attributes between comment and method', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'php-phpdoc-attribute-return-type'),
() => {},
);
}, 60000);
it('detects User and Repo classes with save methods', () => {
expect(getNodesByLabel(result, 'Class')).toContain('User');
expect(getNodesByLabel(result, 'Class')).toContain('Repo');
});
it('resolves $user->save() to User#save despite #[Route] attribute between PHPDoc and method', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
it('resolves $repo->save() to Repo#save despite #[Route] attribute between PHPDoc and method', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processRepo' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
it('resolves $user->save() via PHPDoc @param despite #[Validate] attribute', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'handleUser' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
it('resolves $repo->save() via PHPDoc @param despite #[Validate] attribute', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'handleRepo' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
});
// ---------------------------------------------------------------------------
// $this->method() receiver disambiguation: two classes with same method name
// ---------------------------------------------------------------------------
describe('PHP $this->method() receiver disambiguation', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'php-this-receiver-disambiguation'),
() => {},
);
}, 60000);
it('detects UserService and AdminService classes, both with getUser methods', () => {
expect(getNodesByLabel(result, 'Class')).toContain('UserService');
expect(getNodesByLabel(result, 'Class')).toContain('AdminService');
const getUserMethods = getNodesByLabel(result, 'Method').filter(m => m === 'getUser');
expect(getUserMethods.length).toBe(2);
});
it('resolves $user->save() in UserService to User#save via $this->getUser() disambiguation', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
it('resolves $repo->save() in AdminService to Repo#save via $this->getUser() disambiguation', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processAdmin' && c.targetFilePath.includes('Models.php'),
);
expect(saveCall).toBeDefined();
});
});

View file

@ -945,3 +945,40 @@ describe('JavaScript return type inference via JSDoc @returns annotation', () =>
});
});
// ---------------------------------------------------------------------------
// JavaScript async return type inference via JSDoc @returns {Promise<User>}
// Verifies that wrapper generics (Promise) are unwrapped to the inner type.
// ---------------------------------------------------------------------------
describe('JavaScript async return type inference via JSDoc @returns {Promise<User>}', () => {
let result: PipelineResult;
beforeAll(async () => {
result = await runPipelineFromRepo(
path.join(FIXTURES, 'js-jsdoc-async-return-type'),
() => {},
);
}, 60000);
it('detects User and Repo classes with save methods', () => {
expect(getNodesByLabel(result, 'Class')).toContain('User');
expect(getNodesByLabel(result, 'Class')).toContain('Repo');
});
it('resolves user.save() to User#save via @returns {Promise<User>} unwrapping', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('models.js'),
);
expect(saveCall).toBeDefined();
});
it('resolves repo.save() to Repo#save via @returns {Promise<Repo>} unwrapping', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processRepo' && c.targetFilePath.includes('models.js'),
);
expect(saveCall).toBeDefined();
});
});