diff --git a/CHANGELOG.md b/CHANGELOG.md index 975dff9db..6c636e179 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In R, a call like `c(1, 2)` is no longer linked to a data variable that happens to share a base function's name, since R skips non-function bindings when it looks up a call. Before, ggplot2's facet tests bound `c <- data_frame(b = 3)`, and every `c(…)` call in those files pointed at that data frame. Functions built by a factory, like ggplot2's `geom_point <- make_constructor(…)`, still resolve. - In Swift, a call to an overloaded method now reaches the overload whose argument labels it spells, with defaulted parameters optional. Before, only the argument count was compared. Alamofire's `self.tableView(tableView, numberOfRowsInSection:)` inside another `tableView` overload linked to itself, and `cancel(optionallyProducingResumeData:)` linked to `cancel(producingResumeData:)`. - A Dart `extension on X { … }` with no name is no longer mistaken for the type `X`, and its members are reachable only inside the file that declares it, as in Dart. Before, bloc's linter linked analyzer's `Token` and Dart's `Uri` to small extension blocks, and every flutter_test `find.text(…)` call linked to a `text` getter on another example's `TaskStatus`. - In Java, a bare type name now resolves only to a type in reach. That is a type in the same package, one imported by name or with `.*`, or a nested type used from inside its owner, from a subclass (anonymous classes included), or written through its owner as in `Outer.Inner`. A constructor no longer stands in for its class, and an enum constant is reached by bare name only inside its enum, in a `case` label, or through a static import. Before, retrofit's `new Retrofit.Builder()` linked to `RequestFactory.Builder`, halo's micrometer `Counter` to its own `Counter` class, and jsoup's `java.lang.Class` to a nested selector class. diff --git a/__tests__/r-call-skips-data-binding.test.ts b/__tests__/r-call-skips-data-binding.test.ts new file mode 100644 index 000000000..45e80e318 --- /dev/null +++ b/__tests__/r-call-skips-data-binding.test.ts @@ -0,0 +1,50 @@ +/** + * R looks a call's name up among functions only, skipping other bindings: + * ggplot2's facet tests bind `c <- data_frame(b = 3)`, and every `c(1, 2)` + * in those files — base R's `c` — went to that data frame. A project binding + * made by a function factory (`geom_point <- make_constructor(…)`) stays. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-r-call-')); + const files: Record = { + 'DESCRIPTION': 'Package: ggplot2\n', + 'R/geom-point.R': `geom_point <- make_constructor(GeomPoint, position = "identity") +`, + 'tests/testthat/test-facet-wrap.R': `a <- data_frame(a = 1:3) +c <- data_frame(b = 3) + +test_that("facets", { + p <- geom_point() + expect_equal(c(1, 2), a$a) +}) +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('R calls', () => { + it('skip a data binding that shadows a base function, keep a factory-made function', () => { + const ids = cg.getNodesInFile('tests/testthat/test-facet-wrap.R').map((n) => n.id); + const targets = cg.getOutgoingEdgesFrom(ids).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!.name); + expect(targets).not.toContain('c'); + expect(targets).toContain('geom_point'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index cd7f60beb..c3221f24c 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8886,6 +8886,14 @@ export function matchReference( // selected = useStore(s => s.reset); selected()` is the store's `reset`.) if (target && target.name === ref.referenceName && isOutsideJsLocal(target, ref, context)) return null; } + // R looks a call's name up among FUNCTIONS only, skipping other bindings: + // ggplot2's tests' `c <- data_frame(b = 3)` is never what `c(1, 2)` calls — + // base R's `c` is. (A project binding made by a function factory, ggplot2's + // `geom_point <- make_constructor(…)`, is a function, and stays.) + if (result && ref.language === 'r' && ref.referenceKind === 'calls' && R_BASE_FUNCTIONS.has(ref.referenceName)) { + const target = context.getNodeById?.(result.targetNodeId); + if (target && (target.kind === 'variable' || target.kind === 'constant') && !isRFunctionValue(target, context)) return null; + } // C has no methods, and C code cannot call a C++ one: hiredis' function // pointer `c->funcs->read(c, buf, …)` is no Qt adapter's `read`. if (result && ref.language === 'c' && ref.referenceKind === 'calls' && @@ -8930,6 +8938,21 @@ function ownerPathOf(n: Node, context: ResolutionContext): string { /** Reference kinds a bare JS/TS local can be: a call, a value, a construction. */ const JS_LOCAL_REF_KINDS: ReadonlySet = new Set(['calls', 'references', 'function_ref', 'instantiates']); +/** Base R functions whose names data often shadows (`c <- data_frame(…)`, `df <- …`, `t <- 1`). */ +const R_BASE_FUNCTIONS: ReadonlySet = new Set([ + 'c', 't', 'q', 'df', 'dt', 'data', 'list', 'length', 'names', 'max', 'min', 'sum', 'mean', 'range', 'rev', 'sort', + 'order', 'rep', 'seq', 'cat', 'print', 'paste', 'paste0', 'format', 'levels', 'factor', 'matrix', 'vector', 'table', + 'scale', 'sample', 'exp', 'log', 'abs', 'all', 'any', 'which', 'nchar', 'summary', 'file', 'dir', 'identity', 'unique', + 'nrow', 'ncol', 'rownames', 'colnames', 'array', 'character', 'numeric', 'integer', 'logical', 'mode', 'class', 'body', + 'args', 'environment', 'search', 'diff', 'round', 'sign', 'trunc', 'var', 'sd', 'median', 'quantile', 'weights', +]); + +/** Whether an R binding holds a function: `f <- function(…)`, `f = \\(x) …`, a `purrr::partial(…)` aside. */ +function isRFunctionValue(n: Node, context: ResolutionContext): boolean { + const line = (context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? [])[n.startLine - 1] ?? ''; + return /(?:< = new Set(['variable', 'constant', 'field', 'property']);