mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-11 08:37:35 +00:00
fix(impact): avoid upstream file seeds for private types (#3740)
* fix(impact): do not seed the owning file for a type the index recorded as not exported The #480 seed adds the file that DEFINES a Class or Interface to the walk, so the file's IMPORTS edges stand in for the type's importers. For a type recorded as isExported = false no importer can name it, and the file's importers came back as its direct dependants: a private interface used by one function in its file reported eight route files at depth 1, direct: 8, MEDIUM (#3602, second reproduction). The DEFINES seed query now also returns n.isExported, matched on the target's own label since not every node table has the column, and a row with false is left out of the frontier. true and NULL keep the seed. Refs #3602 * fix(impact): skip the owning-file seed only where a false export verdict is file-private `isExported = false` is a narrower fact than "no other file can name it" in most languages: Java package-private, C# and Kotlin `internal`, Go lowercase, Python and Dart `_name`, a Rust item seen from a child module, a C/C++ `static` in a header. Skipping the #480 file seed on every `false` turned the over-approximation into a false negative for them. The meaning of a stored verdict is a property of the language, so it lives on the provider: `LanguageProvider.notExportedIsFilePrivate`, set on TypeScript, JavaScript, Vue (an ES module is the file), Swift (false only for `private`/`fileprivate`) and Zig (no `pub`; `export fn` is recorded true). The seed resolves the provider from the file's path only for a `false` row and skips the file only when the flag is set. Two regressions in the same test file: a Java package-private class and a Kotlin `internal` class, both recorded `false`, keep their importers as direct dependants. Refs #3602 * fix(impact): require an ES export in the file before trusting a false verdict The stored verdict is the provider's export checker, which for an ECMAScript file answers "under an `export` statement?" whatever the module kind: a CommonJS class (`module.exports = Foo`) and a top-level declaration of a global script are both recorded `false`, and both can be named from other files. Trusting the provider flag alone skipped the owning-file seed for them. The index does not record the module kind, but it records the checker's `true`, which only ES `export` syntax produces. The skip now also requires a `true` declaration in the same file (Function, Class or Interface: the tables that carry the column and that a File DEFINES). A CommonJS file, a script, or a module whose exports all live in `Const`/`TypeAlias` keeps the seed, as before this fix. The Java and Kotlin cases now declare a public sibling class, so only the provider verdict keeps their seed; two new cases cover a CommonJS module and a global script. Refs #3602 * fix(impact): preserve uncertain type dependencies * fix(impact): retain exported type alias dependencies --------- Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
This commit is contained in:
samuel mbabhazi
GitHub
parent
70e226f9c6
commit
51ce19d77a
11 changed files with 571 additions and 2 deletions
|
|
@ -161,6 +161,8 @@ const PLATFORM_LOGIC = [
|
|||
// N-API addon which has known platform-specific behavior (Windows
|
||||
// file-lock lag after close, macOS N-API destructor segfaults)
|
||||
const LBUG_NATIVE = [
|
||||
'test/integration/impact-private-type-file-seed.test.ts',
|
||||
'test/integration/impact-private-type-source.test.ts',
|
||||
// Inline-handler findings must survive native persistence and cold explain
|
||||
// readers on each OS; passing the Ubuntu-only taint suite is insufficient.
|
||||
'test/integration/taint-explain.test.ts',
|
||||
|
|
|
|||
|
|
@ -354,6 +354,33 @@ interface LanguageProviderConfig {
|
|||
/** Export detection: is this AST node a public/exported symbol? */
|
||||
readonly exportChecker: ExportChecker;
|
||||
|
||||
/**
|
||||
* Does a stored `false` from `exportChecker` prove file privacy? A function
|
||||
* can confirm this against the indexed file content; missing content must
|
||||
* retain the fallback. The graph flag is NOT SymbolDefinition's tri-state
|
||||
* export evidence: JS/TS export clauses and CommonJS can also store false.
|
||||
* True for Swift `private`/`fileprivate`, a Zig declaration without
|
||||
* `pub` (an `export fn` without `pub` is recorded true for its C linkage, so
|
||||
* `false` never hides an FFI symbol).
|
||||
*
|
||||
* Absent where `false` is a narrower fact than "file-private": Java
|
||||
* package-private, C# and Kotlin `internal`, Go lowercase (the rest of the
|
||||
* package / assembly / module still sees it); Python and Dart `_name` (a
|
||||
* convention, `from m import _x` works and a Dart `part` shares its
|
||||
* library's privates); Rust (a child module reaches `super::` privates);
|
||||
* C/C++ (`static` or an anonymous namespace in a header is visible to every
|
||||
* file that `#include`s it).
|
||||
*
|
||||
* Read by consumers that drop a file-level fallback for a not-exported
|
||||
* symbol — the owning-file seed of `impact` (#3602) — which must otherwise
|
||||
* keep the fallback and over-approximate rather than miss a dependant. A
|
||||
* capability on the provider rather than a language set in `mcp/`: the
|
||||
* meaning of a stored `isExported` is a property of the language.
|
||||
*
|
||||
* Default: undefined, read as `false`.
|
||||
*/
|
||||
readonly notExportedIsFilePrivate?: boolean | ((filePath: string, content: string) => boolean);
|
||||
|
||||
// ── Calls & Imports (optional) ────────────────────────────────────
|
||||
/** Call routing for languages that express imports/heritage as calls (e.g., Ruby).
|
||||
* Default: no routing (all calls are normal call expressions). */
|
||||
|
|
|
|||
|
|
@ -237,6 +237,9 @@ export const swiftProvider = defineLanguage({
|
|||
treeSitterQueries: SWIFT_QUERIES,
|
||||
typeConfig: swiftConfig,
|
||||
exportChecker: swiftExportChecker,
|
||||
// `swiftExportChecker` returns false only for `private` / `fileprivate`,
|
||||
// both file-scoped; the module-wide default (`internal`) is recorded true.
|
||||
notExportedIsFilePrivate: true,
|
||||
callExtractor: createCallExtractor(swiftCallConfig),
|
||||
fieldExtractor: createFieldExtractor(swiftFieldConfig),
|
||||
methodExtractor: createMethodExtractor({
|
||||
|
|
|
|||
|
|
@ -79,6 +79,7 @@ const memberAssignmentLabel = (node: SyntaxNode): NodeLabel | null => {
|
|||
import { createTypeScriptCfgVisitor } from '../cfg/visitors/typescript.js';
|
||||
import { typeConfig as typescriptConfig } from '../type-extractors/typescript.js';
|
||||
import { tsExportChecker } from '../export-detection.js';
|
||||
import { hasFilePrivateExportSemantics } from './typescript/file-private-exports.js';
|
||||
import { TYPESCRIPT_QUERIES, JAVASCRIPT_QUERIES } from '../tree-sitter-queries.js';
|
||||
import { typescriptFieldExtractor } from '../field-extractors/typescript.js';
|
||||
import { createFieldExtractor } from '../field-extractors/generic.js';
|
||||
|
|
@ -517,6 +518,7 @@ export const typescriptProvider = defineLanguage({
|
|||
typeConfig: typescriptConfig,
|
||||
scopeOwnsReceivers: tsScopeOwnsReceivers,
|
||||
exportChecker: tsExportChecker,
|
||||
notExportedIsFilePrivate: hasFilePrivateExportSemantics,
|
||||
callExtractor: createCallExtractor(typescriptCallConfig),
|
||||
fieldExtractor: typescriptFieldExtractor,
|
||||
methodExtractor: createMethodExtractor({
|
||||
|
|
@ -621,6 +623,7 @@ export const javascriptProvider = defineLanguage({
|
|||
typeConfig: typescriptConfig,
|
||||
scopeOwnsReceivers: tsScopeOwnsReceivers,
|
||||
exportChecker: tsExportChecker,
|
||||
notExportedIsFilePrivate: hasFilePrivateExportSemantics,
|
||||
callExtractor: createCallExtractor(javascriptCallConfig),
|
||||
fieldExtractor: createFieldExtractor(javascriptConfig),
|
||||
methodExtractor: createMethodExtractor({
|
||||
|
|
|
|||
|
|
@ -0,0 +1,45 @@
|
|||
import { parseSourceSafe } from '../../../tree-sitter/safe-parse.js';
|
||||
import { getTreeSitterBufferSize } from '../../constants.js';
|
||||
import { collectEsmExportEvidence } from '../../ts-js-export-marker.js';
|
||||
import { getJsParser } from '../javascript/query.js';
|
||||
import { getTsParser } from './query.js';
|
||||
|
||||
/** Certify only modules whose exports the graph's syntactic checker can see. */
|
||||
export function hasFilePrivateExportSemantics(filePath: string, content: string): boolean {
|
||||
// File.content is FTS-normalized: newlines become spaces. A line comment
|
||||
// could therefore hide a later export from this parse. Never certify it.
|
||||
if (
|
||||
!content ||
|
||||
content.includes('//') ||
|
||||
content.includes('[truncated]') ||
|
||||
/\.d\.[cm]?ts$/i.test(filePath)
|
||||
)
|
||||
return false;
|
||||
try {
|
||||
const parser = /\.[cm]?jsx?$/i.test(filePath) ? getJsParser(filePath) : getTsParser(filePath);
|
||||
const tree = parseSourceSafe(parser, content, undefined, {
|
||||
bufferSize: getTreeSitterBufferSize(content),
|
||||
});
|
||||
if (tree.rootNode.hasError) return false;
|
||||
const evidence = collectEsmExportEvidence(tree.rootNode, filePath);
|
||||
if (!evidence || evidence.commonJs) return false;
|
||||
let isModule = false;
|
||||
for (const statement of tree.rootNode.namedChildren) {
|
||||
// Ambient modules and global augmentations publish otherwise unexported types.
|
||||
if (statement.type === 'ambient_declaration') return false;
|
||||
if (statement.type === 'import_statement') isModule = true;
|
||||
if (statement.type !== 'export_statement') continue;
|
||||
isModule = true;
|
||||
// `export { T }`, `export default T`, and `export = T` may publish a
|
||||
// declaration stored as false. Type aliases can do the same without a
|
||||
// graph edge to the underlying type. Retain the file seed for the whole file.
|
||||
const declaration = statement.childForFieldName('declaration');
|
||||
if (!declaration || declaration.type === 'type_alias_declaration') return false;
|
||||
}
|
||||
// Script declarations may be global across files without any exports.
|
||||
return isModule;
|
||||
} catch {
|
||||
// Unreadable or unsupported source is no proof of privacy.
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
@ -88,6 +88,9 @@ export const zigProvider = defineLanguage({
|
|||
typeConfig: zigTypeConfig,
|
||||
cfgVisitor: createZigCfgVisitor(),
|
||||
exportChecker: zigExportChecker,
|
||||
// No `pub` → unreachable through `@import` from another file. `export fn`
|
||||
// without `pub` is recorded true (C linkage), so false never hides FFI.
|
||||
notExportedIsFilePrivate: true,
|
||||
isExternalModuleSpecifier: (specifier) => ZIG_RESERVED_MODULES.has(specifier),
|
||||
callExtractor: createCallExtractor(zigCallConfig),
|
||||
classExtractor: createClassExtractor(zigClassConfig),
|
||||
|
|
|
|||
|
|
@ -8524,9 +8524,28 @@ export class LocalBackend {
|
|||
// The owning File is kept only as an internal seed (frontier/visited) and
|
||||
// is NOT added to impacted — it is the definition container, not an
|
||||
// upstream dependent. The BFS will discover IMPORTS edges on it naturally.
|
||||
//
|
||||
// #3602: the file's importers stand in for the type's importers, which
|
||||
// only holds when the type can leave the file. Where the index recorded
|
||||
// `isExported = false` AND the language's provider confirms that verdict
|
||||
// is file-private (checking indexed source where needed), no
|
||||
// importer can name the type, so seeding its file would report every
|
||||
// importer of the file as a direct dependant. The seed is skipped for it;
|
||||
// its real dependants still arrive through USES/EXTENDS/IMPLEMENTS edges
|
||||
// inside the file. Everything else keeps the seed: a missing verdict
|
||||
// (NULL) proves nothing; JS/TS also need to exclude indirect exports,
|
||||
// CommonJS and ambient files even when the stored flag is false. Where
|
||||
// `false` means package-, assembly- or module-private (Java, C#, Kotlin,
|
||||
// Go, …) a sibling file can still name the type, so the over-approximation
|
||||
// is the safe side. Downstream still needs the file's imports: a private
|
||||
// type can depend on other files even though they cannot import its name.
|
||||
if (symType === 'Class' || symType === 'Interface') {
|
||||
try {
|
||||
// Run both seed queries in parallel — they are independent.
|
||||
// The label is literal: only these two tables reach this branch, and
|
||||
// not every node table carries `isExported` (Protocol/Category do
|
||||
// not), so an untyped `n.isExported` is not a safe projection.
|
||||
const typeLabel = symType === 'Class' ? 'Class' : 'Interface';
|
||||
const [ctorRows, fileRows] = await Promise.all([
|
||||
executeParameterized(
|
||||
repo.lbugPath,
|
||||
|
|
@ -8542,9 +8561,10 @@ export class LocalBackend {
|
|||
executeParameterized(
|
||||
repo.lbugPath,
|
||||
`
|
||||
MATCH (f:File)-[rel:CodeRelation]->(n)
|
||||
MATCH (f:File)-[rel:CodeRelation]->(n:${typeLabel})
|
||||
WHERE n.id = $symId AND rel.type = 'DEFINES'
|
||||
RETURN f.id AS id, f.name AS name, labels(f)[0] AS type, f.filePath AS filePath
|
||||
RETURN f.id AS id, f.name AS name, labels(f)[0] AS type, f.filePath AS filePath, n.isExported AS isExported,
|
||||
CASE WHEN n.isExported = false THEN f.content ELSE NULL END AS content
|
||||
`,
|
||||
{ symId },
|
||||
),
|
||||
|
|
@ -8560,6 +8580,21 @@ export class LocalBackend {
|
|||
}
|
||||
for (const r of fileRows) {
|
||||
assertSymbolRowIdentity(r);
|
||||
const exported = r.isExported ?? r[4];
|
||||
if (direction === 'upstream' && exported === false) {
|
||||
// Lazy like the other provider lookups on this backend: the
|
||||
// registry pulls every language module, and this branch is the
|
||||
// only reader on the impact path.
|
||||
const { getProviderForFile } = await import('../../core/ingestion/languages/index.js');
|
||||
const filePath = String(r.filePath ?? r[3] ?? '');
|
||||
const filePrivate = getProviderForFile(filePath)?.notExportedIsFilePrivate;
|
||||
if (
|
||||
filePrivate === true ||
|
||||
(typeof filePrivate === 'function' &&
|
||||
filePrivate(filePath, String(r.content ?? r[5] ?? '')))
|
||||
)
|
||||
continue;
|
||||
}
|
||||
const rid = r.id || r[0];
|
||||
if (rid && !visited.has(rid)) {
|
||||
visited.add(rid);
|
||||
|
|
|
|||
252
gitnexus/test/integration/impact-private-type-file-seed.test.ts
Normal file
252
gitnexus/test/integration/impact-private-type-file-seed.test.ts
Normal file
|
|
@ -0,0 +1,252 @@
|
|||
import { beforeAll, expect, it, vi } from 'vitest';
|
||||
import { LocalBackend } from '../../src/mcp/local/local-backend.js';
|
||||
import { listRegisteredRepos } from '../../src/storage/repo-manager.js';
|
||||
import { withTestLbugDB, type IndexedDBHandle } from '../helpers/test-indexed-db.js';
|
||||
|
||||
vi.mock('../../src/storage/repo-manager.js', () => ({
|
||||
listRegisteredRepos: vi.fn().mockResolvedValue([]),
|
||||
cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }),
|
||||
findSiblingClones: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
// #3602 repro 2: `src/lib/svc.ts` declares a module-private interface `Entry`,
|
||||
// used by `parse`, which `verify` (the file's only export) calls. Eight route
|
||||
// files import `verify`. The owning-file seed (#480) turned those eight
|
||||
// importers into direct dependants of `Entry`, a name none of them can reach.
|
||||
const ROUTE_COUNT = 8;
|
||||
const SIBLING_COUNT = 3;
|
||||
const SIBLING_EXTENSION: Record<string, string> = {
|
||||
java: 'java',
|
||||
kotlin: 'kt',
|
||||
cjs: 'js',
|
||||
script: 'ts',
|
||||
};
|
||||
const routeFiles = Array.from(
|
||||
{ length: ROUTE_COUNT },
|
||||
(_, index) =>
|
||||
`CREATE (f:File {id: 'File:src/app/v${index}/route.ts', name: 'route.ts', filePath: 'src/app/v${index}/route.ts', content: ''})`,
|
||||
);
|
||||
const routeImports = Array.from(
|
||||
{ length: ROUTE_COUNT },
|
||||
(_, index) =>
|
||||
`MATCH (a:File {id:'File:src/app/v${index}/route.ts'}), (b:File {id:'File:src/lib/svc.ts'}) CREATE (a)-[:CodeRelation {type:'IMPORTS', confidence:1.0, reason:'import', step:0}]->(b)`,
|
||||
);
|
||||
|
||||
const SEED = [
|
||||
`CREATE (f:File {id: 'File:src/lib/svc.ts', name: 'svc.ts', filePath: 'src/lib/svc.ts', content: 'interface Entry {}\\nexport function verify() {}'})`,
|
||||
...routeFiles,
|
||||
...routeImports,
|
||||
`CREATE (:File {id: 'File:src/lib/dependency.ts', name: 'dependency.ts', filePath: 'src/lib/dependency.ts', content: ''})`,
|
||||
`MATCH (a:File {id:'File:src/lib/svc.ts'}), (b:File {id:'File:src/lib/dependency.ts'}) CREATE (a)-[:CodeRelation {type:'IMPORTS', confidence:1.0, reason:'import', step:0}]->(b)`,
|
||||
// Module-private: a recorded `false` verdict.
|
||||
`CREATE (i:Interface {id: 'Interface:src/lib/svc.ts:Entry', name: 'Entry', filePath: 'src/lib/svc.ts', startLine: 1, endLine: 3, isExported: false, content: '', description: ''})`,
|
||||
// Exported sibling in the same file: the file seed must stay for it.
|
||||
`CREATE (i:Interface {id: 'Interface:src/lib/svc.ts:Options', name: 'Options', filePath: 'src/lib/svc.ts', startLine: 4, endLine: 6, isExported: true, content: '', description: ''})`,
|
||||
// No verdict at all (NULL): the seed stays, as before.
|
||||
`CREATE (i:Interface {id: 'Interface:src/lib/svc.ts:Legacy', name: 'Legacy', filePath: 'src/lib/svc.ts', startLine: 7, endLine: 9, content: '', description: ''})`,
|
||||
`CREATE (fn:Function {id: 'Function:src/lib/svc.ts:parse', name: 'parse', filePath: 'src/lib/svc.ts', startLine: 10, endLine: 12, isExported: false, content: '', description: ''})`,
|
||||
`CREATE (fn:Function {id: 'Function:src/lib/svc.ts:verify', name: 'verify', filePath: 'src/lib/svc.ts', startLine: 13, endLine: 15, isExported: true, content: '', description: ''})`,
|
||||
...['Entry', 'Options', 'Legacy'].map(
|
||||
(name) =>
|
||||
`MATCH (f:File {id:'File:src/lib/svc.ts'}), (i:Interface {id:'Interface:src/lib/svc.ts:${name}'}) CREATE (f)-[:CodeRelation {type:'DEFINES', confidence:1.0, reason:'', step:0}]->(i)`,
|
||||
),
|
||||
`MATCH (a:Function {id:'Function:src/lib/svc.ts:parse'}), (b:Interface {id:'Interface:src/lib/svc.ts:Entry'}) CREATE (a)-[:CodeRelation {type:'USES', confidence:1.0, reason:'type-reference', step:0}]->(b)`,
|
||||
`MATCH (a:Function {id:'Function:src/lib/svc.ts:verify'}), (b:Function {id:'Function:src/lib/svc.ts:parse'}) CREATE (a)-[:CodeRelation {type:'CALLS', confidence:1.0, reason:'direct', step:0}]->(b)`,
|
||||
// Sibling path: `false` that is NOT file-private. A Java package-private
|
||||
// class and a Kotlin `internal` class are both recorded `isExported: false`
|
||||
// and both stay nameable from the other files of their package / module, so
|
||||
// the file seed must survive for them.
|
||||
// Each file also declares a public class, so the file has a `true` row the
|
||||
// way an ES module does: only the provider's verdict keeps the seed here.
|
||||
...packageScopedCase('java', 'src/main/java/app/Svc.java', 'PackageEntry', 'JavaApi'),
|
||||
...packageScopedCase('kotlin', 'src/main/kotlin/app/Svc.kt', 'InternalEntry', 'KotlinApi'),
|
||||
// Same `false`, same provider flag, but the file's export surface is not an
|
||||
// ES one: a CommonJS module (`module.exports = Widget`) and a global script
|
||||
// record every declaration `false` and never a `true`. Both stay nameable
|
||||
// from other files, so the seed must survive for them.
|
||||
...packageScopedCase('cjs', 'src/legacy/widget.js', 'Widget'),
|
||||
...packageScopedCase('script', 'src/global/registry.ts', 'Registry'),
|
||||
];
|
||||
|
||||
function siblingFiles(lang: string): string[] {
|
||||
return Array.from(
|
||||
{ length: SIBLING_COUNT },
|
||||
(_, index) => `src/use/${lang}/Use${index}.${SIBLING_EXTENSION[lang]}`,
|
||||
);
|
||||
}
|
||||
|
||||
function packageScopedCase(
|
||||
lang: string,
|
||||
ownerPath: string,
|
||||
className: string,
|
||||
exportedSibling?: string,
|
||||
): string[] {
|
||||
const ownerId = `File:${ownerPath}`;
|
||||
const classId = `Class:${ownerPath}:${className}`;
|
||||
const siblingId = `Class:${ownerPath}:${exportedSibling}`;
|
||||
return [
|
||||
`CREATE (f:File {id: '${ownerId}', name: '${ownerPath.slice(ownerPath.lastIndexOf('/') + 1)}', filePath: '${ownerPath}', content: ''})`,
|
||||
`CREATE (c:Class {id: '${classId}', name: '${className}', filePath: '${ownerPath}', startLine: 1, endLine: 3, isExported: false, content: '', description: ''})`,
|
||||
`MATCH (f:File {id:'${ownerId}'}), (c:Class {id:'${classId}'}) CREATE (f)-[:CodeRelation {type:'DEFINES', confidence:1.0, reason:'', step:0}]->(c)`,
|
||||
...(exportedSibling === undefined
|
||||
? []
|
||||
: [
|
||||
`CREATE (c:Class {id: '${siblingId}', name: '${exportedSibling}', filePath: '${ownerPath}', startLine: 4, endLine: 6, isExported: true, content: '', description: ''})`,
|
||||
`MATCH (f:File {id:'${ownerId}'}), (c:Class {id:'${siblingId}'}) CREATE (f)-[:CodeRelation {type:'DEFINES', confidence:1.0, reason:'', step:0}]->(c)`,
|
||||
]),
|
||||
...siblingFiles(lang).flatMap((filePath) => [
|
||||
`CREATE (f:File {id: 'File:${filePath}', name: '${filePath.slice(filePath.lastIndexOf('/') + 1)}', filePath: '${filePath}', content: ''})`,
|
||||
`MATCH (a:File {id:'File:${filePath}'}), (b:File {id:'${ownerId}'}) CREATE (a)-[:CodeRelation {type:'IMPORTS', confidence:1.0, reason:'import', step:0}]->(b)`,
|
||||
]),
|
||||
];
|
||||
}
|
||||
|
||||
type BackendHandle = IndexedDBHandle & { _backend?: LocalBackend };
|
||||
|
||||
type ImpactByDepth = { byDepth?: Record<number, Array<{ id?: unknown }>> };
|
||||
|
||||
const ids = (result: ImpactByDepth, depth: number): string[] =>
|
||||
(result.byDepth?.[depth] ?? []).map((item) => String(item.id)).sort();
|
||||
|
||||
withTestLbugDB(
|
||||
'impact-private-type-file-seed',
|
||||
(handle) => {
|
||||
let backend: LocalBackend;
|
||||
beforeAll(() => {
|
||||
const ext = handle as BackendHandle;
|
||||
if (!ext._backend) throw new Error('LocalBackend not initialized');
|
||||
backend = ext._backend;
|
||||
});
|
||||
|
||||
it('does not report the importers of the file as direct dependants of a module-private interface', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Entry',
|
||||
kind: 'Interface',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.target.id).toBe('Interface:src/lib/svc.ts:Entry');
|
||||
expect(ids(result, 1)).toEqual(['Function:src/lib/svc.ts:parse']);
|
||||
expect(ids(result, 2)).toEqual(['Function:src/lib/svc.ts:verify']);
|
||||
expect(result.impactedCount).toBe(2);
|
||||
expect(result.summary.direct).toBe(1);
|
||||
expect(result.risk).toBe('LOW');
|
||||
});
|
||||
|
||||
it('keeps the file seed for an exported interface of the same file', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Options',
|
||||
kind: 'Interface',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(ids(result, 1)).toEqual(
|
||||
Array.from({ length: ROUTE_COUNT }, (_, index) => `File:src/app/v${index}/route.ts`).sort(),
|
||||
);
|
||||
expect(result.impactedCount).toBe(ROUTE_COUNT);
|
||||
});
|
||||
|
||||
it('preserves downstream file dependencies even for a private type', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Entry',
|
||||
kind: 'Interface',
|
||||
direction: 'downstream',
|
||||
});
|
||||
expect(ids(result, 1)).toEqual(['File:src/lib/dependency.ts']);
|
||||
});
|
||||
|
||||
it('keeps the file seed when the index recorded no export verdict', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Legacy',
|
||||
kind: 'Interface',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.impactedCount).toBe(ROUTE_COUNT);
|
||||
expect(ids(result, 1).every((id) => id.startsWith('File:src/app/'))).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps the file seed for a Java package-private class: false is not file-private there', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'PackageEntry',
|
||||
kind: 'Class',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.target.id).toBe('Class:src/main/java/app/Svc.java:PackageEntry');
|
||||
expect(ids(result, 1)).toEqual(
|
||||
siblingFiles('java')
|
||||
.map((p) => `File:${p}`)
|
||||
.sort(),
|
||||
);
|
||||
expect(result.impactedCount).toBe(SIBLING_COUNT);
|
||||
});
|
||||
|
||||
it('keeps the file seed for a Kotlin internal class: false is module-wide there', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'InternalEntry',
|
||||
kind: 'Class',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.target.id).toBe('Class:src/main/kotlin/app/Svc.kt:InternalEntry');
|
||||
expect(ids(result, 1)).toEqual(
|
||||
siblingFiles('kotlin')
|
||||
.map((p) => `File:${p}`)
|
||||
.sort(),
|
||||
);
|
||||
expect(result.impactedCount).toBe(SIBLING_COUNT);
|
||||
});
|
||||
|
||||
it('keeps the file seed for a class of a CommonJS module: no ES export in the file, so false is no verdict', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Widget',
|
||||
kind: 'Class',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.target.id).toBe('Class:src/legacy/widget.js:Widget');
|
||||
expect(ids(result, 1)).toEqual(
|
||||
siblingFiles('cjs')
|
||||
.map((p) => `File:${p}`)
|
||||
.sort(),
|
||||
);
|
||||
expect(result.impactedCount).toBe(SIBLING_COUNT);
|
||||
});
|
||||
|
||||
it('keeps the file seed for a declaration of a global TS script for the same reason', async () => {
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'Registry',
|
||||
kind: 'Class',
|
||||
direction: 'upstream',
|
||||
});
|
||||
|
||||
expect(result.target.id).toBe('Class:src/global/registry.ts:Registry');
|
||||
expect(ids(result, 1)).toEqual(
|
||||
siblingFiles('script')
|
||||
.map((p) => `File:${p}`)
|
||||
.sort(),
|
||||
);
|
||||
expect(result.impactedCount).toBe(SIBLING_COUNT);
|
||||
});
|
||||
},
|
||||
{
|
||||
seed: SEED,
|
||||
poolAdapter: true,
|
||||
afterSetup: async (handle) => {
|
||||
vi.mocked(listRegisteredRepos).mockResolvedValue([
|
||||
{
|
||||
name: 'test-repo',
|
||||
path: '/test/repo',
|
||||
storagePath: handle.tmpHandle.dbPath,
|
||||
indexedAt: new Date().toISOString(),
|
||||
lastCommit: 'abc123',
|
||||
stats: { files: ROUTE_COUNT + 1, nodes: ROUTE_COUNT + 6, communities: 0, processes: 0 },
|
||||
},
|
||||
]);
|
||||
const backend = new LocalBackend();
|
||||
await backend.init();
|
||||
(handle as BackendHandle)._backend = backend;
|
||||
},
|
||||
},
|
||||
);
|
||||
142
gitnexus/test/integration/impact-private-type-source.test.ts
Normal file
142
gitnexus/test/integration/impact-private-type-source.test.ts
Normal file
|
|
@ -0,0 +1,142 @@
|
|||
import fs from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
import { expect, it, vi } from 'vitest';
|
||||
import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js';
|
||||
import { LocalBackend } from '../../src/mcp/local/local-backend.js';
|
||||
import { listRegisteredRepos } from '../../src/storage/repo-manager.js';
|
||||
import { withTestLbugDB } from '../helpers/test-indexed-db.js';
|
||||
|
||||
vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../../src/storage/repo-manager.js')>()),
|
||||
listRegisteredRepos: vi.fn().mockResolvedValue([]),
|
||||
cleanupOldKuzuFiles: vi.fn().mockResolvedValue({ found: false, needsReindex: false }),
|
||||
findSiblingClones: vi.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
const sources: Record<string, string> = {
|
||||
'clause.ts': 'interface ClauseEntry { value: string }\nexport { ClauseEntry };\n',
|
||||
'commented.ts':
|
||||
'interface CommentedEntry {}\nexport function verifyCommented() {}\n// exported separately\nexport { CommentedEntry };\n',
|
||||
'commented-use.ts':
|
||||
"import type { CommentedEntry } from './commented';\nexport type Alias = CommentedEntry;\n",
|
||||
'clause-use.ts':
|
||||
"import type { ClauseEntry } from './clause';\nexport function consume(x: ClauseEntry) { return x.value; }\n",
|
||||
'default.ts': 'class DefaultEntry {}\nexport default DefaultEntry;\n',
|
||||
'default-use.ts': "import Entry from './default';\nexport { Entry };\n",
|
||||
'type-alias.ts':
|
||||
'interface AliasedEntry { value: string }\nexport type PublicEntry = AliasedEntry;\n',
|
||||
'type-alias-use.ts':
|
||||
"import type { PublicEntry } from './type-alias';\nexport function consumeAlias(x: PublicEntry) { return x.value; }\n",
|
||||
'wrapped-alias.ts':
|
||||
"class AliasedClass { value = ''; }\nexport type PublicClass = Readonly<AliasedClass>;\n",
|
||||
'wrapped-alias-use.ts':
|
||||
"import type { PublicClass } from './wrapped-alias';\nexport function consumeWrapped(x: PublicClass) { return x.value; }\n",
|
||||
'value-alias.js':
|
||||
'class ValueAliasedClass {}\nexport const PublishedClass = ValueAliasedClass;\n',
|
||||
'value-alias-use.js':
|
||||
"import { PublishedClass } from './value-alias';\nexport function constructAlias() { return new PublishedClass(); }\n",
|
||||
'cjs.js': 'class CjsEntry {}\nmodule.exports = CjsEntry;\n',
|
||||
'cjs-use.js': "const Entry = require('./cjs');\nmodule.exports = Entry;\n",
|
||||
'ambient.d.ts': 'interface AmbientEntry { value: string }\n',
|
||||
'global.ts': "import './side';\ndeclare global { interface GlobalEntry { value: string } }\n",
|
||||
'side.ts': 'export const side = true;\n',
|
||||
'global-use.ts':
|
||||
"import './global';\nexport function consumeGlobal(x: GlobalEntry) { return x.value; }\n",
|
||||
'ambient-use.ts':
|
||||
"import type { AmbientEntry } from './ambient';\nexport function consumeAmbient(x: AmbientEntry) { return x.value; }\n",
|
||||
'private.ts':
|
||||
'interface PrivateEntry { value: string }\nexport function verify() { return true; }\n',
|
||||
'private-use.ts':
|
||||
"import { verify } from './private';\nexport function check() { return verify(); }\n",
|
||||
'private.js': 'class PrivateClass {}\nexport function verifyClass() { return true; }\n',
|
||||
'private-js-use.js':
|
||||
"import { verifyClass } from './private.js';\nexport function checkClass() { return verifyClass(); }\n",
|
||||
};
|
||||
|
||||
let backend: LocalBackend;
|
||||
let repoPath: string;
|
||||
const exportFlags = new Map<string, boolean | undefined>();
|
||||
|
||||
withTestLbugDB(
|
||||
'impact-private-type-source',
|
||||
() => {
|
||||
it.each([
|
||||
['ClauseEntry', 'clause-use.ts'],
|
||||
['CommentedEntry', 'commented-use.ts'],
|
||||
['DefaultEntry', 'default-use.ts'],
|
||||
['AliasedEntry', 'type-alias-use.ts'],
|
||||
['AliasedClass', 'wrapped-alias-use.ts'],
|
||||
['CjsEntry', 'cjs-use.js'],
|
||||
['AmbientEntry', 'ambient-use.ts'],
|
||||
['GlobalEntry', 'global-use.ts'],
|
||||
])('retains importers for %s despite the parser storing false', async (target, importer) => {
|
||||
expect(exportFlags.get(target)).toBe(false);
|
||||
const result = await backend.callTool('impact', { target, direction: 'upstream' });
|
||||
expect(
|
||||
(result.byDepth[1] ?? [])
|
||||
.filter((item: { id: string }) => item.id.startsWith('File:'))
|
||||
.map((item: { id: string }) => item.id),
|
||||
).toEqual([`File:${importer}`]);
|
||||
});
|
||||
|
||||
it('retains importers of a class exposed through an exported value alias', async () => {
|
||||
expect(exportFlags.get('ValueAliasedClass')).toBe(false);
|
||||
const result = await backend.callTool('impact', {
|
||||
target: 'ValueAliasedClass',
|
||||
direction: 'upstream',
|
||||
});
|
||||
expect(
|
||||
Object.values(result.byDepth as Record<string, { id: string }[]>)
|
||||
.flat()
|
||||
.map((item) => item.id),
|
||||
).toContain('File:value-alias-use.js');
|
||||
});
|
||||
|
||||
it.each(['PrivateEntry', 'PrivateClass'])(
|
||||
'does not borrow file importers for %s',
|
||||
async (target) => {
|
||||
expect(exportFlags.get(target)).toBe(false);
|
||||
const result = await backend.callTool('impact', { target, direction: 'upstream' });
|
||||
expect(result.impactedCount).toBe(0);
|
||||
expect(result.risk).toBe('UNKNOWN');
|
||||
},
|
||||
);
|
||||
},
|
||||
{
|
||||
poolAdapter: true,
|
||||
beforeFTS: async (dbPath) => {
|
||||
const storageDir = path.dirname(dbPath);
|
||||
repoPath = path.join(storageDir, 'source');
|
||||
await fs.mkdir(repoPath);
|
||||
await Promise.all(
|
||||
Object.entries(sources).map(([name, content]) =>
|
||||
fs.writeFile(path.join(repoPath, name), content),
|
||||
),
|
||||
);
|
||||
const result = await runPipelineFromRepo(repoPath, () => {}, {
|
||||
skipGraphPhases: true,
|
||||
workerPoolSize: 1,
|
||||
});
|
||||
for (const node of result.graph.nodes) {
|
||||
if (node.label === 'Class' || node.label === 'Interface') {
|
||||
exportFlags.set(node.properties.name, node.properties.isExported);
|
||||
}
|
||||
}
|
||||
const adapter = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
await adapter.loadGraphToLbug(result.graph, repoPath, storageDir);
|
||||
},
|
||||
afterSetup: async (handle) => {
|
||||
vi.mocked(listRegisteredRepos).mockResolvedValue([
|
||||
{
|
||||
name: 'private-source',
|
||||
path: repoPath,
|
||||
storagePath: handle.tmpHandle.dbPath,
|
||||
indexedAt: new Date().toISOString(),
|
||||
lastCommit: 'abc123',
|
||||
},
|
||||
]);
|
||||
backend = new LocalBackend();
|
||||
await backend.init();
|
||||
},
|
||||
},
|
||||
);
|
||||
53
gitnexus/test/unit/file-private-exports.test.ts
Normal file
53
gitnexus/test/unit/file-private-exports.test.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import { expect, it } from 'vitest';
|
||||
import { hasFilePrivateExportSemantics } from '../../src/core/ingestion/languages/typescript/file-private-exports.js';
|
||||
|
||||
it.each([
|
||||
['plain module', 'entry.ts', 'interface Entry {}\nexport function verify() {}', true],
|
||||
['JS module', 'entry.js', 'class Entry {}\nexport function verify() {}', true],
|
||||
['TSX module', 'entry.tsx', 'interface Entry {}\nexport const View = () => <div/>;', true],
|
||||
['JSX module', 'entry.jsx', 'class Entry {}\nexport const View = () => <div/>;', true],
|
||||
['named export', 'entry.ts', 'interface Entry {}\nexport { Entry };', false],
|
||||
['renamed export', 'entry.ts', 'interface Entry {}\nexport { Entry as Public };', false],
|
||||
['default export', 'entry.ts', 'class Entry {}\nexport default Entry;', false],
|
||||
['assignment export', 'entry.ts', 'class Entry {}\nexport = Entry;', false],
|
||||
['type alias', 'entry.ts', 'interface Entry {}\nexport type Public = Entry;', false],
|
||||
[
|
||||
'wrapped type alias',
|
||||
'entry.ts',
|
||||
'class Entry {}\nexport type Public = Readonly<Entry>;',
|
||||
false,
|
||||
],
|
||||
['CommonJS', 'entry.js', 'class Entry {}\nmodule.exports = Entry;', false],
|
||||
['bracket CommonJS', 'entry.js', "class Entry {}\nmodule['exports'] = Entry;", false],
|
||||
[
|
||||
'mixed CommonJS',
|
||||
'entry.js',
|
||||
'class Entry {}\nexports.Entry = Entry;\nexport const n = 1;',
|
||||
false,
|
||||
],
|
||||
['ambient', 'entry.d.ts', 'interface Entry {}\nexport const n: number;', false],
|
||||
['ambient ESM', 'entry.d.mts', 'interface Entry {}\nexport const n: number;', false],
|
||||
['script global', 'entry.ts', 'interface Entry {}', false],
|
||||
[
|
||||
'global augmentation',
|
||||
'entry.ts',
|
||||
"import './side'; declare global { interface Entry {} }",
|
||||
false,
|
||||
],
|
||||
['missing retained source', 'entry.ts', '', false],
|
||||
[
|
||||
'normalized line comment',
|
||||
'entry.ts',
|
||||
'interface Entry {} export function verify() {} // comment export { Entry };',
|
||||
false,
|
||||
],
|
||||
[
|
||||
'truncated source',
|
||||
'entry.ts',
|
||||
'interface Entry {} export function verify() {} /* [truncated] */',
|
||||
false,
|
||||
],
|
||||
['invalid source', 'entry.ts', 'interface Entry {\nexport const', false],
|
||||
])('%s has conservative file privacy', (_label, filePath, content, expected) => {
|
||||
expect(hasFilePrivateExportSemantics(filePath, content)).toBe(expected);
|
||||
});
|
||||
|
|
@ -79,6 +79,8 @@ export default defineConfig({
|
|||
'test/integration/pdg-explicit-owner.test.ts',
|
||||
'test/integration/skip-fts.test.ts',
|
||||
'test/integration/impact-callable-value-references.test.ts',
|
||||
'test/integration/impact-private-type-file-seed.test.ts',
|
||||
'test/integration/impact-private-type-source.test.ts',
|
||||
'test/integration/impact-context-integrity.test.ts',
|
||||
'test/integration/incremental-worktree-context.test.ts',
|
||||
'test/integration/incremental-method-integrity.test.ts',
|
||||
|
|
@ -188,6 +190,8 @@ export default defineConfig({
|
|||
'test/integration/pdg-explicit-owner.test.ts',
|
||||
'test/integration/skip-fts.test.ts',
|
||||
'test/integration/impact-callable-value-references.test.ts',
|
||||
'test/integration/impact-private-type-file-seed.test.ts',
|
||||
'test/integration/impact-private-type-source.test.ts',
|
||||
'test/integration/impact-context-integrity.test.ts',
|
||||
'test/integration/incremental-worktree-context.test.ts',
|
||||
'test/integration/incremental-method-integrity.test.ts',
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue