From 25483b28cd4b75f37e20e08e7ecdef45d5f40900 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Sun, 15 Mar 2026 13:33:34 +0000 Subject: [PATCH] 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> unwrap correctly instead of producing malformed "Result" → "Result" (no top-level comma) + * "User, Error" → "User" + * "Map, string" → "Map" + */ +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) - const firstArg = args.split(',')[0].trim(); + // Take the first type argument, using bracket-balanced splitting so that + // nested generics like Result are not split at the inner comma. + const firstArg = extractFirstGenericArg(args); return extractReturnTypeName(firstArg); } // Non-wrapper generic: return the base type (e.g., Map → Map) diff --git a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/app.js b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/app.js index c89180d61..fc60b05ca 100644 --- a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/app.js +++ b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/app.js @@ -1,4 +1,5 @@ -const { User, Repo } = require('./models'); +const { User } = require('./user'); +const { Repo } = require('./repo'); /** * @returns {User} diff --git a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/models.js b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/models.js deleted file mode 100644 index f590df38b..000000000 --- a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/models.js +++ /dev/null @@ -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 }; diff --git a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/repo.js b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/repo.js new file mode 100644 index 000000000..a85e9c39a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/repo.js @@ -0,0 +1,11 @@ +class Repo { + constructor(path) { + this.path = path; + } + + save() { + return true; + } +} + +module.exports = { Repo }; diff --git a/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/user.js b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/user.js new file mode 100644 index 000000000..7f19a622d --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/js-jsdoc-return-type/user.js @@ -0,0 +1,11 @@ +class User { + constructor(name) { + this.name = name; + } + + save() { + return true; + } +} + +module.exports = { User }; diff --git a/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/admin_service.rb b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/admin_service.rb new file mode 100644 index 000000000..257322f78 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/admin_service.rb @@ -0,0 +1,9 @@ +class AdminService + def process + true + end + + def validate + true + end +end diff --git a/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/app.rb b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/app.rb index 955fc08aa..c87e32d6f 100644 --- a/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/app.rb +++ b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/app.rb @@ -1,4 +1,5 @@ require_relative 'user_service' +require_relative 'admin_service' # @return [UserService] def build_service diff --git a/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/user_service.rb b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/user_service.rb index f053bf949..088018571 100644 --- a/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/user_service.rb +++ b/gitnexus/test/fixtures/lang-resolution/ruby-constant-factory-call/user_service.rb @@ -7,13 +7,3 @@ class UserService true end end - -class AdminService - def process - true - end - - def validate - true - end -end diff --git a/gitnexus/test/integration/resolvers/ruby.test.ts b/gitnexus/test/integration/resolvers/ruby.test.ts index 669de7961..9c3891b61 100644 --- a/gitnexus/test/integration/resolvers/ruby.test.ts +++ b/gitnexus/test/integration/resolvers/ruby.test.ts @@ -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(); }); }); diff --git a/gitnexus/test/integration/resolvers/typescript.test.ts b/gitnexus/test/integration/resolvers/typescript.test.ts index 1529ce05e..8c355f8b5 100644 --- a/gitnexus/test/integration/resolvers/typescript.test.ts +++ b/gitnexus/test/integration/resolvers/typescript.test.ts @@ -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(); }); }); diff --git a/gitnexus/test/unit/call-processor.test.ts b/gitnexus/test/unit/call-processor.test.ts index 1d130f090..85e9c2f3f 100644 --- a/gitnexus/test/unit/call-processor.test.ts +++ b/gitnexus/test/unit/call-processor.test.ts @@ -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> — comma splits inside nested generics produce incorrect results. - // Fixing requires balanced-bracket-aware splitting, which is out of scope. - expect(extractReturnTypeName('Promise>')).toBeUndefined(); + it('unwraps wrapper over non-wrapper generic: Promise> → Map', () => { + // Promise is a wrapper — unwrap it to get Map. + // Map is not a wrapper, so return its base type: Map. + expect(extractReturnTypeName('Promise>')).toBe('Map'); + }); + + it('unwraps doubly-nested wrapper: Future> → User', () => { + // Future → unwrap → Result; Result → unwrap first arg → User + expect(extractReturnTypeName('Future>')).toBe('User'); + }); + + it('unwraps CompletableFuture> → User', () => { + // CompletableFuture → unwrap → Optional; Optional → unwrap → User + expect(extractReturnTypeName('CompletableFuture>')).toBe('User'); }); it('returns undefined for lowercase non-class types', () => {