scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-09 03:17:54 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

fix(group): stop Node gRPC loadPackageDefinition gate from matching every member call (#1916)

Browse source 
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.<Capitalized>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) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar 2026-05-30 09:31:36 +01:00 • committed by GitHub
parent 5d710413d7
commit f18ff521fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 64 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
gitnexus/src/core/group/extractors/grpc-patterns/node.ts
View file

@ -86,16 +86,33 @@ const NEW_QUALIFIED_CTOR_SPEC: PatternSpec<Record<string, never>> = {
// 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<Record<string, never>> = {
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<Record<string, never>>[] = [
{
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<Record<string, never>>;
@ -107,11 +124,14 @@ interface NodeGrpcPatternBundle {
}
function compileBundle(language: unknown, name: string): NodeGrpcPatternBundle {
const mk = (spec: PatternSpec<Record<string, never>>, suffix: string) =>
const mk = (
spec: PatternSpec<Record<string, never>> | PatternSpec<Record<string, never>>[],
suffix: string,
) =>
compilePatterns({
name: `${name}-${suffix}`,
language,
patterns: [spec],
patterns: Array.isArray(spec) ? spec : [spec],
} satisfies LanguagePatterns<Record<string, never>>);
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'),
};
}

31
gitnexus/test/unit/group/grpc-extractor.test.ts
View file

@ -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',