From 8be61d66815dda51a4db50efe326491d8d2bf944 Mon Sep 17 00:00:00 2001 From: liyipeng06 Date: Tue, 28 Apr 2026 16:16:19 +0800 Subject: [PATCH] fix(group): constrain weak thrift matching --- .../core/group/extractors/thrift-extractor.ts | 8 +++- .../group/extractors/thrift-patterns/java.ts | 47 ++++++++++++------- .../group/extractors/thrift-patterns/types.ts | 1 + gitnexus/src/core/group/matching.ts | 3 +- gitnexus/test/unit/group/matching.test.ts | 27 +++++++++++ .../test/unit/group/thrift-extractor.test.ts | 19 ++++++++ 6 files changed, 87 insertions(+), 18 deletions(-) diff --git a/gitnexus/src/core/group/extractors/thrift-extractor.ts b/gitnexus/src/core/group/extractors/thrift-extractor.ts index e853fafec..5ff9dd60c 100644 --- a/gitnexus/src/core/group/extractors/thrift-extractor.ts +++ b/gitnexus/src/core/group/extractors/thrift-extractor.ts @@ -330,7 +330,13 @@ export class ThriftExtractor implements ContractExtractor { ); } - if (detection.role !== 'consumer' || !detection.methodName) return null; + if ( + detection.role !== 'consumer' || + !detection.methodName || + !detection.usesGeneratedServiceMember + ) { + return null; + } return makeContract( thriftMethodContractId('', detection.serviceName, detection.methodName), detection.role, diff --git a/gitnexus/src/core/group/extractors/thrift-patterns/java.ts b/gitnexus/src/core/group/extractors/thrift-patterns/java.ts index fa8e98a7d..20bdf95ec 100644 --- a/gitnexus/src/core/group/extractors/thrift-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/thrift-patterns/java.ts @@ -13,12 +13,18 @@ const SERVICE_TYPE_RE = /^[A-Z][A-Za-z0-9]*(?:Service|Management)$/; interface VariableBinding { name: string; serviceName: string; + usesGeneratedServiceMember: boolean; scopeStart: number; scopeEnd: number; declarationEnd: number; scopeSize: number; } +interface ServiceTypeMatch { + serviceName: string; + usesGeneratedServiceMember: boolean; +} + const VARIABLE_PATTERNS = compilePatterns({ name: 'java-thrift-variables', language: Java, @@ -130,11 +136,18 @@ const PROVIDER_PATTERNS = compilePatterns({ ], } satisfies LanguagePatterns<{ scoped: boolean }>); -function serviceFromType(serviceText: string, memberText: string | undefined): string | null { +function serviceFromType( + serviceText: string, + memberText: string | undefined, +): ServiceTypeMatch | null { if (memberText !== undefined) { - return GENERATED_MEMBER_TYPES.has(memberText) ? serviceText : null; + return GENERATED_MEMBER_TYPES.has(memberText) + ? { serviceName: serviceText, usesGeneratedServiceMember: true } + : null; } - return SERVICE_TYPE_RE.test(serviceText) ? serviceText : null; + return SERVICE_TYPE_RE.test(serviceText) + ? { serviceName: serviceText, usesGeneratedServiceMember: false } + : null; } function methodNamesInClassBody(body: Parser.SyntaxNode): string[] { @@ -191,7 +204,7 @@ function resolveServiceForReceiver( bindings: VariableBinding[], receiver: string, callNode: Parser.SyntaxNode, -): string | null { +): VariableBinding | null { const callStart = callNode.startIndex; const candidates = bindings.filter( (binding) => @@ -204,7 +217,7 @@ function resolveServiceForReceiver( if (a.scopeSize !== b.scopeSize) return a.scopeSize - b.scopeSize; return b.declarationEnd - a.declarationEnd; }); - return candidates[0]?.serviceName ?? null; + return candidates[0] ?? null; } export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { @@ -219,13 +232,14 @@ export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { const varNode = match.captures.var; if (!serviceNode || !varNode) continue; const memberNode = match.meta.scoped ? match.captures.member : undefined; - const serviceName = serviceFromType(serviceNode.text, memberNode?.text); - if (!serviceName) continue; + const service = serviceFromType(serviceNode.text, memberNode?.text); + if (!service) continue; const scope = bindingScope(varNode); if (!scope) continue; bindings.push({ name: varNode.text, - serviceName, + serviceName: service.serviceName, + usesGeneratedServiceMember: service.usesGeneratedServiceMember, scopeStart: scope.scope.startIndex, scopeEnd: scope.scope.endIndex, declarationEnd: scope.declarationEnd, @@ -239,16 +253,17 @@ export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { const callNode = match.captures.receiver?.parent; if (!receiver || !methodName) continue; if (!callNode) continue; - const serviceName = resolveServiceForReceiver(bindings, receiver, callNode); - if (!serviceName) continue; + const binding = resolveServiceForReceiver(bindings, receiver, callNode); + if (!binding) continue; out.push({ role: 'consumer', - serviceName, + serviceName: binding.serviceName, methodName, symbolName: `${receiver}.${methodName}`, source: 'java_thrift_consumer', confidenceWithIdl: 0.75, confidenceWithoutIdl: 0.45, + usesGeneratedServiceMember: binding.usesGeneratedServiceMember, }); } @@ -258,18 +273,18 @@ export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { const bodyNode = match.captures.body; if (!serviceNode || !bodyNode) continue; const memberNode = match.meta.scoped ? match.captures.member : undefined; - const serviceName = serviceFromType(serviceNode.text, memberNode?.text); - if (!serviceName) continue; + const service = serviceFromType(serviceNode.text, memberNode?.text); + if (!service) continue; for (const methodName of methodNamesInClassBody(bodyNode)) { - const key = `${serviceName}.${methodName}`; + const key = `${service.serviceName}.${methodName}`; if (emittedProviders.has(key)) continue; emittedProviders.add(key); out.push({ role: 'provider', - serviceName, + serviceName: service.serviceName, methodName, - symbolName: `${serviceName}.${methodName}`, + symbolName: `${service.serviceName}.${methodName}`, source: 'java_thrift_provider', confidenceWithIdl: 0.8, confidenceWithoutIdl: 0, diff --git a/gitnexus/src/core/group/extractors/thrift-patterns/types.ts b/gitnexus/src/core/group/extractors/thrift-patterns/types.ts index 0976ce86d..e1550efca 100644 --- a/gitnexus/src/core/group/extractors/thrift-patterns/types.ts +++ b/gitnexus/src/core/group/extractors/thrift-patterns/types.ts @@ -10,6 +10,7 @@ export interface ThriftDetection { source: string; confidenceWithIdl: number; confidenceWithoutIdl: number; + usesGeneratedServiceMember?: boolean; } export interface ThriftLanguagePlugin { diff --git a/gitnexus/src/core/group/matching.ts b/gitnexus/src/core/group/matching.ts index 43081d8d7..495c9bd36 100644 --- a/gitnexus/src/core/group/matching.ts +++ b/gitnexus/src/core/group/matching.ts @@ -110,7 +110,8 @@ function findMatchingKeys(contractId: string, index: Map { expect(unmatched).toEqual([consumer, provider]); }); + it('does not match bare thrift service method when multiple package-qualified providers match', () => { + const consumer = makeThriftContract( + 'thrift::OrderService/PlaceOrder', + 'consumer', + 'frontend', + ); + const billingProvider = makeThriftContract( + 'thrift::billing.v1.OrderService/PlaceOrder', + 'provider', + 'billing', + ); + const salesProvider = makeThriftContract( + 'thrift::sales.v1.OrderService/PlaceOrder', + 'provider', + 'sales', + ); + + const providerIndex = buildProviderIndex([salesProvider, billingProvider]); + const { matched, unmatched } = runExactMatch( + [consumer, salesProvider, billingProvider], + providerIndex, + ); + + expect(matched).toHaveLength(0); + expect(unmatched).toEqual([consumer, salesProvider, billingProvider]); + }); + it('does not match a thrift wildcard to a gRPC provider', () => { const consumer = makeThriftContract('thrift::OrderService/*', 'consumer', 'frontend'); const provider = makeGrpcContract( diff --git a/gitnexus/test/unit/group/thrift-extractor.test.ts b/gitnexus/test/unit/group/thrift-extractor.test.ts index 22b5e004a..c27ab85fd 100644 --- a/gitnexus/test/unit/group/thrift-extractor.test.ts +++ b/gitnexus/test/unit/group/thrift-extractor.test.ts @@ -406,6 +406,25 @@ class BillingWorkflow { }); expect(contracts[0].symbolRef.filePath).toBe('src/main/java/example/BillingWorkflow.java'); }); + + it('test_extract_java_thrift_direct_service_consumer_without_idl_returns_empty', async () => { + writeFile( + 'src/main/java/example/PaymentWorkflow.java', + `package example; + +class PaymentWorkflow { + private PaymentService paymentService; + + void submit() { + paymentService.charge(); + } +}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + + expect(contracts).toEqual([]); + }); }); describe('buildThriftContext', () => {