From f18ff521fcbe5ca2c2eefc5475963dc4cc283cf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 30 May 2026 09:31:36 +0100 Subject: [PATCH] fix(group): stop Node gRPC loadPackageDefinition gate from matching every member call (#1916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LOAD_PACKAGE_DEFINITION_SPEC matched `loadPackageDefinition` via a single `function: [ (identifier) @fn (#eq?) (member_expression property:(property_identifier) @fn (#eq?)) ]` alternation. Under the pinned tree-sitter@0.21.1 binding a top-level alternation whose branches reuse one capture name collapses to a single pattern with a shared predicate bucket: the member-expression branch's `@fn` is left unbound and its `#eq?` is never enforced, so that branch matches EVERY `obj.method(...)` call (`console.log(...)`, `logger.info(...)`, …). Since virtually every TS/JS file has some member call, the `usesLoadPackage` gate was effectively always-open and `new pkg.Service(...)` was emitted as a spurious gRPC consumer — the exact false positive the gate was added to prevent. Split the spec into two single-branch PatternSpecs; each compiles to its own Parser.Query with an independent predicate bucket where the `#eq?` is enforced correctly. `runCompiledPatterns` concatenates their matches, so the `.length > 0` gate is unchanged. `mk` now accepts a spec or a spec array. Adds test_extract_ts_qualified_ctor_without_loadPackageDefinition_is_ignored, a negative regression test verified to FAIL on the pre-fix code and PASS with the fix: a file with no loadPackageDefinition but an unrelated member call + `new authProto.auth.v1.AuthService(...)` must emit no consumer. grpc-extractor suite 65/65; tsc + prettier + pre-commit hook clean. Co-authored-by: Claude Opus 4.8 (1M context) --- .../group/extractors/grpc-patterns/node.ts | 46 +++++++++++++------ .../test/unit/group/grpc-extractor.test.ts | 31 +++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/gitnexus/src/core/group/extractors/grpc-patterns/node.ts b/gitnexus/src/core/group/extractors/grpc-patterns/node.ts index 033962206..72a237fba 100644 --- a/gitnexus/src/core/group/extractors/grpc-patterns/node.ts +++ b/gitnexus/src/core/group/extractors/grpc-patterns/node.ts @@ -86,16 +86,33 @@ const NEW_QUALIFIED_CTOR_SPEC: PatternSpec> = { // proto loader). Matches either a bare call or an `obj.loadPackageDefinition(...)` // call. Plugin gates the qualified-constructor consumer on this — // structural check avoids materializing `tree.rootNode.text` for every file. -const LOAD_PACKAGE_DEFINITION_SPEC: PatternSpec> = { - meta: {}, - query: ` - (call_expression - function: [ - (identifier) @fn (#eq? @fn "loadPackageDefinition") - (member_expression property: (property_identifier) @fn (#eq? @fn "loadPackageDefinition")) - ]) - `, -}; +// +// These are TWO separate specs, NOT one `function: [ (identifier) ... (member_expression) ... ]` +// alternation. Under the pinned tree-sitter@0.21.1 binding a top-level alternation +// whose branches reuse the same capture name (`@fn`) collapses to one pattern with +// a shared predicate bucket; the second branch's `@fn` is left unbound and its +// `#eq?` is never enforced, so the member-expression branch would match EVERY +// `obj.method(...)` call (e.g. `console.log(...)`) — turning this gate always-on +// and emitting spurious qualified-constructor consumers. Two specs compile to two +// queries with independent predicate buckets; `runCompiledPatterns` concatenates +// their matches, so the `.length > 0` gate still means "either form is present". +const LOAD_PACKAGE_DEFINITION_SPECS: PatternSpec>[] = [ + { + meta: {}, + query: ` + (call_expression + function: (identifier) @fn (#eq? @fn "loadPackageDefinition")) + `, + }, + { + meta: {}, + query: ` + (call_expression + function: (member_expression + property: (property_identifier) @fn (#eq? @fn "loadPackageDefinition"))) + `, + }, +]; interface NodeGrpcPatternBundle { grpcMethod: CompiledPatterns>; @@ -107,11 +124,14 @@ interface NodeGrpcPatternBundle { } function compileBundle(language: unknown, name: string): NodeGrpcPatternBundle { - const mk = (spec: PatternSpec>, suffix: string) => + const mk = ( + spec: PatternSpec> | PatternSpec>[], + suffix: string, + ) => compilePatterns({ name: `${name}-${suffix}`, language, - patterns: [spec], + patterns: Array.isArray(spec) ? spec : [spec], } satisfies LanguagePatterns>); return { grpcMethod: mk(GRPC_METHOD_SPEC, 'grpc-method'), @@ -119,7 +139,7 @@ function compileBundle(language: unknown, name: string): NodeGrpcPatternBundle { getService: mk(GET_SERVICE_SPEC, 'get-service'), newSimpleCtor: mk(NEW_SIMPLE_CTOR_SPEC, 'new-simple-ctor'), newQualifiedCtor: mk(NEW_QUALIFIED_CTOR_SPEC, 'new-qualified-ctor'), - loadPackageDefinition: mk(LOAD_PACKAGE_DEFINITION_SPEC, 'load-package-definition'), + loadPackageDefinition: mk(LOAD_PACKAGE_DEFINITION_SPECS, 'load-package-definition'), }; } diff --git a/gitnexus/test/unit/group/grpc-extractor.test.ts b/gitnexus/test/unit/group/grpc-extractor.test.ts index 1a4a6ef47..bf1a98033 100644 --- a/gitnexus/test/unit/group/grpc-extractor.test.ts +++ b/gitnexus/test/unit/group/grpc-extractor.test.ts @@ -1135,6 +1135,37 @@ export const authClient = new authProto.auth.v1.AuthService( expect(consumers[0].contractId).toBe('grpc::auth.v1.AuthService/*'); }); + it('test_extract_ts_qualified_ctor_without_loadPackageDefinition_is_ignored', async () => { + // Regression: an unrelated `obj.method(...)` member call must not trip the + // loadPackageDefinition gate. With no loadPackageDefinition call present, a + // qualified `new pkg...Service(...)` constructor must NOT become a consumer. + // Pre-fix, the gate's shared-capture `function: [...]` alternation matched + // every member call, so this spuriously emitted an AuthService consumer. + writeFile( + 'proto/auth.proto', + `syntax = "proto3"; +package auth.v1; +service AuthService { + rpc Login (LoginRequest) returns (LoginResponse); +}`, + ); + writeFile( + 'src/auth.client.ts', + `import * as grpc from '@grpc/grpc-js'; + +logger.info('starting up'); +export const authClient = new authProto.auth.v1.AuthService( + 'localhost:50051', + grpc.credentials.createInsecure(), +);`, + ); + + const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir)); + const consumers = contracts.filter((c) => c.role === 'consumer'); + + expect(consumers).toHaveLength(0); + }); + it('test_extract_ts_duplicate_consumer_patterns_in_one_file_dedupes_deterministically', async () => { writeFile( 'proto/auth.proto',