From 86ec528225f125a69b6daf8ebcf9753f64ccbf70 Mon Sep 17 00:00:00 2001 From: Mick Cooney Date: Sun, 18 Oct 2026 15:45:51 +0100 Subject: [PATCH] fix(r): make rename refuse when the NAMESPACE reader may have hidden an export The NAMESPACE scanner skipped conditional (if/else) bodies, dropped raw-string and multi-line arguments, and discarded a malformed directive together with everything up to the next column-0 directive, all silently. Rename treated 'not in the export lists' as proof of privacy, so it could rename a function whose export() entry the reader had never seen and leave NAMESPACE naming a missing function. The scanner now reports when a skipped or discarded region names an export-family directive, and parseRNamespaceExports also flags an argument of one that it cannot read. loadRPackageConfig stores that as RNamespaceInfo.exportsUnreadable and rename refuses with incomplete_semantics. An ordinary NAMESPACE, including conditional imports, does not set it. The indexed isExported is unchanged. r-provider.md updated. --- docs/languages/r-provider.md | 10 +-- .../languages/r/namespace-imports.ts | 63 ++++++++++++++++--- .../ingestion/languages/r/package-config.ts | 8 +++ .../src/core/ingestion/languages/r/rename.ts | 7 +++ .../after/DESCRIPTION | 3 + .../after/NAMESPACE | 2 + .../after/R/pub.R | 3 + .../before/DESCRIPTION | 3 + .../before/NAMESPACE | 2 + .../before/R/pub.R | 3 + .../r/namespace-conditional-export/case.json | 9 +++ .../integration/rename-provider-r.test.ts | 2 +- gitnexus/test/unit/r-namespace-parser.test.ts | 44 +++++++++++++ .../unit/r-rename-namespace-export.test.ts | 37 +++++++++++ 14 files changed, 180 insertions(+), 16 deletions(-) create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/DESCRIPTION create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/NAMESPACE create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/R/pub.R create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/DESCRIPTION create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/NAMESPACE create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/R/pub.R create mode 100644 gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/case.json diff --git a/docs/languages/r-provider.md b/docs/languages/r-provider.md index 3ce8e4009..414d1d956 100644 --- a/docs/languages/r-provider.md +++ b/docs/languages/r-provider.md @@ -45,7 +45,7 @@ Heritage: Exports: -- Read from each local package's NAMESPACE: `export()`, `exportClasses()`, `exportMethods()`, `S3method()` and `exportPattern()`. `S3method(generic, class, method)` exports `method`, the function R registers, else `generic.class`; a generic of another package, `S3method(pkg::generic, class)`, exports `generic.class`. A `DESCRIPTION` file with a `Package:` line marks a package root. +- Read from each local package's NAMESPACE: `export()`, `exportClasses()`, `exportMethods()`, `S3method()` and `exportPattern()`. `S3method(generic, class, method)` exports `method`, the function R registers, else `generic.class`; a generic of another package, `S3method(pkg::generic, class)`, exports `generic.class`. Conditional (`if`/`else`) exports, raw-string and multi-line arguments and the text after a malformed directive are not read (see Limits and known gaps). A `DESCRIPTION` file with a `Package:` line marks a package root. ## Resolution model @@ -61,7 +61,7 @@ The scope query (`languages/r/query.ts`) captures three call shapes only: `f(x)` - `::` and `:::` are not distinguished, so `pkg::name` binds to a definition whether or not the package exports it. - Package discovery is bounded to depth 3 and 200 directories (when the bound may have hidden a package this is flagged internally and nothing is logged), reads directories in code-unit order, ranks duplicate package names by shallowest then by whole path, and never searches the directories the indexer itself ignores (dot-directories, `vendor`, `build`, `dist`, `lib64`, `fixtures` and the rest of its built-in list) or those named exactly `renv`, `packrat`, `revdep` or `.Rcheck`. A `DESCRIPTION` with a `Built:` field is an installed library package (`lib/`, `library/`) and registers nothing. The shared file scanner has no `renv`, `packrat`, `revdep` or `.Rcheck` entries, so `.R` files inside those trees are still parsed even though discovery skips them. - Export status: the language-level export check treats every symbol as public. The NAMESPACE pass then marks the remaining `R/` symbols of a NAMESPACE-bearing package non-exported. Symbols outside `R/` (`tests/`, `vignettes/`, `inst/`) are not package members and stay public; files in `R/` subdirectories are treated as `R/` files although R itself does not load them. A package without a NAMESPACE stays fully public, and a nested package without one does not inherit its enclosing package's. roxygen2 `@export` tags are not consulted. The refinement changes the graph nodes' `isExported` only; the definitions scope resolution works from carry no export verdict, so `library()`, `pkg::name` and the global-name fallback do not consult NAMESPACE (only `importFrom()` does). -- `exportPattern()` text comes from the analysed repository, so patterns are compiled to a linear-time automaton (`linear-regex.ts`) with a work budget. Constructs with no linear-time algorithm (back-references, look-around) and oversized patterns are dropped with one logged warning per pattern per package. Character classes use ASCII semantics. A directive argument the NAMESPACE reader cannot read at all is dropped without a warning: an R 4.0 raw string (`exportPattern(r"(^[^.])")`, `export(r"(foo)")`) and any string that spans lines. It exports nothing, so the names it was meant to cover read as non-exported. +- `exportPattern()` text comes from the analysed repository, so patterns are compiled to a linear-time automaton (`linear-regex.ts`) with a work budget. Constructs with no linear-time algorithm (back-references, look-around) and oversized patterns are dropped with one logged warning per pattern per package. Character classes use ASCII semantics. A directive argument the NAMESPACE reader cannot read at all is dropped without a warning: an R 4.0 raw string (`exportPattern(r"(^[^.])")`, `export(r"(foo)")`) and any string that spans lines. It exports nothing, so the names it was meant to cover read as non-exported in the index. The reader records that it dropped something that may hide an export (this, a conditional `export()` below, or a malformed directive that swallowed one), and `rename` refuses on that record (see Rename). ## Limits and known gaps @@ -89,7 +89,7 @@ Known gaps in call resolution and class handling (each was reproduced and is lef - A function nested in an R6, RefClass or `setMethod()` body has an id such as `Counter.function.bump@7:21`: the enclosing function's name is a placeholder and the line and column keep ids apart, so ids change when lines shift. - A base class named by `inherit =` or `contains =` that is defined more than once in the repository (in two files, or twice in one) gets no `EXTENDS` edge from a class in another file; a base defined in the deriving class's own file is found. - Export patterns use JavaScript regular-expression syntax where R uses TRE, with three silent differences: `\<` and `\>` are the literal characters `<` and `>` (TRE word boundaries), `a{,2}` is the literal text `a{,2}` (TRE `{0,2}`), and a backslash inside `[...]` is an escape (TRE: a literal member). -- NAMESPACE `if (...)` and `else` bodies are skipped, so a function exported only conditionally (`if (getRversion() >= "4.1.0") export(new)`) reads as not exported. +- NAMESPACE `if (...)` and `else` bodies are skipped, so a function exported only conditionally (`if (getRversion() >= "4.1.0") export(new)`) reads as not exported in the index. So does anything after a malformed directive up to the next line that starts with `name(` in column 0. Skipping a body or a malformed directive that names an export-family directive (`export`, `exportClasses`, `exportMethods`, `S3method`, `exportPattern`) is recorded, and `rename` refuses a function of such a package; the persisted `isExported` is not changed by it. A class name that is not unique: a member attaches to the class of its own file when several classes share the name (the same helper class in `R/` and `tests/`, a class defined twice in one file counts once). When the member's file defines none of them, or several, the member stays unowned with its `ownerNameHint`, and one warning for methods and one for fields name the class names concerned (at most five each). @@ -111,7 +111,7 @@ Not supported: - Argument matching follows R for named and reordered arguments but does not model partial argument names: `setClass(Cla = "D")`, `new(Cla = "E")`, `setRefClass(Cla = "A")` and `setMethod("show", sig = "Foo")` are not recognised, so no definition or owner is attached. - Members are read only from the named forms. The positional second argument of `setClass("A", representation(x = "numeric"))`, `R6Class("X", list(...))` and `setRefClass("A", list(...))` creates no field or slot nodes; use `slots=` / `representation=`, `public =` / `private =` / `active =` and `fields =`. - S4 and RefClass constructors, and the import calls, are recognised only as a bare callee: `setClass(...)`, `setRefClass(...)`, `setGeneric(...)`, `setMethod(...)`, `library(...)`, `require(...)` and `source(...)`. A namespace-qualified call such as `methods::setClass("A", slots = list(x = "numeric"))` creates no class, slot, generic or method, and `base::library("pkg")` creates no import. `R6::R6Class(...)` is recognised. -- For `S3method(generic, class, method)` the positional third argument names the exported method. A named third argument such as `S3method(print, foo, method = impl)` is not read: R registers `impl`, but the export is recorded for `print.foo`. +- For `S3method(generic, class, method)` the positional third argument names the exported method. A named third argument such as `S3method(print, foo, method = impl)` is not read: R registers `impl`, but the export is recorded for `print.foo`, and `rename` treats the NAMESPACE as unreadable. - A leading `]` in a bracket expression of a NAMESPACE `exportPattern()` (for example `[]a]` or `[^]a]`) is read with JavaScript RegExp semantics, not POSIX/TRE, unless the pattern also contains a POSIX class such as `[:digit:]`; R would treat that `]` as a literal member. ### Bounds on NAMESPACE exportPattern matching @@ -156,7 +156,7 @@ Not modelled: - a new name that the sources already read as a value or call (`name_collision`): `sapply(x, paste)`, `Reduce(max, x)`, `x <- letters`, a default `f = assist`, a formula term, `assist[1]`, `assist(1)`. After the rename the function is the global of that name and would capture the read. A parameter of an enclosing function of that name covers a read, and covers a call only when it has no default. Not reads: bound names, argument names (`f(assist = 1)`), members (`x$assist`, `x@assist`) and qualified names (`pkg::assist`); - a call of the name under a same-named parameter that may hold a function (`ambiguous_reference`): a function, `NULL` or computed default, or an assignment to the parameter in the body, because the call may reach the parameter or the function being renamed; - a function that is also assigned with `<<-` or `->>` anywhere in the indexed R sources (`ambiguous_reference`, as the top-level form already was): the global re-assignment is a second definition that a rename would leave behind; - - a package function under `R/` that is not proven private, because `NAMESPACE` is not an indexed source and its entry would not be renamed. Two checks must both pass. The persisted `isExported` flag must be false, which it is only for a function that a `NAMESPACE` file beside the package leaves out of `export()`, `exportPattern()` and `S3method()` (the third argument of `S3method(generic, class, method)` counts): a package with no `NAMESPACE` file, or an `R/` folder that belongs to no package, keeps the default public flag and is refused. The flag is not enough alone, because an edit of `NAMESPACE` with no `R/` file touched leaves it stale until a full analyse. So every rename plan also reads the package's `NAMESPACE` from disk again, through the parsing the indexer uses, and refuses (`unsupported_occurrence`) when it now exports the name by `export()`, `exportPattern()` or `S3method()`. It fails closed with `incomplete_semantics` when that `NAMESPACE` was deleted or cannot be read, or when an `exportPattern()` of it was dropped or ran out of work budget (a name such a pattern was meant to cover is then not provably private). A private function in a package whose `NAMESPACE` is unchanged is renamed; a script outside `R/` is not affected. The check does not need `--force`; re-run `analyze` to bring the flag itself up to date. + - a package function under `R/` that is not proven private, because `NAMESPACE` is not an indexed source and its entry would not be renamed. Two checks must both pass. The persisted `isExported` flag must be false, which it is only for a function that a `NAMESPACE` file beside the package leaves out of `export()`, `exportPattern()` and `S3method()` (the third argument of `S3method(generic, class, method)` counts): a package with no `NAMESPACE` file, or an `R/` folder that belongs to no package, keeps the default public flag and is refused. The flag is not enough alone, because an edit of `NAMESPACE` with no `R/` file touched leaves it stale until a full analyse. So every rename plan also reads the package's `NAMESPACE` from disk again, through the parsing the indexer uses, and refuses (`unsupported_occurrence`) when it now exports the name by `export()`, `exportPattern()` or `S3method()`. It fails closed with `incomplete_semantics` when that `NAMESPACE` was deleted or cannot be read, when an `exportPattern()` of it was dropped or ran out of work budget (a name such a pattern was meant to cover is then not provably private), or when the reader skipped or could not read something that may hide an export: a conditional `if (...) export(x)`, a raw-string or multi-line argument, an argument that is an expression, an `S3method()` it cannot read, or a malformed directive that swallowed an export. A private function in a package whose `NAMESPACE` is unchanged is renamed; a script outside `R/` is not affected. The check does not need `--force`; re-run `analyze` to bring the flag itself up to date. - **Refused by the shared engine (fail-closed, not provider code):** S3 methods and any dotted function name (`area.circle` reads as an owner-qualified member, so `stale_graph`); S4 class names (`unsupported_occurrence`, no exact source-name span). - **Invalid names (`invalid_name`):** anything that is not a nonreserved R identifier: the reserved words, `...` and `..1`, `..2`, ..., and any name with a non-ASCII character, because new names are limited to ASCII letters, digits, `.` and `_` (conservative by design; the message says so). An old name made only of non-ASCII characters (`λ`) is refused as not a supported identifier. Backtick-quoting a new name is never offered: a name that needs it is refused rather than quoted at every use. - **Not proven:** unqualified calls to a function defined in another file (including the package `R/` layout) fail with `ambiguous_reference`, because R's global-name fallback is not a provable binding. Renaming across files works only where the call is qualified or otherwise resolved. diff --git a/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts b/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts index 5019a08e1..b08269d77 100644 --- a/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts +++ b/gitnexus/src/core/ingestion/languages/r/namespace-imports.ts @@ -66,6 +66,12 @@ const EXPORT_DIRECTIVES: ReadonlySet = new Set([ 'exportPattern', ]); +/** An export-family directive head followed by `(`, anywhere in a piece of NAMESPACE text. */ +const EXPORT_HEAD_IN_TEXT = new RegExp( + `(? = new Set(['if', 'else']); @@ -361,9 +367,18 @@ interface RawDirective { * before and after is kept. Directives indented after a malformed one are * dropped with it. */ -function scanRNamespaceDirectives(text: string, heads: ReadonlySet): RawDirective[] { +function scanRNamespaceDirectives( + text: string, + heads: ReadonlySet, +): { directives: RawDirective[]; hidesExport: boolean } { const directives: RawDirective[] = []; const blocks = new BraceBlocks(text); + // Set when the scan skipped or discarded text that names an export-family directive. The + // regions are disjoint (every skip moves `i` forward), so the tests cost one pass in total. + let hidesExport = false; + const skipped = (from: number, to: number): void => { + if (!hidesExport && EXPORT_HEAD_IN_TEXT.test(text.slice(from, to))) hidesExport = true; + }; let i = 0; while (i < text.length) { const ch = text[i]; @@ -374,7 +389,9 @@ function scanRNamespaceDirectives(text: string, heads: ReadonlySet): Raw if (isQuote(ch)) { const end = skipQuoted(text, i); if (end === -1) { - i = resyncAfterMalformed(text, i); // stray unterminated quote + const resume = resyncAfterMalformed(text, i); // stray unterminated quote + skipped(i, resume); + i = resume; continue; } i = end; @@ -394,7 +411,9 @@ function scanRNamespaceDirectives(text: string, heads: ReadonlySet): Raw if (text[k] !== '(') { // `else` may be followed directly by a directive with no parens of its own. if (head === 'else') { - i = skipConditionalBody(text, k, blocks); + const end = skipConditionalBody(text, k, blocks); + skipped(k, end); + i = end; continue; } i = j; @@ -403,18 +422,22 @@ function scanRNamespaceDirectives(text: string, heads: ReadonlySet): Raw const group = parseArgs(text, k); if (!group) { - i = resyncAfterMalformed(text, i); // discard this directive, resume at the next one + const resume = resyncAfterMalformed(text, i); // discard this directive, resume at the next one + skipped(i, resume); + i = resume; continue; } i = group.end; if (CONDITIONAL_HEADS.has(head)) { - i = skipConditionalBody(text, i, blocks); + const end = skipConditionalBody(text, i, blocks); + skipped(i, end); + i = end; continue; } if (heads.has(head)) directives.push({ head, args: group.args }); } - return directives; + return { directives, hidesExport }; } /** @@ -423,7 +446,7 @@ function scanRNamespaceDirectives(text: string, heads: ReadonlySet): Raw */ export function parseRNamespaceImportFrom(text: string): RNamespaceImportFromEntry[] { const entries: RNamespaceImportFromEntry[] = []; - for (const { args } of scanRNamespaceDirectives(text, IMPORT_FROM_DIRECTIVES)) { + for (const { args } of scanRNamespaceDirectives(text, IMPORT_FROM_DIRECTIVES).directives) { const values = args.map(normaliseArg); const pkg = values[0]; if (!pkg) continue; @@ -440,6 +463,15 @@ export interface RNamespaceExports { readonly namedExports: readonly string[]; /** `exportPattern()` arguments, R-unescaped (an R string `"\\."` yields the regex source `\.`), in file order. */ readonly exportPatterns: readonly string[]; + /** + * True when the reader skipped, dropped or could not read something that may hide an export: + * a conditional (`if` / `else`) body or a malformed directive that names an export-family + * directive, or an argument of one it cannot read (an R 4.0 raw string, a string that spans + * lines, a name given as an expression, an `S3method()` without a readable generic and class). + * The lists above are then a lower bound, so the absence of a name from them does not show + * that it is private. + */ + readonly unreadable: boolean; } /** @@ -461,16 +493,21 @@ function exportPatternArg(raw: string): string { * (else `generic.class`; `pkg::generic` names the generic after the `::`), and `exportPattern("…")` sources. Multi-line, quoted, * backticked, commented and CRLF input is handled like {@link * parseRNamespaceImportFrom}. Non-simple arguments (named, nested calls) are - * skipped. Never throws; returns empty lists for empty or garbled input. + * skipped, and so are conditional bodies and malformed directives; each of those that may hide + * an export sets {@link RNamespaceExports.unreadable}. Never throws; returns empty lists for + * empty or garbled input. */ export function parseRNamespaceExports(text: string): RNamespaceExports { const namedExports: string[] = []; const exportPatterns: string[] = []; - for (const { head, args } of scanRNamespaceDirectives(text, EXPORT_DIRECTIVES)) { + const scan = scanRNamespaceDirectives(text, EXPORT_DIRECTIVES); + let unreadable = scan.hidesExport; + for (const { head, args } of scan.directives) { if (head === 'exportPattern') { for (const raw of args) { const pattern = exportPatternArg(raw); if (pattern) exportPatterns.push(pattern); + else if (raw.trim()) unreadable = true; } } else if (head === 'S3method') { const generic = s3GenericArg(args[0] ?? ''); @@ -480,15 +517,21 @@ export function parseRNamespaceExports(text: string): RNamespaceExports { // without it the method is `generic.class`. const method = normaliseArg(args[2] ?? ''); namedExports.push(method || `${generic}.${cls}`); + // A third argument that is not a plain name (`method = impl`) registers a function + // this reader does not name. + if (!method && (args[2] ?? '').trim()) unreadable = true; + } else { + unreadable = true; } } else { for (const raw of args) { const name = normaliseArg(raw); if (name) namedExports.push(name); + else if (raw.trim()) unreadable = true; } } } - return { namedExports, exportPatterns }; + return { namedExports, exportPatterns, unreadable }; } // ─── Package lookup ───────────────────────────────────────────────────────── diff --git a/gitnexus/src/core/ingestion/languages/r/package-config.ts b/gitnexus/src/core/ingestion/languages/r/package-config.ts index 72ac1b598..384055882 100644 --- a/gitnexus/src/core/ingestion/languages/r/package-config.ts +++ b/gitnexus/src/core/ingestion/languages/r/package-config.ts @@ -97,6 +97,13 @@ export interface RNamespaceInfo { * is spent. */ exportPatternBudget?: SharedWorkBudget; + /** + * True when the NAMESPACE reader skipped, dropped or could not read something that may hide an + * export (see {@link RNamespaceExports.unreadable}): a conditional export, a raw-string or + * multi-line argument, a malformed directive. A name absent from the export lists is then not + * proven private. Optional so that hand-built configs read as "nothing hidden". + */ + exportsUnreadable?: boolean; /** `importFrom(pkg, name)` pairs in NAMESPACE file order (all entries, incl. self-imports and duplicates). */ importFrom: readonly RNamespaceImportFromEntry[]; } @@ -490,6 +497,7 @@ export async function loadRPackageConfig(repoRoot: string): Promise= "4.1.0") export(newfun) diff --git a/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/R/pub.R b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/R/pub.R new file mode 100644 index 000000000..7cdaa4c03 --- /dev/null +++ b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/after/R/pub.R @@ -0,0 +1,3 @@ +pub <- function() 1 + +newfun <- function() 2 diff --git a/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/DESCRIPTION b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/DESCRIPTION new file mode 100644 index 000000000..2eb67eae1 --- /dev/null +++ b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/DESCRIPTION @@ -0,0 +1,3 @@ +Package: pk +Version: 0.1.0 +Title: Fixture diff --git a/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/NAMESPACE b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/NAMESPACE new file mode 100644 index 000000000..6da0ecdff --- /dev/null +++ b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/NAMESPACE @@ -0,0 +1,2 @@ +export(pub) +if (getRversion() >= "4.1.0") export(newfun) diff --git a/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/R/pub.R b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/R/pub.R new file mode 100644 index 000000000..7cdaa4c03 --- /dev/null +++ b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/before/R/pub.R @@ -0,0 +1,3 @@ +pub <- function() 1 + +newfun <- function() 2 diff --git a/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/case.json b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/case.json new file mode 100644 index 000000000..bd4630758 --- /dev/null +++ b/gitnexus/test/fixtures/rename-integration/r/namespace-conditional-export/case.json @@ -0,0 +1,9 @@ +{ + "language": "r", + "description": "Refuse a package function that NAMESPACE exports only under a condition: the reader skips conditional bodies, so the function cannot be proven private and the export entry would go stale.", + "categories": ["namespace", "package-export", "negative", "unchanged-on-error"], + "target": { "name": "newfun", "kind": "Function", "file": "R/pub.R", "startLine": 2 }, + "newName": "newfun2", + "expect": { "status": "error", "code": "incomplete_semantics", "planningStatus": "unsupported" }, + "edges": [] +} diff --git a/gitnexus/test/integration/rename-provider-r.test.ts b/gitnexus/test/integration/rename-provider-r.test.ts index db4236e1e..277a5e49e 100644 --- a/gitnexus/test/integration/rename-provider-r.test.ts +++ b/gitnexus/test/integration/rename-provider-r.test.ts @@ -5,7 +5,7 @@ import { defineRenameFixtureTests, renameCases } from '../helpers/rename-integra // would define no test and stay green. Pin the number of R cases: adding one means raising it. describe('R rename fixtures', () => { it('are all discovered', () => { - expect(renameCases.filter((fixture) => fixture.language === 'r')).toHaveLength(52); + expect(renameCases.filter((fixture) => fixture.language === 'r')).toHaveLength(53); }); }); diff --git a/gitnexus/test/unit/r-namespace-parser.test.ts b/gitnexus/test/unit/r-namespace-parser.test.ts index 5097caeb9..9045d76b0 100644 --- a/gitnexus/test/unit/r-namespace-parser.test.ts +++ b/gitnexus/test/unit/r-namespace-parser.test.ts @@ -450,3 +450,47 @@ describe('loadRPackageConfig -> RNamespaceInfo.importFrom', () => { expect(config?.namespaceInfoByPackageDir.get('')?.importFrom).toEqual([]); }); }); + +describe('parseRNamespaceExports: what the reader could not read', () => { + const unreadable = (src: string): boolean => parseRNamespaceExports(src).unreadable; + + it.each([ + ['a conditional export', 'if (getRversion() >= "4.1.0") export(newfun)\n'], + ['an else export', 'if (a) {\n} else export(newfun)\n'], + ['a conditional export block', 'if (a) {\n export(newfun)\n}\n'], + ['a conditional S3method', 'if (a) S3method(print, foo)\n'], + ['a conditional exportPattern', 'if (a) exportPattern("^x")\n'], + ['a raw-string export', 'export(r"(newfun)")\n'], + ['a raw-string exportPattern', 'exportPattern(r"(^x)")\n'], + ['an expression argument', 'export(paste0("a", "b"))\n'], + ['an unbalanced directive that swallows an export', 'export(a\n export(newfun)\nexport(b)\n'], + ['an unterminated quote that swallows an export', 'export("a)\n export(newfun)\n'], + ['a string that spans lines', 'export("new\nfun")\n'], + ['an S3method without a readable class', 'S3method(print, r"(foo)")\n'], + ['an S3method with a named third argument', 'S3method(print, foo, method = impl)\n'], + ])('flags %s', (_label, src) => { + expect(unreadable(src)).toBe(true); + }); + + it.each([ + ['an empty file', ''], + ['comments only', '# export(x)\n'], + [ + 'plain directives', + 'export(a, b)\nexportClasses(C)\nS3method(print, foo, impl)\nexportPattern("^x")\n', + ], + ['a trailing comma', 'export(a, b,)\n'], + ['a multi-line export', 'export(\n a,\n b\n)\n'], + ['a conditional import', 'if (a) importFrom(x, y)\nelse import(z)\n'], + ['a conditional block with no export', 'if (a) {\n importFrom(x, y)\n}\n'], + ['a malformed import that hides no export', 'importFrom(x, y\nexport(a)\n'], + ])('does not flag %s', (_label, src) => { + expect(unreadable(src)).toBe(false); + }); + + it('keeps the exports it can read next to the flag', () => { + const parsed = parseRNamespaceExports('export(kept)\nif (a) export(hidden)\n'); + expect(parsed.namedExports).toEqual(['kept']); + expect(parsed.unreadable).toBe(true); + }); +}); diff --git a/gitnexus/test/unit/r-rename-namespace-export.test.ts b/gitnexus/test/unit/r-rename-namespace-export.test.ts index 70f082567..594a42567 100644 --- a/gitnexus/test/unit/r-rename-namespace-export.test.ts +++ b/gitnexus/test/unit/r-rename-namespace-export.test.ts @@ -235,6 +235,43 @@ describe('R rename: the NAMESPACE on disk decides, not the persisted flag', () = expect(await stale('export(pub)\nexportPattern("(")\n')).toBe('incomplete_semantics'); }); + it.each([ + ['a conditional export', 'export(pub)\nif (getRversion() >= "4.1.0") export(hidden)\n'], + ['a conditional export in a block', 'export(pub)\nif (TRUE) {\n export(hidden)\n}\n'], + ['an else export', 'export(pub)\nif (FALSE) export(a) else export(hidden)\n'], + ['a raw-string export', 'export(pub)\nexport(r"(hidden)")\n'], + ['a raw-string exportPattern', 'export(pub)\nexportPattern(r"(^hid)")\n'], + ['an export spanning lines in a string', 'export(pub)\nexport("hid\nden")\n'], + ['an export given as an expression', 'export(pub)\nexport(paste0("hid", "den"))\n'], + [ + 'a malformed directive that swallows an export', + 'export(pub\n export(hidden)\nexport(pub)\n', + ], + ['an unreadable S3method()', 'export(pub)\nS3method(print, foo, method = hidden)\n'], + ])('refuses, as unreadable, a NAMESPACE with %s', async (_label, namespace) => { + expect(await stale(namespace)).toBe('incomplete_semantics'); + }); + + it('does not treat an ordinary NAMESPACE as unreadable', async () => { + const ordinary = [ + '# Generated by roxygen2: do not edit by hand', + '', + 'S3method(print, foo)', + 'export(pub)', + 'export(', + ' other,', + ' another', + ')', + 'exportPattern("^api_")', + 'importFrom(stats, sd)', + 'import(utils)', + 'useDynLib(pk, .registration = TRUE)', + 'if (getRversion() >= "4.1.0") importFrom(stats, setNames)', + '', + ].join('\n'); + expect(await stale(ordinary)).toBe('ok'); + }); + it('still refuses a function the index records as exported when NAMESPACE no longer does', async () => { // The index is stale the other way round; the persisted half of the proof fails. expect(await stale('export(pub)\n', 'hidden', 'export(pub)\nexport(hidden)\n')).toBe(