document generates wrong S3 exports if NAMESPACE doesn’t exist yet

[klmr]

My package ‹modules› has an insidious bug related to incorrect exports of S3 methods, generated via roxygen2. However, the roxygen2 tags inside my package are correct:

#' @importFrom roxygen2 roclet_tags
#' @export
roclet_tags.roclet_export = function (x) {

The problem is that my deployment process (which generates the documentation) starts from a blank slate, i.e. no NAMESPACE file exists yet. As a consequence, even though the Imports are correctly listed in the DESCRIPTION, devtools fails to import the necessary S3 generic which my method extends. Subsequently roxygen2 fails to recognise that these methods are, indeed, S3 methods; and it generates the following NAMESPACE entry:

export(roclet_tags.roclet_export)

---

To reproduce this issue:

1. Generate an empty package using Rscript -e 'dummy = 1; package.skeleton("foo", "dummy")'

2. cd foo

3. Add imports: echo 'Imports: roxygen2' >> DESCRIPTION

4. rm NAMESPACE

5. Add a new file R/test.r with the contents

   #' @importFrom roxygen2 roclet_tags
   #' @export
   roclet_tags.roclet_foobar = function (x) { list() }

6. Run Rscript -e 'devtools::document()'

Expected output:

# Generated by roxygen2: do not edit by hand

S3method(roclet_tags,roclet_foobar)
importFrom(roxygen2,roclet_tags)

Actual output:

# Generated by roxygen2: do not edit by hand

export(roclet_tags.roclet_foobar)
importFrom(roxygen2,roclet_tags)

Running Rscript -e 'devtools::document()' a second time yields a corrected NAMESPACE file.

---

I am reporting this bug here since using roxygen2::roxygenize() instead of devtools::document() actually works: roxygen2::roxygenize is “dumber” and performs less sophisticated loading. devtools::load_all, by contrast, is “too smart” and attempts to use the (non-existent) NAMESPACE as a hint for loading. This is fundamentally circular and, though a good heuristic, bound to produce false results on occasion.

Unfortunately devtools gives no indication of potential problems, and the result is very hard to debug failures (see linked bug report): I took several days of trial and error and going through roxygen2 and devtools internals to find the problem; if I hadn’t already known my way around both code bases this would have taken even longer.

At the very least document should produce a clear warning about potentially mis-classifying S3 methods; even better would be a way of controlling the behaviour so that a single pass of document produces the correct output.

[jimhester]

The easy solution is as you noted to just run document() twice.

Having to run document() twice ends up being fairly common when you make changes that affect the NAMESPACE.

[klmr]

Fair enough, that’s an acceptable solution if it’s clearly documented.

I’m wondering if it’s actually necessary to load the package so meticulously when generating the NAMESPACE file, rather than simply loading all imports — similar to what roxygen2:::source_package does (although it would probably be better not to use require, which attaches packages).

… But I’m probably overlooking some corner cases where that would yield wrong results, am I not?

[hadley]

The long term solution is here: r-lib/roxygen2#372

[jimhester]

Also FWIW the only reason that roxygen2::roxygenize() seems to 'work' in the example package is because calling roxygenize() implicitly loads the roxygen2 namespace. i.e. it will only work with generics defined in roxygen2 itself. Modify your example to define a method for remote_sha, a generic defined in devtools, run roxygen2::roxygenize() and you will see the same behavior as document().

[klmr]

@jimhester roxygen2:::load_pkg_dependencies (which is called internally) should take care of that, shouldn’t it?

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/