diff --git a/docs/languages/r-provider.md b/docs/languages/r-provider.md index 757650a31..2e69d79e7 100644 --- a/docs/languages/r-provider.md +++ b/docs/languages/r-provider.md @@ -49,7 +49,7 @@ Exports: The scope query (`languages/r/query.ts`) captures three call shapes only: `f(x)`, `pkg::f(x)` and `obj$f(x)`. -- A bare call binds lexically first. `library()` and `require()` of a local package make every top-level name of that package visible, unfiltered by NAMESPACE. This is wider than R's `library()` of an installed package, which attaches only its exports; it follows `devtools::load_all()`, which attaches every top-level name, exported or not. `source()` binds no name; a call into a sourced file, or into another file of the same package, binds through a workspace-wide unique-name fallback at confidence 0.5 (`global-name-fallback`). An ambiguous name yields no edge. +- A bare call binds lexically first. `library()` and `require()` of a local package make every top-level name of that package visible, unfiltered by NAMESPACE. This is wider than R's `library()` of an installed package, which attaches only its exports; it follows `devtools::load_all()`, which attaches every top-level name, exported or not. `source()` binds no name; a call into a sourced file, or into another file of the same package, binds through a workspace-wide unique-name fallback at confidence 0.5 (`global-name-fallback`). An ambiguous name yields no edge, and a function defined inside another function is never a candidate (no other function can reach it by name). - A bare call never binds to a dotted definition, so `foo(x)` cannot reach `print.foo`. Dotted (S3-style) names are never import-resolved; a call that spells the full dotted name binds by qualified-name lookup. Dotted names are not bound through `importFrom()`. - `pkg::name()` is decided by its qualifier: - `pkg` is a package of the repository and defines `name` exactly once: the call binds to that definition at confidence 0.85 (`import-resolved`). diff --git a/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts b/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts index 609619598..c011749c8 100644 --- a/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts +++ b/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts @@ -650,6 +650,39 @@ export function rFileTopLevel(parsed: ParsedFile): RFileTopLevel { return result; } +interface RDefIds { + /** Ids of the function and class definitions the file binds at top level. */ + readonly topLevel: ReadonlySet; + /** Ids of every definition the file's scope model holds, nested or not. */ + readonly all: ReadonlySet; +} + +const defIdsCache = new WeakMap(); + +/** + * The function and class definitions of `parsed` that are bound at top level: the `local` + * bindings of its Module scope. A function written inside another function is owned by that + * function's scope (and `localDefs` lists it all the same), so its id is in `all` only. + */ +function rDefIds(parsed: ParsedFile): RDefIds { + const cached = defIdsCache.get(parsed); + if (cached !== undefined) return cached; + const topLevel = new Set(); + const moduleScope = parsed.scopes.find((s) => s.id === parsed.moduleScope); + if (moduleScope !== undefined) { + for (const refs of moduleScope.bindings.values()) { + for (const ref of refs) { + if (ref.origin === 'local' && TOP_LEVEL_BINDABLE_TYPES.has(ref.def.type)) { + topLevel.add(ref.def.nodeId); + } + } + } + } + const result = { topLevel, all: new Set(parsed.localDefs.map((d) => d.nodeId)) }; + defIdsCache.set(parsed, result); + return result; +} + // ─── Import synthesis (populateWorkspaceReferences) ───────────────────────── /** @@ -880,15 +913,38 @@ export function rQualifiedDefinitionCount( */ export interface RGlobalNameFallbackContext { readonly callerParsed: { readonly filePath: string }; - readonly candidate: { readonly filePath: string }; + readonly candidate: { + readonly filePath: string; + readonly nodeId?: string; + readonly type?: string; + }; + /** Reaches the candidate's parse; absent in hand-built contexts, which read as "cannot decide". */ + readonly parsedFileOf?: (filePath: string) => ParsedFile | undefined; /** Opaque `loadResolutionConfig` result; an {@link RPackageConfig} for R. */ readonly resolutionConfig?: unknown; readonly site: { readonly name: string; readonly rawQualifiedName?: string }; } /** - * Refuse a `global-name-fallback` guess that the caller package's NAMESPACE - * contradicts. Returns `false` (veto) only when ALL of these hold: + * True when the candidate is a function or class that its file's scope model holds and binds + * below the top level. A candidate the scope model does not hold (the semantic model's own + * entry for a dotted name carries a graph id, not a scope definition id) is not judged. + */ +function isClosureLocalCandidate(ctx: RGlobalNameFallbackContext): boolean { + const { candidate } = ctx; + if (candidate.nodeId === undefined || candidate.type === undefined) return false; + if (!TOP_LEVEL_BINDABLE_TYPES.has(candidate.type)) return false; + const parsed = ctx.parsedFileOf?.(candidate.filePath); + if (parsed === undefined) return false; + const ids = rDefIds(parsed); + return ids.all.has(candidate.nodeId) && !ids.topLevel.has(candidate.nodeId); +} + +/** + * Refuse a `global-name-fallback` guess that R cannot make. Returns `false` (veto) when the + * candidate is a function or class bound inside another function (not at top level), or when + * the caller package's NAMESPACE contradicts the guess. The NAMESPACE veto applies only when + * ALL of these hold: * * - the caller file lives under `/R/**` of a package with a NAMESPACE * that has an `importFrom(P, )` entry (the LAST such entry over @@ -901,7 +957,7 @@ export interface RGlobalNameFallbackContext { * re-exports the name from a third package and the candidate may be the * real origin (unknown per-package names = cannot decide = allow). * - * Everything else — no config, caller outside a package `R/` (`tests/`, + * Everything else — no config, no parse of the candidate's file, caller outside a package `R/` (`tests/`, * `scripts/`, root `plumber.R`), no entry, candidate outside any package * `R/` — answers `true`, so `source()`/`library()` script flows are untouched. * @@ -916,6 +972,11 @@ export interface RGlobalNameFallbackContext { * tokenised, so a later `import(pkg)` that R would prefer is invisible. */ export function isRGlobalNameFallbackPlausible(ctx: RGlobalNameFallbackContext): boolean { + // A function or class bound inside another function lives in that call's frame: no other + // function, in this file or another, can reach it by its bare name. Only top-level + // definitions are candidates for a unique-name guess. + if (isClosureLocalCandidate(ctx)) return false; + const cfg = ctx.resolutionConfig as RPackageConfig | null | undefined; if ( typeof cfg !== 'object' || diff --git a/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/a.R b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/a.R new file mode 100644 index 000000000..5d95cbc3e --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/a.R @@ -0,0 +1,6 @@ +make_counter <- function() { + count_fn <- function() 2 + count_fn +} + +use_local <- function() count_fn() diff --git a/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/b.R b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/b.R new file mode 100644 index 000000000..5fda25cd8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/b.R @@ -0,0 +1,8 @@ +use_other <- function() count_fn() + +use_top <- function() top_helper() + +wrap <- function() { + shared_name <- function() 2 + shared_name() +} diff --git a/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/c.R b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/c.R new file mode 100644 index 000000000..93eaec6ba --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/r-closure-local-fallback/R/c.R @@ -0,0 +1,9 @@ +top_helper <- function() 1 + +other <- function() { + inner_only <- function() top_helper() + inner_only() +} + +reach_inner <- function() inner_only() +reach_top <- function() top_helper() diff --git a/gitnexus/test/integration/resolvers/r-closure-local-fallback.test.ts b/gitnexus/test/integration/resolvers/r-closure-local-fallback.test.ts new file mode 100644 index 000000000..595effe45 --- /dev/null +++ b/gitnexus/test/integration/resolvers/r-closure-local-fallback.test.ts @@ -0,0 +1,44 @@ +/** + * R: a function bound inside another function lives in that call's frame, so no other function + * (in its file or another) can call it by its bare name. The unique-name fallback used to bind + * such calls to it at 0.5 whenever the name was unique in the repository. + */ +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import path from 'path'; +import { FIXTURES, getRelationships, runPipelineFromRepo } from './helpers.js'; +import type { PipelineResult } from './helpers.js'; + +// Safe under `pool: 'forks'`: the env stub is process-global, but each file gets its own process. +beforeAll(() => vi.stubEnv('GITNEXUS_WORKER_READY_TIMEOUT_MS', '60000')); +afterAll(() => vi.unstubAllEnvs()); + +describe('R unique-name fallback and closure-local functions', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'r-closure-local-fallback'), () => {}); + }, 60000); + + /** `caller -> callee (reason)` for every CALLS edge, sorted. */ + const calls = (): string[] => + getRelationships(result, 'CALLS') + .map((e) => `${e.source}->${e.target}`) + .sort(); + + it('does not bind a bare call to a closure-local function of another function', () => { + expect(calls()).not.toContain('use_local->count_fn'); + expect(calls()).not.toContain('use_other->count_fn'); + expect(calls()).not.toContain('reach_inner->inner_only'); + }); + + it('keeps the unique-name guess to a top-level function in another file (positive control)', () => { + expect(calls()).toContain('use_top->top_helper'); + expect(calls()).toContain('reach_top->top_helper'); + }); + + it('keeps the lexical edges inside a function that defines the closure itself', () => { + expect(calls()).toContain('wrap->shared_name'); + expect(calls()).toContain('inner_only->top_helper'); + expect(calls()).toContain('other->inner_only'); + }); +}); diff --git a/gitnexus/test/unit/scope-resolution/r/r-closure-local-veto.test.ts b/gitnexus/test/unit/scope-resolution/r/r-closure-local-veto.test.ts new file mode 100644 index 000000000..bba75764b --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/r/r-closure-local-veto.test.ts @@ -0,0 +1,75 @@ +/** + * The R unique-name fallback veto refuses a candidate bound inside another function, and only that: + * a top-level definition, a candidate it cannot inspect, and every non-function kind stay plausible. + */ +import { describe, expect, it } from 'vitest'; +import type { ParsedFile, SymbolDefinition } from 'gitnexus-shared'; +import { extractParsedFile } from '../../../../src/core/ingestion/scope-extractor-bridge.js'; +import { rProvider } from '../../../../src/core/ingestion/languages/r.js'; +import { isRGlobalNameFallbackPlausible } from '../../../../src/core/ingestion/languages/r/namespace-imports.js'; + +const SOURCE = [ + 'make_counter <- function() {', + ' inner_fn <- function() 2', + ' Inner <- setRefClass("Inner", fields = list(a = "numeric"))', + ' inner_fn', + '}', + 'top_fn <- function() 1', +].join('\n'); + +const parsed = (): ParsedFile => { + const file = extractParsedFile(rProvider, SOURCE, 'R/a.R'); + if (file === undefined) throw new Error('no ParsedFile'); + return file; +}; + +const defNamed = (file: ParsedFile, name: string): SymbolDefinition => { + const def = file.localDefs.find((d) => d.qualifiedName?.split('.').pop() === name); + if (def === undefined) throw new Error(`no def ${name}`); + return def; +}; + +const verdict = ( + file: ParsedFile, + candidate: Pick, + withParse = true, +): boolean => + isRGlobalNameFallbackPlausible({ + callerParsed: { filePath: 'R/b.R' }, + candidate, + parsedFileOf: withParse ? (p) => (p === file.filePath ? file : undefined) : undefined, + site: { name: 'x' }, + }); + +describe('isRGlobalNameFallbackPlausible: closure-local candidates', () => { + it('refuses a function bound inside another function', () => { + const file = parsed(); + expect(verdict(file, defNamed(file, 'inner_fn'))).toBe(false); + expect(verdict(file, defNamed(file, 'Inner'))).toBe(false); + }); + + it('keeps a top-level function', () => { + const file = parsed(); + expect(verdict(file, defNamed(file, 'top_fn'))).toBe(true); + expect(verdict(file, defNamed(file, 'make_counter'))).toBe(true); + }); + + it('does not judge a candidate the scope model does not hold (a graph node id)', () => { + const file = parsed(); + expect( + verdict(file, { nodeId: 'Function:R/a.R:inner_fn', filePath: 'R/a.R', type: 'Function' }), + ).toBe(true); + }); + + it('cannot decide without the candidate parse, and allows', () => { + const file = parsed(); + expect(verdict(file, defNamed(file, 'inner_fn'), false)).toBe(true); + expect( + isRGlobalNameFallbackPlausible({ + callerParsed: { filePath: 'R/b.R' }, + candidate: { filePath: 'R/a.R' }, + site: { name: 'x' }, + }), + ).toBe(true); + }); +});