Improve feedback from @inherits and friends

DESCRIPTION

@@ -21,6 +21,7 @@ Depends:
     R (>= 3.3)
 Imports: 
     brew,
+    cli,
     commonmark,
     desc (>= 1.2.0),
     digest,

NEWS.md

@@ -1,5 +1,9 @@
 # roxygen2 (development version)
 
+* `@inherit` and friends perform less aggressive link tweaking, eliminating
+  many spurious warnings. Additionally, when you do get a warning, you'll 
+  now always learn which topic it's coming from (#1135).
+
 * Add support for inheriting 'note' fields via `@inherit pkg::fun note` (@pat-s, #1218)
 
 * Problems with the first tag in each block are now reported with the

R/rd-inherit.R

@@ -133,15 +133,18 @@ inherit_params <- function(topic, topics) {
   needed <- topic$get_value("formals")
   missing <- setdiff(needed, documented)
   if (length(missing) == 0) {
-    warn(paste0(
-      "Topic '", topic$get_name(), "': ",
-      "no parameters to inherit with @inheritParams"
-    ))
+    cli::cli_warn(
+      c(
+        "@inheritParams failed in topic {.str {topic$get_name()}}.",
+        x = "All parameteres are already documented; none remain to be inherited."
+      ),
+      call = NULL
+    )
     return()
   }
 
   for (inheritor in inheritors) {
-    inherited <- find_params(inheritor, topics)
+    inherited <- find_params(inheritor, topics, source = topic$get_name())
 
     matches <- map_chr(missing, match_param, names(inherited))
     new_match <- !is.na(matches)
@@ -219,8 +222,8 @@ get_documented_params <- function(topic, only_first = FALSE) {
   documented
 }
 
-find_params <- function(name, topics) {
-  topic <- get_rd(name, topics)
+find_params <- function(name, topics, source) {
+  topic <- get_rd(name, topics, source = source)
   if (is.null(topic)) {
     return()
   }
@@ -239,24 +242,23 @@ find_params <- function(name, topics) {
   setNames(rep.int(params, reps), unlist(individual_names))
 }
 
-topic_params <- function(x) UseMethod("topic_params")
-topic_params.Rd <- function(x) {
-  arguments <- get_tags(x, "\\arguments")
-  if (length(arguments) != 1) {
-    return(list())
-  }
-  items <- get_tags(arguments[[1]], "\\item")
+topic_params <- function(x) {
+  if (inherits(x, "Rd")) {
+    arguments <- get_tags(x, "\\arguments")
+    if (length(arguments) != 1) {
+      return(list())
+    }
+    items <- get_tags(arguments[[1]], "\\item")
 
-  values <- map_chr(items, function(x) rd2text(x[[2]]))
-  params <- map_chr(items, function(x) rd2text(x[[1]]))
+    values <- map_chr(items, function(y) rd2text(y[[2]], attr(x, "package")))
+    params <- map_chr(items, function(y) rd2text(y[[1]], attr(x, "package")))
 
-  setNames(values, params)
-}
-topic_params.RoxyTopic <- function(x) {
-  x$get_value("param")
+    setNames(values, params)
+  } else {
+    x$get_value("param")
+  }
 }
 
-
 # Inherit sections --------------------------------------------------------
 
 inherit_sections <- function(topic, topics) {
@@ -294,10 +296,14 @@ inherit_section <- function(topic, topics) {
     selected <- new_section$title %in% titles[[i]]
 
     if (sum(selected) != 1) {
-      warning(
-        "Can't find section '", titles[[i]], "' in ?",
-        sources[[i]], call. = FALSE
+      cli::cli_warn(
+        c(
+          "@inheritSection failed in topic {.str {topic$get_name()}}.",
+          x = "Can't find section {.str {titles[[i]]}} in topic {sources[[i]]}."
+        ),
+        call = NULL
       )
+      return()
     }
 
     topic$add(
@@ -310,8 +316,8 @@ find_sections <- function(topic) {
   if (inherits(topic, "Rd")) {
     tag <- get_tags(topic, "\\section")
 
-    titles <- map_chr(map(tag, 1), rd2text)
-    contents <- map_chr(map(tag, 2), rd2text)
+    titles <- map_chr(map(tag, 1), rd2text, package = attr(topic, "package"))
+    contents <- map_chr(map(tag, 2), rd2text, package = attr(topic, "package"))
 
     list(title = titles, content = contents)
   } else {
@@ -329,7 +335,7 @@ inherit_field <- function(topic, topics, rd_name, roxy_name = rd_name) {
 
   # Otherwise, try each try function listed in inherits
   for (inherit_from in topic$inherits_from(roxy_name)) {
-    inherit_topic <- get_rd(inherit_from, topics)
+    inherit_topic <- get_rd(inherit_from, topics, source = topic$get_name())
     if (is.null(inherit_topic)) {
       next
     }
@@ -352,48 +358,100 @@ find_field <- function(topic, field_name) {
     value <- tag[[1]]
     attr(value, "Rd_tag") <- NULL
 
-    str_trim(rd2text(value))
+    str_trim(rd2text(value, attr(topic, "package")))
   } else {
     topic$get_value(field_name)
   }
 }
 
+rd2text <- function(x, package) {
+  x <- tweak_links(x, package)
+  chr <- as_character_rd(structure(x, class = "Rd"), deparse = TRUE)
+  paste(chr, collapse = "")
+}
+
+# Convert relative to absolute links
+tweak_links <- function(x, package) {
+  tag <- attr(x, "Rd_tag")
+
+  if (is.list(x)) {
+    if (!is.null(tag) && tag == "\\link") {
+      opt <- attr(x, "Rd_option")
+      if (is.null(opt)) {
+        if (has_topic(x[[1]], package)) {
+          attr(x, "Rd_option") <- structure(package, Rd_tag = "TEXT")
+        }
+      } else if (is_string(opt) && substr(opt, 1, 1) == "=") {
+        topic <- substr(opt, 2, nchar(opt))
+
+        if (has_topic(topic, package)) {
+          attr(x, "Rd_option") <- structure(paste0(package, ":", topic), Rd_tag = "TEXT")
+        }
+      }
+    } else if (length(x) > 0) {
+      x[] <- map(x, tweak_links, package = package)
+    }
+  }
+
+  x
+}
+
+
 # Find info in Rd or topic ------------------------------------------------
 
-get_rd <- function(name, topics) {
+get_rd <- function(name, topics, source) {
   if (has_colons(name)) {
     # External package
     parsed <- parse_expr(name)
     pkg <- as.character(parsed[[2]])
     fun <- as.character(parsed[[3]])
 
-    tweak_links(get_rd_from_help(pkg, fun), package = pkg)
+    get_rd_from_help(pkg, fun, source)
   } else {
     # Current package
     rd_name <- topics$find_filename(name)
     if (identical(rd_name, NA_character_)) {
-      warn(paste0("Can't find help topic '", name, "' in current package"))
+      cli::cli_warn(
+        c(
+          "@inherits failed in topic {.str {source}}.",
+          x = "Can't find topic {.str {name}}."
+        ),
+        call = NULL
+      )
     }
     topics$get(rd_name)
   }
 }
 
-get_rd_from_help <- function(package, alias) {
+get_rd_from_help <- function(package, alias, source) {
   if (!is_installed(package)) {
-    warn(paste0("Can't find package '", package, "'"))
+    cli::cli_warn(
+      c(
+        "@inherits failed in topic {.str {source}}.",
+        x = "Package {package} is not installed."
+      ),
+      call = NULL
+    )
     return()
   }
 
-  help <- eval(expr(help(!!alias, !!package)))
+  help <- utils::help((alias), (package))
   if (length(help) == 0) {
-    warn(paste0("Can't find help topic '", alias, "' in '", package, "' package"))
+    cli::cli_warn(
+      c(
+        "@inherits failed in topic {.str {source}}.",
+        x = "Can't find topic {package}::{alias}."
+      ),
+      call = NULL
+    )
     return()
   }
 
-  internal_f("utils", ".getHelpFile")(help)
+  out <- internal_f("utils", ".getHelpFile")(help)
+  attr(out, "package") <- package
+  out
 }
 
-
 # helpers -----------------------------------------------------------------
 
 # Returns matching parameter name in haystack

R/utils-rd.R

@@ -68,48 +68,6 @@ get_tags <- function(rd, tag) {
   Filter(function(x) identical(attr(x, "Rd_tag"), tag), rd)
 }
 
-rd2text <- function(x) {
-  chr <- as_character_rd(structure(x, class = "Rd"), deparse = TRUE)
-  paste(chr, collapse = "")
-}
-
-tweak_links <- function(x, package) {
-  tag <- attr(x, "Rd_tag")
-
-  if (is.list(x)) {
-    if (!is.null(tag) && tag == "\\link") {
-      opt <- attr(x, "Rd_option")
-      if (is.null(opt)) {
-        if (has_topic(x[[1]], package)) {
-          attr(x, "Rd_option") <- structure(package, Rd_tag = "TEXT")
-        }
-      } else {
-        if (is.character(opt) && length(opt) == 1 && substr(opt, 1, 1) == "=") {
-          topic <- substr(opt, 2, nchar(opt))
-
-          if (has_topic(topic, package)) {
-            file <- find_topic_in_package(package, topic)
-            attr(x, "Rd_option") <- structure(paste0(package, ":", file), Rd_tag = "TEXT")
-          }
-        } else if (grepl(":", opt)) {
-          # need to fix the link to point to a file
-          target <- str_split_fixed(opt, ":", n = 2)
-          file <- try_find_topic_in_package(
-            target[1],
-            target[2],
-            where = " in inherited text"
-          )
-          attr(x, "Rd_option") <- structure(paste0(target[1], ":", file), Rd_tag = "TEXT")
-        }
-      }
-    } else if (length(x) > 0) {
-      x[] <- map(x, tweak_links, package = package)
-    }
-  }
-
-  x
-}
-
 # helpers -----------------------------------------------------------------
 
 parse_rd <- function(x) {

tests/testthat/_snaps/rd-inherit.md

@@ -5,6 +5,16 @@
     "sha1"}
     } 
 
+# warns if can't find section
+
+    @inheritSection failed in topic "b".
+    x Can't find section "A" in topic a.
+
+# warned if no params need documentation
+
+    @inheritParams failed in topic "x".
+    x All parameteres are already documented; none remain to be inherited.
+
 # can inherit all from single function
 
     [1] "test-rd-inherit-dots.txt"
@@ -25,3 +35,30 @@
       }}
     } 
 
+# useful warnings if can't find topics
+
+    Code
+      get_rd("base2::attach", source = "source")
+    Condition
+      Warning:
+      @inherits failed in topic "source".
+      x Package base2 is not installed.
+    Output
+      NULL
+    Code
+      get_rd("base::function_not_found", source = "source")
+    Condition
+      Warning:
+      @inherits failed in topic "source".
+      x Can't find topic base::function_not_found.
+    Output
+      NULL
+    Code
+      get_rd("function", RoxyTopics$new(), source = "source")
+    Condition
+      Warning:
+      @inherits failed in topic "source".
+      x Can't find topic "function".
+    Output
+      NULL
+