mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-04 02:31:36 +00:00
fix: address 3 review findings — static leakage in Phase 2, build-dir skip list, same-directory preference
Finding 1: Apply isFileLocalDef filtering in Phase 2 of pickUniqueGlobalCallable so cross-file static defs cannot leak through the SemanticModel fallback path. Finding 2: Expand scanHeaderFiles skip list with dist, build, out, target, _build, .next, cmake-build-* to avoid generated headers shadowing source ones. Finding 3: Implement same-directory sibling preference in resolveCImportTarget, matching C compiler #include "…" relative-lookup semantics. Sibling check now runs before exact match and suffix fallback. Tests: 11 new header-scan tests, 4 new import-target tests (96 total C tests). Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/5236f2b8-72a0-476d-bf39-cca041781014 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com>
This commit is contained in:
parent
56f4e4d0f5
commit
79bbd7872e
5 changed files with 189 additions and 13 deletions
|
|
@ -27,8 +27,21 @@ function walk(dir: string, root: string, out: Set<string>): void {
|
|||
const name = entry.name;
|
||||
const full = join(dir, name);
|
||||
if (entry.isDirectory()) {
|
||||
// Skip common non-source directories
|
||||
if (name === 'node_modules' || name === '.git' || name === 'vendor') {
|
||||
// Skip common non-source directories and build output dirs.
|
||||
// Build dirs (dist, build, out, target, _build, .next, cmake-build-*)
|
||||
// may contain generated headers that shadow source headers.
|
||||
if (
|
||||
name === 'node_modules' ||
|
||||
name === '.git' ||
|
||||
name === 'vendor' ||
|
||||
name === 'dist' ||
|
||||
name === 'build' ||
|
||||
name === 'out' ||
|
||||
name === 'target' ||
|
||||
name === '_build' ||
|
||||
name === '.next' ||
|
||||
name.startsWith('cmake-build')
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
walk(full, root, out);
|
||||
|
|
|
|||
|
|
@ -1,26 +1,41 @@
|
|||
import { dirname, join } from 'path';
|
||||
|
||||
/**
|
||||
* Resolve a C #include path to a file in the workspace.
|
||||
*
|
||||
* Strategy: match the include path suffix against all file paths in
|
||||
* the workspace. "foo.h" matches "src/foo.h", "include/foo.h", etc.
|
||||
* For paths with directory components ("dir/foo.h"), match the full
|
||||
* relative suffix.
|
||||
*
|
||||
* Tie-breaking: prefer the match with the fewest path components
|
||||
* (closest to root). On equal depth, break ties lexicographically
|
||||
* by normalized path to ensure deterministic resolution regardless
|
||||
* of filesystem iteration order.
|
||||
* Strategy:
|
||||
* 1. Check for a same-directory sibling relative to the including file
|
||||
* (matches C compiler `#include "…"` relative-lookup semantics).
|
||||
* 2. Check for an exact match (path as-is in the workspace).
|
||||
* 3. Fall back to suffix matching against all workspace file paths.
|
||||
* Tie-breaking: prefer the match with the fewest path components
|
||||
* (closest to root). On equal depth, break ties lexicographically
|
||||
* by normalized path to ensure deterministic resolution regardless
|
||||
* of filesystem iteration order.
|
||||
*/
|
||||
export function resolveCImportTarget(
|
||||
targetRaw: string,
|
||||
_fromFile: string,
|
||||
fromFile: string,
|
||||
allFilePaths: ReadonlySet<string>,
|
||||
): string | null {
|
||||
if (!targetRaw) return null;
|
||||
|
||||
const normalizedTarget = targetRaw.replace(/\\/g, '/');
|
||||
|
||||
// Exact match first
|
||||
// Same-directory sibling first: mirrors the C compiler's #include "…"
|
||||
// relative-lookup semantics where the directory of the including
|
||||
// file is searched before the include-path list.
|
||||
if (fromFile) {
|
||||
const siblingRaw = join(dirname(fromFile), targetRaw);
|
||||
const sibling = siblingRaw.replace(/\\/g, '/');
|
||||
if (allFilePaths.has(sibling)) return sibling;
|
||||
// Also try normalized target in case targetRaw has slashes
|
||||
const siblingAlt = join(dirname(fromFile), normalizedTarget);
|
||||
const siblingAltNorm = siblingAlt.replace(/\\/g, '/');
|
||||
if (siblingAltNorm !== sibling && allFilePaths.has(siblingAltNorm)) return siblingAltNorm;
|
||||
}
|
||||
|
||||
// Exact match (path as-is in the workspace)
|
||||
if (allFilePaths.has(normalizedTarget)) return normalizedTarget;
|
||||
|
||||
// Suffix match: find files ending with /targetRaw or equal to targetRaw
|
||||
|
|
|
|||
|
|
@ -141,6 +141,12 @@ function pickUniqueGlobalCallable(
|
|||
const seen = new Set<string>();
|
||||
const push = (pool: readonly SymbolDefinition[]): void => {
|
||||
for (const def of pool) {
|
||||
// Apply the same file-local linkage filter as Phase 1 —
|
||||
// cross-file static defs must never leak through the
|
||||
// SemanticModel fallback path.
|
||||
if (isFileLocalDef !== undefined && def.filePath !== callerFilePath && isFileLocalDef(def)) {
|
||||
continue;
|
||||
}
|
||||
const key = logicalCallableKey(def);
|
||||
if (seen.has(key)) continue;
|
||||
seen.add(key);
|
||||
|
|
|
|||
103
gitnexus/test/unit/scope-resolution/c/c-header-scan.test.ts
Normal file
103
gitnexus/test/unit/scope-resolution/c/c-header-scan.test.ts
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
/**
|
||||
* Unit tests for C header scanning — specifically the skip-list
|
||||
* for build output directories.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdirSync, writeFileSync, rmSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { scanHeaderFiles } from '../../../../src/core/ingestion/languages/c/header-scan.js';
|
||||
|
||||
const TMP = join(__dirname, '__header_scan_tmp__');
|
||||
|
||||
function touch(rel: string): void {
|
||||
const full = join(TMP, rel);
|
||||
mkdirSync(join(full, '..'), { recursive: true });
|
||||
writeFileSync(full, '');
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mkdirSync(TMP, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(TMP, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('scanHeaderFiles — build-directory skip list', () => {
|
||||
it('finds .h files in source directories', () => {
|
||||
touch('src/foo.h');
|
||||
touch('include/bar.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers).toContain('src/foo.h');
|
||||
expect(headers).toContain('include/bar.h');
|
||||
});
|
||||
|
||||
it('skips node_modules', () => {
|
||||
touch('node_modules/dep/header.h');
|
||||
touch('src/real.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers).not.toContain('node_modules/dep/header.h');
|
||||
expect(headers).toContain('src/real.h');
|
||||
});
|
||||
|
||||
it('skips .git directory', () => {
|
||||
touch('.git/refs/header.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips vendor directory', () => {
|
||||
touch('vendor/lib/header.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips dist directory', () => {
|
||||
touch('dist/generated.h');
|
||||
touch('src/real.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers).not.toContain('dist/generated.h');
|
||||
expect(headers).toContain('src/real.h');
|
||||
});
|
||||
|
||||
it('skips build directory', () => {
|
||||
touch('build/config.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers).not.toContain('build/config.h');
|
||||
});
|
||||
|
||||
it('skips out directory', () => {
|
||||
touch('out/gen/auto.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips target directory', () => {
|
||||
touch('target/release/bindings.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips _build directory', () => {
|
||||
touch('_build/default/lib.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips .next directory', () => {
|
||||
touch('.next/cache/header.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers.size).toBe(0);
|
||||
});
|
||||
|
||||
it('skips cmake-build-* directories', () => {
|
||||
touch('cmake-build-debug/generated.h');
|
||||
touch('cmake-build-release/generated.h');
|
||||
touch('src/real.h');
|
||||
const headers = scanHeaderFiles(TMP);
|
||||
expect(headers).not.toContain('cmake-build-debug/generated.h');
|
||||
expect(headers).not.toContain('cmake-build-release/generated.h');
|
||||
expect(headers).toContain('src/real.h');
|
||||
});
|
||||
});
|
||||
|
|
@ -128,4 +128,43 @@ describe('C import target resolution (resolveCImportTarget)', () => {
|
|||
const result = resolveCImportTarget('foo.h', 'main.c', new Set(['include\\foo.h']));
|
||||
expect(result).toBe('include\\foo.h');
|
||||
});
|
||||
|
||||
it('prefers same-directory sibling over deeper suffix match', () => {
|
||||
// src/foo.c includes "bar.h" — src/bar.h should win over include/bar.h
|
||||
const result = resolveCImportTarget(
|
||||
'bar.h',
|
||||
'src/foo.c',
|
||||
new Set(['include/bar.h', 'src/bar.h']),
|
||||
);
|
||||
expect(result).toBe('src/bar.h');
|
||||
});
|
||||
|
||||
it('prefers same-directory sibling over shallower suffix match', () => {
|
||||
// deep/nested/main.c includes "foo.h" — deep/nested/foo.h wins over foo.h
|
||||
const result = resolveCImportTarget(
|
||||
'foo.h',
|
||||
'deep/nested/main.c',
|
||||
new Set(['foo.h', 'deep/nested/foo.h']),
|
||||
);
|
||||
expect(result).toBe('deep/nested/foo.h');
|
||||
});
|
||||
|
||||
it('falls back to suffix match when no same-directory sibling exists', () => {
|
||||
const result = resolveCImportTarget(
|
||||
'missing.h',
|
||||
'src/foo.c',
|
||||
new Set(['lib/missing.h']),
|
||||
);
|
||||
expect(result).toBe('lib/missing.h');
|
||||
});
|
||||
|
||||
it('same-directory sibling with nested target path', () => {
|
||||
// src/foo.c includes "sub/bar.h" — src/sub/bar.h should win
|
||||
const result = resolveCImportTarget(
|
||||
'sub/bar.h',
|
||||
'src/foo.c',
|
||||
new Set(['other/sub/bar.h', 'src/sub/bar.h']),
|
||||
);
|
||||
expect(result).toBe('src/sub/bar.h');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue