fix: nested generic arg splitting, JS/Ruby test false positives

- Replace naive comma split in extractReturnTypeName with bracket-balanced
  extractFirstGenericArg so nested types like Future<Result<User, Error>>
  unwrap correctly instead of producing malformed "Result<User"
- Add CompletableFuture to WRAPPER_GENERICS for Java async unwrapping
- Split js-jsdoc-return-type fixture models.js into user.js/repo.js and
  add negative assertions to prove disambiguation (not just file match)
- Split ruby-constant-factory-call fixture into separate service files
  and add negative assertions against AdminService resolution
This commit is contained in:
Gergo Magyar 2026-03-15 13:33:34 +00:00
parent 9fbbe2da8d
commit 25483b28cd
11 changed files with 107 additions and 45 deletions

View file

@ -401,14 +401,31 @@ const PRIMITIVE_TYPES = new Set([
* Returns undefined for complex types or primitives.
*/
const WRAPPER_GENERICS = new Set([
'Promise', 'Observable', 'Future', 'Task', 'ValueTask', // async wrappers
'Option', 'Some', 'Optional', 'Maybe', // nullable wrappers
'Result', 'Either', // result wrappers
'Promise', 'Observable', 'Future', 'CompletableFuture', 'Task', 'ValueTask', // async wrappers
'Option', 'Some', 'Optional', 'Maybe', // nullable wrappers
'Result', 'Either', // result wrappers
// Containers (List, Array, Vec, Set, etc.) are intentionally excluded —
// methods are called on the container, not the element type.
// Non-wrapper generics return the base type (e.g., List) via the else branch.
]);
/**
* Extracts the first type argument from a comma-separated generic argument string,
* respecting nested angle brackets. For example:
* "Result<User, Error>" → "Result<User, Error>" (no top-level comma)
* "User, Error" → "User"
* "Map<K, V>, string" → "Map<K, V>"
*/
function extractFirstGenericArg(args: string): string {
let depth = 0;
for (let i = 0; i < args.length; i++) {
if (args[i] === '<') depth++;
else if (args[i] === '>') depth--;
else if (args[i] === ',' && depth === 0) return args.slice(0, i).trim();
}
return args.trim();
}
export const extractReturnTypeName = (raw: string): string | undefined => {
let text = raw.trim();
if (!text) return undefined;
@ -433,8 +450,9 @@ export const extractReturnTypeName = (raw: string): string | undefined => {
if (genericMatch) {
const [, base, args] = genericMatch;
if (WRAPPER_GENERICS.has(base)) {
// Take the first type argument (e.g., Result<User, Error> → User)
const firstArg = args.split(',')[0].trim();
// Take the first type argument, using bracket-balanced splitting so that
// nested generics like Result<User, Error> are not split at the inner comma.
const firstArg = extractFirstGenericArg(args);
return extractReturnTypeName(firstArg);
}
// Non-wrapper generic: return the base type (e.g., Map<K,V> → Map)

View file

@ -1,4 +1,5 @@
const { User, Repo } = require('./models');
const { User } = require('./user');
const { Repo } = require('./repo');
/**
* @returns {User}

View file

@ -1,21 +0,0 @@
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,11 @@
class Repo {
constructor(path) {
this.path = path;
}
save() {
return true;
}
}
module.exports = { Repo };

View file

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

View file

@ -0,0 +1,9 @@
class AdminService
def process
true
end
def validate
true
end
end

View file

@ -1,4 +1,5 @@
require_relative 'user_service'
require_relative 'admin_service'
# @return [UserService]
def build_service

View file

@ -7,13 +7,3 @@ class UserService
true
end
end
class AdminService
def process
true
end
def validate
true
end
end

View file

@ -671,6 +671,12 @@ describe('Ruby constant factory call resolution (SERVICE = build_service())', ()
c.target === 'process' && c.targetFilePath.includes('user_service.rb'),
);
expect(processCall).toBeDefined();
const wrongCall = calls.find(c =>
c.target === 'process' &&
c.sourceFilePath?.includes('app.rb') &&
c.targetFilePath.includes('admin_service.rb'),
);
expect(wrongCall).toBeUndefined();
});
it('resolves SERVICE.validate() to UserService#validate via constant factory call', () => {
@ -679,5 +685,11 @@ describe('Ruby constant factory call resolution (SERVICE = build_service())', ()
c.target === 'validate' && c.targetFilePath.includes('user_service.rb'),
);
expect(validateCall).toBeDefined();
const wrongCall = calls.find(c =>
c.target === 'validate' &&
c.sourceFilePath?.includes('app.rb') &&
c.targetFilePath.includes('admin_service.rb'),
);
expect(wrongCall).toBeUndefined();
});
});

View file

@ -915,33 +915,53 @@ describe('JavaScript return type inference via JSDoc @returns annotation', () =>
it('resolves user.save() to User#save via JSDoc @returns {User}', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('models.js'),
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('user.js'),
);
expect(saveCall).toBeDefined();
// Negative: must NOT resolve to Repo#save
const wrongCall = calls.find(c =>
c.target === 'save' && c.source === 'processUser' && c.targetFilePath.includes('repo.js'),
);
expect(wrongCall).toBeUndefined();
});
it('resolves repo.save() to Repo#save via JSDoc @returns {Repo}', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'processRepo' && c.targetFilePath.includes('models.js'),
c.target === 'save' && c.source === 'processRepo' && c.targetFilePath.includes('repo.js'),
);
expect(saveCall).toBeDefined();
// Negative: must NOT resolve to User#save
const wrongCall = calls.find(c =>
c.target === 'save' && c.source === 'processRepo' && c.targetFilePath.includes('user.js'),
);
expect(wrongCall).toBeUndefined();
});
it('resolves user.save() via JSDoc @param {User} in handleUser()', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'handleUser' && c.targetFilePath.includes('models.js'),
c.target === 'save' && c.source === 'handleUser' && c.targetFilePath.includes('user.js'),
);
expect(saveCall).toBeDefined();
// Negative: must NOT resolve to Repo#save
const wrongCall = calls.find(c =>
c.target === 'save' && c.source === 'handleUser' && c.targetFilePath.includes('repo.js'),
);
expect(wrongCall).toBeUndefined();
});
it('resolves repo.save() via JSDoc @param {Repo} in handleRepo()', () => {
const calls = getRelationships(result, 'CALLS');
const saveCall = calls.find(c =>
c.target === 'save' && c.source === 'handleRepo' && c.targetFilePath.includes('models.js'),
c.target === 'save' && c.source === 'handleRepo' && c.targetFilePath.includes('repo.js'),
);
expect(saveCall).toBeDefined();
// Negative: must NOT resolve to User#save
const wrongCall = calls.find(c =>
c.target === 'save' && c.source === 'handleRepo' && c.targetFilePath.includes('user.js'),
);
expect(wrongCall).toBeUndefined();
});
});

View file

@ -625,10 +625,20 @@ describe('extractReturnTypeName', () => {
expect(extractReturnTypeName('com.example.models.User')).toBe('User');
});
it('returns undefined for nested generic with comma (known limitation)', () => {
// Promise<Map<string, User>> — comma splits inside nested generics produce incorrect results.
// Fixing requires balanced-bracket-aware splitting, which is out of scope.
expect(extractReturnTypeName('Promise<Map<string, User>>')).toBeUndefined();
it('unwraps wrapper over non-wrapper generic: Promise<Map<string, User>> → Map', () => {
// Promise is a wrapper — unwrap it to get Map<string, User>.
// Map is not a wrapper, so return its base type: Map.
expect(extractReturnTypeName('Promise<Map<string, User>>')).toBe('Map');
});
it('unwraps doubly-nested wrapper: Future<Result<User, Error>> → User', () => {
// Future → unwrap → Result<User, Error>; Result → unwrap first arg → User
expect(extractReturnTypeName('Future<Result<User, Error>>')).toBe('User');
});
it('unwraps CompletableFuture<Optional<User>> → User', () => {
// CompletableFuture → unwrap → Optional<User>; Optional → unwrap → User
expect(extractReturnTypeName('CompletableFuture<Optional<User>>')).toBe('User');
});
it('returns undefined for lowercase non-class types', () => {