mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-11-01 07:02:35 +00:00
fix(r): let the package attached last mask a name two library() calls define
Two library() calls that attach packages defining the same name bound the call to whichever package file sorted first by id, so a script that attached lp2 after lp reached lp's function while R reaches lp2's. rMergeBindings now keeps, within the wildcard tier, only the bindings of the library() call that comes last in the file. A package of several files that defines the name twice stays ambiguous, and a binding with no call position leaves the tier as it was.
This commit is contained in:
parent
d3e06d373d
commit
32fb2973d8
10 changed files with 163 additions and 7 deletions
|
|
@ -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, and a function defined inside another function is never a candidate (no other function can reach it by name).
|
||||
- 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. When two attached packages define the same name, the package attached last wins, as R masks it: of the `library()` calls whose package defines the name, the one that comes last in the file decides (calls inside a function are not told apart from top-level ones, and a package named by several calls counts at its last one). Two files of that one package that both define the name stay ambiguous, with no edge. 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`).
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { BindingRef, Callsite, SymbolDefinition } from 'gitnexus-shared';
|
||||
import type { BindingRef, Callsite, Range, SymbolDefinition } from 'gitnexus-shared';
|
||||
|
||||
const TIER: Record<BindingRef['origin'], number> = {
|
||||
local: 0,
|
||||
|
|
@ -8,9 +8,25 @@ const TIER: Record<BindingRef['origin'], number> = {
|
|||
wildcard: 4,
|
||||
};
|
||||
|
||||
/** Local declarations shadow imports; deterministic order within a tier.
|
||||
* Same shape as Zig's `zigMergeBindings` — R has no per-scope binding
|
||||
* precedence rule beyond "the tightest-origin binding wins". */
|
||||
/** `a` before `b` by the line, then the column, at which each starts. */
|
||||
const startsBefore = (a: Range, b: Range): boolean =>
|
||||
a.startLine !== b.startLine ? a.startLine < b.startLine : a.startCol < b.startCol;
|
||||
|
||||
/**
|
||||
* Local declarations shadow imports; deterministic order within a tier.
|
||||
* Same shape as Zig's `zigMergeBindings`.
|
||||
*
|
||||
* Within the wildcard tier (`library()` / `require()` of a local package) a name that several
|
||||
* attached packages define is masked as R masks it: the package attached last wins, so only the
|
||||
* bindings of the `library()` call that comes last in the file are kept. A call that attaches a
|
||||
* package of several files keeps all of that package's bindings for the name, because a name
|
||||
* defined in two of its files is a real ambiguity the order cannot settle. The order is the
|
||||
* position of the call in the file, which is the order the file attaches them in when it runs
|
||||
* top to bottom; a `library()` inside a function, run later or never, is not told apart, and a
|
||||
* package named by several calls counts at its last one (R would keep it where it was first
|
||||
* attached). When any of the bindings has no call position the whole tier is kept, as it was
|
||||
* before.
|
||||
*/
|
||||
export function rMergeBindings(
|
||||
existing: readonly BindingRef[],
|
||||
incoming: readonly BindingRef[],
|
||||
|
|
@ -28,12 +44,28 @@ export function rMergeBindings(
|
|||
if ((TIER[b.origin] ?? 99) !== bestTier) continue;
|
||||
if (!byNode.has(b.def.nodeId)) byNode.set(b.def.nodeId, b);
|
||||
}
|
||||
let tier = [...byNode.values()];
|
||||
if (bestTier === TIER.wildcard && tier.length > 1) tier = lastAttached(tier);
|
||||
// By code unit, not `localeCompare`: the order must not depend on the machine's locale.
|
||||
return [...byNode.values()].sort((a, b) =>
|
||||
return tier.sort((a, b) =>
|
||||
a.def.nodeId < b.def.nodeId ? -1 : a.def.nodeId > b.def.nodeId ? 1 : 0,
|
||||
);
|
||||
}
|
||||
|
||||
/** The bindings that the last of the `library()` calls behind `bindings` brought in. */
|
||||
function lastAttached(bindings: BindingRef[]): BindingRef[] {
|
||||
const at: Range[] = [];
|
||||
for (const b of bindings) {
|
||||
if (b.via?.atRange === undefined) return bindings;
|
||||
at.push(b.via.atRange);
|
||||
}
|
||||
let last = at[0];
|
||||
for (const range of at) if (startsBefore(last, range)) last = range;
|
||||
return bindings.filter(
|
||||
(_, i) => at[i].startLine === last.startLine && at[i].startCol === last.startCol,
|
||||
);
|
||||
}
|
||||
|
||||
/** R declarations carry no synthesized arity metadata (no `...`/default-arg
|
||||
* capture exists yet), so the comparison is always 'unknown' — a real bounds
|
||||
* check that turns on if arity captures are added later. Same rationale as
|
||||
|
|
|
|||
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp/DESCRIPTION
vendored
Normal file
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp/DESCRIPTION
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
Package: lp
|
||||
Version: 1.0
|
||||
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp/R/f.R
vendored
Normal file
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp/R/f.R
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
dup_fn <- function() 1
|
||||
only_lp <- function() 1
|
||||
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp2/DESCRIPTION
vendored
Normal file
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp2/DESCRIPTION
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
Package: lp2
|
||||
Version: 1.0
|
||||
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp2/R/f.R
vendored
Normal file
2
gitnexus/test/fixtures/lang-resolution/r-library-masking/lp2/R/f.R
vendored
Normal file
|
|
@ -0,0 +1,2 @@
|
|||
dup_fn <- function() 2
|
||||
only_lp2 <- function() 2
|
||||
5
gitnexus/test/fixtures/lang-resolution/r-library-masking/scripts/a.R
vendored
Normal file
5
gitnexus/test/fixtures/lang-resolution/r-library-masking/scripts/a.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
library(lp)
|
||||
library(lp2)
|
||||
|
||||
run_a <- function() dup_fn()
|
||||
keep_a <- function() only_lp()
|
||||
5
gitnexus/test/fixtures/lang-resolution/r-library-masking/scripts/b.R
vendored
Normal file
5
gitnexus/test/fixtures/lang-resolution/r-library-masking/scripts/b.R
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
library(lp2)
|
||||
library(lp)
|
||||
|
||||
run_b <- function() dup_fn()
|
||||
keep_b <- function() only_lp2()
|
||||
|
|
@ -0,0 +1,38 @@
|
|||
/**
|
||||
* R: two `library()` calls that attach packages defining the same name. R masks with the package
|
||||
* attached last, so the call reaches that one whichever way the file names sort; the binding used
|
||||
* to be the first package file in id order.
|
||||
*/
|
||||
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 library() masking', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(path.join(FIXTURES, 'r-library-masking'), () => {});
|
||||
}, 60000);
|
||||
|
||||
/** `target file` of every CALLS edge from `source`. */
|
||||
const targets = (source: string): string[] =>
|
||||
getRelationships(result, 'CALLS')
|
||||
.filter((e) => e.source === source)
|
||||
.map((e) => e.targetFilePath)
|
||||
.sort();
|
||||
|
||||
it('binds the name to the package attached last', () => {
|
||||
expect(targets('run_a')).toEqual(['lp2/R/f.R']);
|
||||
expect(targets('run_b')).toEqual(['lp/R/f.R']);
|
||||
});
|
||||
|
||||
it('still binds a name that only one attached package defines', () => {
|
||||
expect(targets('keep_a')).toEqual(['lp/R/f.R']);
|
||||
expect(targets('keep_b')).toEqual(['lp2/R/f.R']);
|
||||
});
|
||||
});
|
||||
|
|
@ -4,7 +4,7 @@
|
|||
* English collation `a` sorts before `B` and `é` before `f`; by code unit `B` and `f` come first.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import type { BindingRef, SymbolDefinition } from 'gitnexus-shared';
|
||||
import type { BindingRef, ImportEdge, Range, SymbolDefinition } from 'gitnexus-shared';
|
||||
import { rMergeBindings } from '../../../../src/core/ingestion/languages/r/simple-hooks.js';
|
||||
|
||||
const local = (nodeId: string): BindingRef => ({
|
||||
|
|
@ -34,3 +34,71 @@ describe('rMergeBindings', () => {
|
|||
expect(merged.map((b) => b.def.nodeId)).toEqual(['Function:a.R:x']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('rMergeBindings: library() masking in the wildcard tier', () => {
|
||||
const at = (startLine: number, startCol = 0): Range => ({
|
||||
startLine,
|
||||
startCol,
|
||||
endLine: startLine,
|
||||
endCol: startCol + 12,
|
||||
});
|
||||
/** A name brought in by the `library()` call that starts at `range` (none: no position known). */
|
||||
const attached = (nodeId: string, range?: Range): BindingRef => ({
|
||||
origin: 'wildcard',
|
||||
def: { nodeId } as SymbolDefinition,
|
||||
via: { atRange: range } as ImportEdge,
|
||||
});
|
||||
const ids = (merged: BindingRef[]): string[] => merged.map((b) => b.def.nodeId);
|
||||
|
||||
it('keeps only the package attached last, whichever way round the bindings arrive', () => {
|
||||
const lp = attached('Function:lp/R/f.R:dup', at(1));
|
||||
const lp2 = attached('Function:lp2/R/f.R:dup', at(2));
|
||||
expect(ids(rMergeBindings([lp], [lp2], 's'))).toEqual(['Function:lp2/R/f.R:dup']);
|
||||
expect(ids(rMergeBindings([lp2], [lp], 's'))).toEqual(['Function:lp2/R/f.R:dup']);
|
||||
expect(ids(rMergeBindings([lp, lp2], [], 's'))).toEqual(['Function:lp2/R/f.R:dup']);
|
||||
});
|
||||
|
||||
it('is decided by the position of the call, not by the id order of the files', () => {
|
||||
const early = attached('Function:z/R/f.R:dup', at(1));
|
||||
const late = attached('Function:a/R/f.R:dup', at(2));
|
||||
expect(ids(rMergeBindings([early, late], [], 's'))).toEqual(['Function:a/R/f.R:dup']);
|
||||
});
|
||||
|
||||
it('compares the column when two calls share a line', () => {
|
||||
const first = attached('Function:b/R/f.R:dup', at(3, 0));
|
||||
const second = attached('Function:a/R/f.R:dup', at(3, 14));
|
||||
expect(ids(rMergeBindings([first, second], [], 's'))).toEqual(['Function:a/R/f.R:dup']);
|
||||
});
|
||||
|
||||
it('keeps every file of the one package the last call attaches: that ambiguity stays', () => {
|
||||
const old = attached('Function:lp/R/f.R:dup', at(1));
|
||||
const x = attached('Function:lp2/R/x.R:dup', at(2));
|
||||
const y = attached('Function:lp2/R/y.R:dup', at(2));
|
||||
expect(ids(rMergeBindings([old, y, x], [], 's'))).toEqual([
|
||||
'Function:lp2/R/x.R:dup',
|
||||
'Function:lp2/R/y.R:dup',
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps the whole tier when a binding has no call position', () => {
|
||||
const a = attached('Function:a/R/f.R:dup', at(1));
|
||||
const b = attached('Function:b/R/f.R:dup');
|
||||
expect(ids(rMergeBindings([a, b], [], 's'))).toEqual([
|
||||
'Function:a/R/f.R:dup',
|
||||
'Function:b/R/f.R:dup',
|
||||
]);
|
||||
});
|
||||
|
||||
it('leaves a name that one package defines, and the other tiers, as they were', () => {
|
||||
const only = attached('Function:lp/R/f.R:only', at(1));
|
||||
expect(ids(rMergeBindings([only], [], 's'))).toEqual(['Function:lp/R/f.R:only']);
|
||||
const named: BindingRef = {
|
||||
origin: 'import',
|
||||
def: { nodeId: 'Function:p/R/f.R:dup' } as SymbolDefinition,
|
||||
via: { atRange: at(9) } as ImportEdge,
|
||||
};
|
||||
const lp = attached('Function:lp/R/f.R:dup', at(1));
|
||||
const lp2 = attached('Function:lp2/R/f.R:dup', at(2));
|
||||
expect(ids(rMergeBindings([lp, lp2], [named], 's'))).toEqual(['Function:p/R/f.R:dup']);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue