diff --git a/docs/guides/microservices-thrift.md b/docs/guides/microservices-thrift.md index 4cb8b5d4f..01d0b89dc 100644 --- a/docs/guides/microservices-thrift.md +++ b/docs/guides/microservices-thrift.md @@ -9,7 +9,7 @@ This is not a framework integration guide. GitNexus reads portable Thrift IDL an ## Mental model - `.thrift` files define the canonical service contract. A method in an IDL service becomes a stable contract id in the form `thrift::./`. -- When GitNexus can identify a service but not a specific method, it emits a service wildcard in the form `thrift::./*`. +- Service wildcard ids in the form `thrift::./*` are supported as manifest and matching fallback forms when a service-level link is needed. - Java generated-code usage points GitNexus toward implementation and call sites. Providers commonly implement generated `Service.Iface`; consumers commonly hold or construct generated service interfaces or clients. - Group sync matches provider and consumer contracts with the same id, then cross-repo impact can hop through those links. - Framework-specific wiring should be modeled by extractor plugins, manifest links, or downstream integrations rather than hard-coded into core Thrift support. @@ -47,7 +47,7 @@ The service methods above produce canonical ids: - `thrift::billing.v1.OrderService/PlaceOrder` - `thrift::billing.v1.OrderService/GetOrder` -- `thrift::billing.v1.OrderService/*` when only the service is known +- `thrift::billing.v1.OrderService/*` as a service-level manifest or matching fallback form ## Java provider example diff --git a/gitnexus/src/core/group/extractors/thrift-patterns/java.ts b/gitnexus/src/core/group/extractors/thrift-patterns/java.ts index 20bdf95ec..cc067479a 100644 --- a/gitnexus/src/core/group/extractors/thrift-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/thrift-patterns/java.ts @@ -30,65 +30,33 @@ const VARIABLE_PATTERNS = compilePatterns({ language: Java, patterns: [ { - meta: { scoped: true }, + meta: {}, query: ` (field_declaration - (scoped_type_identifier - (type_identifier) @service - (type_identifier) @member) - (variable_declarator - (identifier) @var)) + type: (_) @type + declarator: (variable_declarator + name: (identifier) @var)) `, }, { - meta: { scoped: true }, + meta: {}, query: ` (local_variable_declaration - (scoped_type_identifier - (type_identifier) @service - (type_identifier) @member) - (variable_declarator - (identifier) @var)) + type: (_) @type + declarator: (variable_declarator + name: (identifier) @var)) `, }, { - meta: { scoped: true }, + meta: {}, query: ` (formal_parameter - (scoped_type_identifier - (type_identifier) @service - (type_identifier) @member) - (identifier) @var) - `, - }, - { - meta: { scoped: false }, - query: ` - (field_declaration - (type_identifier) @service - (variable_declarator - (identifier) @var)) - `, - }, - { - meta: { scoped: false }, - query: ` - (local_variable_declaration - (type_identifier) @service - (variable_declarator - (identifier) @var)) - `, - }, - { - meta: { scoped: false }, - query: ` - (formal_parameter - (type_identifier) @service - (identifier) @var) + type: (_) @type + name: (identifier) @var) `, }, ], -} satisfies LanguagePatterns<{ scoped: boolean }>); +} satisfies LanguagePatterns>); const CALL_PATTERNS = compilePatterns({ name: 'java-thrift-method-calls', @@ -102,6 +70,16 @@ const CALL_PATTERNS = compilePatterns({ name: (identifier) @method) `, }, + { + meta: {}, + query: ` + (method_invocation + object: (field_access + object: (this) + field: (identifier) @receiver) + name: (identifier) @method) + `, + }, ], } satisfies LanguagePatterns>); @@ -110,43 +88,28 @@ const PROVIDER_PATTERNS = compilePatterns({ language: Java, patterns: [ { - meta: { scoped: true }, + meta: {}, query: ` (class_declaration name: (identifier) @class_name (super_interfaces (type_list - (scoped_type_identifier - (type_identifier) @service - (type_identifier) @member))) - body: (class_body) @body) @class - `, - }, - { - meta: { scoped: false }, - query: ` - (class_declaration - name: (identifier) @class_name - (super_interfaces - (type_list - (type_identifier) @service)) + (_) @type)) body: (class_body) @body) @class `, }, ], -} satisfies LanguagePatterns<{ scoped: boolean }>); +} satisfies LanguagePatterns>); -function serviceFromType( - serviceText: string, - memberText: string | undefined, -): ServiceTypeMatch | null { - if (memberText !== undefined) { - return GENERATED_MEMBER_TYPES.has(memberText) - ? { serviceName: serviceText, usesGeneratedServiceMember: true } - : null; +function serviceFromType(typeText: string): ServiceTypeMatch | null { + const segments = typeText.split('.').filter((segment) => segment.length > 0); + const last = segments.at(-1); + const service = segments.at(-2); + if (last && service && GENERATED_MEMBER_TYPES.has(last)) { + return { serviceName: service, usesGeneratedServiceMember: true }; } - return SERVICE_TYPE_RE.test(serviceText) - ? { serviceName: serviceText, usesGeneratedServiceMember: false } + return last && SERVICE_TYPE_RE.test(last) + ? { serviceName: last, usesGeneratedServiceMember: false } : null; } @@ -228,11 +191,10 @@ export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { const bindings: VariableBinding[] = []; for (const match of runCompiledPatterns(VARIABLE_PATTERNS, tree)) { - const serviceNode = match.captures.service; + const typeNode = match.captures.type; const varNode = match.captures.var; - if (!serviceNode || !varNode) continue; - const memberNode = match.meta.scoped ? match.captures.member : undefined; - const service = serviceFromType(serviceNode.text, memberNode?.text); + if (!typeNode || !varNode) continue; + const service = serviceFromType(typeNode.text); if (!service) continue; const scope = bindingScope(varNode); if (!scope) continue; @@ -269,11 +231,10 @@ export const JAVA_THRIFT_PLUGIN: ThriftLanguagePlugin = { const emittedProviders = new Set(); for (const match of runCompiledPatterns(PROVIDER_PATTERNS, tree)) { - const serviceNode = match.captures.service; + const typeNode = match.captures.type; const bodyNode = match.captures.body; - if (!serviceNode || !bodyNode) continue; - const memberNode = match.meta.scoped ? match.captures.member : undefined; - const service = serviceFromType(serviceNode.text, memberNode?.text); + if (!typeNode || !bodyNode) continue; + const service = serviceFromType(typeNode.text); if (!service) continue; for (const methodName of methodNamesInClassBody(bodyNode)) { diff --git a/gitnexus/test/unit/group/thrift-extractor.test.ts b/gitnexus/test/unit/group/thrift-extractor.test.ts index c27ab85fd..224db8b6f 100644 --- a/gitnexus/test/unit/group/thrift-extractor.test.ts +++ b/gitnexus/test/unit/group/thrift-extractor.test.ts @@ -239,6 +239,86 @@ class BillingWorkflow { } }); + it('test_extract_java_thrift_consumers_from_this_field_access', async () => { + writeFile( + 'idl/order.thrift', + `namespace java billing.v1 + +service OrderService { + PlaceOrderResponse PlaceOrder(1: PlaceOrderRequest request) +}`, + ); + writeFile( + 'src/main/java/example/BillingWorkflow.java', + `package example; + +class BillingWorkflow { + private OrderService.Client orderClient; + + void submit(PlaceOrderRequest request) throws Exception { + this.orderClient.PlaceOrder(request); + } +}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(1); + expect(consumers[0]).toMatchObject({ + contractId: 'thrift::billing.v1.OrderService/PlaceOrder', + type: 'thrift', + role: 'consumer', + symbolName: 'orderClient.PlaceOrder', + confidence: 0.75, + meta: { + namespace: 'billing.v1', + service: 'OrderService', + method: 'PlaceOrder', + source: 'java_thrift_consumer', + }, + }); + }); + + it('test_extract_java_thrift_consumers_from_fully_qualified_generated_types', async () => { + writeFile( + 'idl/order.thrift', + `namespace java billing.v1 + +service OrderService { + PlaceOrderResponse PlaceOrder(1: PlaceOrderRequest request) +}`, + ); + writeFile( + 'src/main/java/example/BillingWorkflow.java', + `package example; + +class BillingWorkflow { + private billing.v1.OrderService.Iface orderService; + private billing.v1.OrderService.Client orderClient; + + void submit(PlaceOrderRequest request) throws Exception { + orderService.PlaceOrder(request); + orderClient.PlaceOrder(request); + } +}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts + .filter((c) => c.role === 'consumer') + .sort((a, b) => a.symbolName.localeCompare(b.symbolName)); + + expect(consumers).toHaveLength(2); + expect(consumers.map((c) => c.symbolName)).toEqual([ + 'orderClient.PlaceOrder', + 'orderService.PlaceOrder', + ]); + expect(new Set(consumers.map((c) => c.contractId))).toEqual( + new Set(['thrift::billing.v1.OrderService/PlaceOrder']), + ); + }); + it('test_extract_java_thrift_consumers_from_local_variables', async () => { writeFile( 'idl/order.thrift', @@ -375,6 +455,45 @@ class GeneratedOrderHandler implements OrderService { } }); + it('test_extract_java_thrift_providers_from_fully_qualified_generated_iface', async () => { + writeFile( + 'idl/order.thrift', + `namespace java billing.v1 + +service OrderService { + PlaceOrderResponse PlaceOrder(1: PlaceOrderRequest request) +}`, + ); + writeFile( + 'src/main/java/example/IfaceOrderHandler.java', + `package example; + +class IfaceOrderHandler implements billing.v1.OrderService.Iface { + public PlaceOrderResponse PlaceOrder(PlaceOrderRequest request) { + return new PlaceOrderResponse(); + } +}`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const providers = contracts.filter((c) => c.meta.source === 'java_thrift_provider'); + + expect(providers).toHaveLength(1); + expect(providers[0]).toMatchObject({ + contractId: 'thrift::billing.v1.OrderService/PlaceOrder', + type: 'thrift', + role: 'provider', + symbolName: 'OrderService.PlaceOrder', + confidence: 0.8, + meta: { + namespace: 'billing.v1', + service: 'OrderService', + method: 'PlaceOrder', + source: 'java_thrift_provider', + }, + }); + }); + it('test_extract_java_thrift_consumer_without_idl_emits_weak_method_contract', async () => { writeFile( 'src/main/java/example/BillingWorkflow.java',