Inherit group params exactly (#1316)

Fixes #950

NEWS.md

@@ -1,5 +1,9 @@
 # roxygen2 (development version)
 
+* `@inheritParams` now only inherits exact multiparameter matches, so if you're
+  inheriting from a function with `@param x,y` you'll only get the parameter
+  documentation if your function needs docs for both x and y (#950).
+
 * All tags warn now if you only provide whitespace (#1228).
 
 * Add support for inheriting 'note' fields via `@inherit pkg::fun note` (@pat-s, #1218)

R/rd-inherit.R

@@ -141,26 +141,28 @@ inherit_params <- function(topic, topics) {
   }
 
   for (inheritor in inheritors) {
-    inherited <- find_params(inheritor, topics)
-
-    matches <- map_chr(missing, match_param, names(inherited))
-    new_match <- !is.na(matches)
-
-    if (!any(new_match)) {
-      # Can't warn here because @inherit inherits parameters
-      next
+    inherited_params <- find_params(inheritor, topics)
+
+    for (param in inherited_params) {
+      match <- match_param(param$name, missing)
+      if (all(!is.na(match))) {
+        param_val <- setNames(param$value, paste(match, collapse = ","))
+        topic$add(rd_section("param", param_val))
+        missing <- setdiff(missing, match)
+      }
     }
-
-    topic$add(
-      rd_section(
-        "param",
-        setNames(inherited[matches[new_match]], missing[new_match])
-      )
-    )
-    missing <- missing[!new_match]
   }
 }
 
+# Ignore . prefix since it's sometimes necessary to add because a
+# function uses ...
+match_param <- function(needle, haystack) {
+  needle_std <- gsub("^\\.", "", needle)
+  haystack_std <- gsub("^\\.", "", haystack)
+
+  haystack[match(needle_std, haystack_std)]
+}
+
 inherit_dot_params <- function(topic, topics, env) {
   inheritors <- topic$get_value("inherit_dot_params")
   if (is.null(inheritors))
@@ -173,9 +175,12 @@ inherit_dot_params <- function(topic, topics, env) {
   # Then pull out the ones we need
   docs <- lapply(inheritors$source, find_params, topics = topics)
   arg_matches <- function(args, docs) {
-    doc_args <- str_split(names(docs), ", ?")
-    match <- map_lgl(doc_args, function(x) x %in% args)
-    docs[match]
+    match <- map_lgl(docs, function(x) x$name %in% args)
+    matched <- docs[match]
+    setNames(
+      lapply(matched, "[[", "value"),
+      map_chr(matched, function(x) paste(x$name, collapse = ","))
+    )
   }
   docs_selected <- unlist(map2(args, docs, arg_matches))
 
@@ -232,11 +237,10 @@ find_params <- function(name, topics) {
   param_names <- str_trim(names(params))
   param_names[param_names == "\\dots"] <- "..."
 
-  # Split up compound names on , (swallowing spaces) duplicating their contents
-  individual_names <- strsplit(param_names, ",\\s*")
-  reps <- map_int(individual_names, length)
-
-  setNames(rep.int(params, reps), unlist(individual_names))
+  Map(list,
+    name = strsplit(param_names, ",\\s*"),
+    value = unlist(params)
+  )
 }
 
 topic_params <- function(x) UseMethod("topic_params")
@@ -392,26 +396,3 @@ get_rd_from_help <- function(package, alias) {
 
   internal_f("utils", ".getHelpFile")(help)
 }
-
-
-# helpers -----------------------------------------------------------------
-
-# Returns matching parameter name in haystack
-match_param <- function(needle, haystack) {
-  if (needle %in% haystack) {
-    return(needle)
-  }
-
-  if (substr(needle, 1, 1) == ".") {
-    if (needle %in% paste0(".", haystack)) {
-      return(substr(needle, 2, nchar(needle)))
-    }
-  } else {
-    if (paste0(".", needle) %in% haystack) {
-      return(paste0(".", needle))
-    }
-  }
-
-  NA
-}
-

tests/testthat/test-rd-inherit.R

@@ -325,17 +325,48 @@ test_that("multiple @inheritParam inherits from existing topics", {
 })
 
 
-test_that("@inheritParam can cope with multivariable argument definitions", {
+test_that("@inheritParam can inherit multivariable arguments", {
   out <- roc_proc_text(rd_roclet(), "
-                       #' My merge
-                       #'
-                       #' @inheritParams base::merge
-                       mymerge <- function(x, y) {}")[[1]]
-  params <- out$get_value("param")
-  expect_equal(length(params), 2)
-  expect_equal(sort(names(params)), c("x", "y"))
+    #' A
+    #' @param x,y X and Y
+    A <- function(x, y) {}
+
+    #' B
+    #'
+    #' @inheritParams A
+    B <- function(x, y) {}"
+  )[[2]]
+  expect_equal(out$get_value("param"), c("x,y" = "X and Y"))
+
+  # Even when the names only match without .
+  out <- roc_proc_text(rd_roclet(), "
+    #' A
+    #' @param x,y X and Y
+    A <- function(x, y) {}
+
+    #' B
+    #'
+    #' @inheritParams A
+    B <- function(.x, .y) {}"
+  )[[2]]
+  expect_equal(out$get_value("param"), c(".x,.y" = "X and Y"))
+})
+
+test_that("@inheritParam only inherits exact multiparam matches", {
+  out <- roc_proc_text(rd_roclet(), "
+    #' A
+    #' @param x,y X and Y
+    A <- function(x, y) {}
+
+    #' B
+    #'
+    #' @inheritParams A
+    B <- function(x) {}"
+  )[[2]]
+  expect_equal(out$get_value("param"), NULL)
 })
 
+
 test_that("@inheritParam understands compound docs", {
   out <- roc_proc_text(rd_roclet(), "
     #' Title
@@ -607,12 +638,3 @@ test_that("can find section in existing docs", {
   out <- find_sections(get_rd("base::attach"))
   expect_equal(out$title, "Good practice")
 })
-
-# find_params -------------------------------------------------------------
-
-test_that("find_params parses input", {
-  params <- find_params("utils::`?`", NULL)
-  expect_equal(names(params), c("topic", "type"))
-})
-
-