fix(group): drop macro-style #include from consumer contracts

Tree-sitter's `(_) @import.source` wildcard matches the identifier node
of `#include PLATFORM_HEADER`, so the cleaned value `PLATFORM_HEADER`
slipped past the system-header / `..` filters and was emitted as a
permanently orphaned consumer contract (no file is named after a macro
identifier, so no provider can ever match). Add a shape guard that
skips cleaned values lacking both a path separator and an extension
dot, plus regression tests for single and multi-macro files.

Also document `IncludeExtractor.canExtract()` as unused by sync.ts
(gated via `config.detect.includes` instead) and kept solely for
ContractExtractor interface uniformity.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-05-09 06:49:18 +01:00
parent 8adf4cbaba
commit 651a4e15ac
2 changed files with 56 additions and 0 deletions

View file

@ -296,6 +296,11 @@ function isLocalInclude(cleaned: string, suffixIndex: SuffixIndex): boolean {
export class IncludeExtractor implements ContractExtractor {
type = 'include' as const;
/**
* Always returns `true`. NOT called by `sync.ts`, which gates extraction via
* `config.detect.includes` instead (see `sync.ts:174`). Kept solely to satisfy
* the `ContractExtractor` interface so the type stays uniform across extractors.
*/
async canExtract(_repo: RepoHandle): Promise<boolean> {
return true;
}
@ -544,6 +549,19 @@ export class IncludeExtractor implements ContractExtractor {
// spurious consumer contracts.)
if (cleaned.startsWith('../') || cleaned.startsWith('..\\')) continue;
// Skip macro-style includes: `#include PLATFORM_HEADER` parses as an
// identifier under tree-sitter's `(_) @import.source` wildcard. The
// identifier text passes the strip/clean step unchanged, so without
// this guard we would emit `include::platform_header` as a consumer
// contract — and no provider in any repo will ever expose a contract
// for a macro identifier (no file is named `PLATFORM_HEADER`). The
// contract would sit permanently orphaned in the registry. Real
// header references always contain a path separator (`/`, `\`) or an
// extension dot (`foo.h`), so an absent both is a reliable signal we
// are looking at a macro identifier. (PR #1156 follow-up review:
// macro includes emit orphaned consumer contracts.)
if (!/[./\\]/.test(cleaned)) continue;
// Local resolution (PR #1156 review finding #4): only accept an
// exact-suffix match on the *full* include path. The generic
// suffixResolve() iterates all truncated suffixes, which would

View file

@ -398,6 +398,44 @@ int main() { return 0; }`,
});
});
// ---- PR #1156 follow-up: macro-style includes ----
describe('follow-up: macro-style #include emits no consumer contract', () => {
it('does not emit a consumer contract for `#include PLATFORM_HEADER` (no separator, no dot)', async () => {
// `#include PLATFORM_HEADER` parses under tree-sitter as an identifier
// node, slips past the existing system-header / `..` filters, and used
// to leak through as a permanently orphaned consumer contract because
// no file is ever named `PLATFORM_HEADER`. Verify the macro guard
// suppresses it while preserving the real cross-repo include.
writeFile(
'src/main.cpp',
`#include PLATFORM_HEADER
#include "real/api.h"
int main() { return 0; }`,
);
const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir));
const consumers = contracts.filter((c) => c.role === 'consumer');
expect(consumers.map((c) => c.contractId)).toEqual(['include::real/api.h']);
});
it('skips multiple macro identifiers in the same translation unit', async () => {
writeFile(
'src/cfg.cpp',
`#include CONFIG_HEADER
#include PLATFORM_HEADER
#include ASSERT_H_
int main(){return 0;}`,
);
const contracts = await extractor.extract(null, tmpDir, makeRepo(tmpDir));
const consumers = contracts.filter((c) => c.role === 'consumer');
expect(consumers).toHaveLength(0);
});
});
// ---- PR #1156 follow-up: graph provider absolute paths ----
describe('follow-up: extractProvidersGraph strips repo root from absolute paths', () => {