search

extendr / extendr

R extension library for rust designed to be familiar to R users.
https://extendr.github.io
MIT License
425 stars 42 forks source link

Wrapper info generated from Rust needs to be more configurable #145

Closed clauswilke closed 3 years ago

clauswilke commented 3 years ago

The metadata wrapper code currently makes a few incorrect assumptions and needs to be fixed:

  1. Roxygen tags (#' @param, #' @export, etc) should not be added automatically. They need to be under the control of the user, because not every function needs to be exported. (In fact, many Rust functions will likely not be exported from a package.) See how this is done in Rcpp: https://gallery.rcpp.org/articles/documenting-rcpp-packages/
  2. This point is irrelevant if we properly address point 1, but at present, #' @export is written before the documentation, and that is invalid roxygen output. It'll break if you try to document as is.
  3. The wrapper code assumes that we're developing a package (by adding things such as #' @useDynLib packagename), but that is not the only use case. Wrappers for rextendr use should not contain these lines. So this needs to be configurable.

The wrapper code is written here: https://github.com/extendr/extendr/blob/43c5c7eafd48577f824c7d61ae353527e9116cb3/extendr-api/src/metadata.rs#L110-L111

and here: https://github.com/extendr/extendr/blob/43c5c7eafd48577f824c7d61ae353527e9116cb3/extendr-api/src/metadata.rs#L193-L224

clauswilke commented 3 years ago

Ok, looking at the Rust code in extendrtests, it looks like the fix for functions is very simple: https://github.com/extendr/extendr/blob/43c5c7eafd48577f824c7d61ae353527e9116cb3/tests/extendrtests/src/rust/src/lib.rs#L11-L14

The @param is already in the docstring. We should similarly move the @export into the docstring.

For classes it's more complicated, because we need to export things such as $.MyClass if we want the class to be exported. This may require us to parse the docstring and look for an @export tag.

clauswilke commented 3 years ago

I think for classes we should read the doc strings for the class and check if they contain the string /// @export. If yes we export the relevant $ function and if not we don't.

If the $ function gets exported it should also have roxygen tags @usage NULL and @rdname <classname> where <classname> is the name of the class we're generating.

clauswilke commented 3 years ago

I can try to make a PR with these fixes later this week.