Improve messages for two part tags

And provide a helpful suggestion for `@describeIn`. Fixes #1493.

NEWS.md

@@ -1,5 +1,8 @@
 # roxygen2 (development version)
 
+* `@describeIn foo` now suggests that you might want `@rdname` instead 
+  (#1493).
+
 * If you document a function from another package it is automatically 
   imported. Additionally, if you set `@rdname` or `@name` you can opt out 
   of the default `reexports` topic generation and provide your own docs 

R/rd-describe-in.R

@@ -1,6 +1,14 @@
 #' @export
 roxy_tag_parse.roxy_tag_describeIn <- function(x) {
-  tag_name_description(x)
+  if (!is.na(x$raw) && !str_detect(x$raw, "[[:space:]]+")) {
+    warn_roxy_tag(x, c(
+      "requires a name and description",
+      i = "Did you want @rdname instead?"
+    ))
+    NULL
+  } else {
+    tag_two_part(x, "a topic name", "a description")
+  }
 }
 
 topic_add_describe_in <- function(topic, block, env) {

R/rd-include-rmd.R

@@ -5,13 +5,13 @@ roxy_tag_parse.roxy_tag_includeRmd <- function(x) {
     return()
   }
 
-  tag_two_part(x, "path", "section", required = FALSE, markdown = FALSE)
+  tag_two_part(x, "a path", "a section", required = FALSE, markdown = FALSE)
 }
 
 #' @export
 roxy_tag_rd.roxy_tag_includeRmd <- function(x, base_path, env) {
-  rmd <- rel_rmd <- x$val$path
-  section <- x$val$section
+  rmd <- rel_rmd <- x$val$name
+  section <- x$val$description
 
   if (!file.exists(rmd)) {
     warn_roxy_tag(x, "Can't find Rmd {.path {rmd}}")

R/rd-inherit.R

@@ -16,15 +16,17 @@ roxy_tag_rd.roxy_tag_inheritParams <- function(x, base_path, env) {
 
 #' @export
 roxy_tag_parse.roxy_tag_inheritDotParams <- function(x) {
-  tag_two_part(x, "source", "args", required = FALSE, markdown = FALSE)
+  tag_two_part(x, "a source", "an argument list", required = FALSE, markdown = FALSE)
 }
 #' @export
 roxy_tag_rd.roxy_tag_inheritDotParams <- function(x, base_path, env) {
-  rd_section_inherit_dot_params(x$val$source, x$val$args)
+  rd_section_inherit_dot_params(x$val$name, x$val$description)
 }
 
 #' @export
-roxy_tag_parse.roxy_tag_inheritSection <- function(x) tag_name_description(x)
+roxy_tag_parse.roxy_tag_inheritSection <- function(x) {
+  tag_two_part(x, "a topic name", "a section title")
+}
 #' @export
 roxy_tag_rd.roxy_tag_inheritSection <- function(x, base_path, env) {
   rd_section_inherit_section(x$val$name, x$val$description)

R/rd-params.R

@@ -1,6 +1,6 @@
 #' @export
 roxy_tag_parse.roxy_tag_param <- function(x) {
-  tag_name_description(x)
+  tag_two_part(x, "an argument name", "a description")
 }
 
 #' @export

R/rd-s4.R

@@ -1,5 +1,7 @@
 #' @export
-roxy_tag_parse.roxy_tag_field <- function(x) tag_name_description(x)
+roxy_tag_parse.roxy_tag_field <- function(x) {
+  tag_two_part(x, "a field name", "a description")
+}
 #' @export
 roxy_tag_rd.roxy_tag_field <- function(x, base_path, env) {
   value <- setNames(x$val$description, x$val$name)
@@ -11,7 +13,9 @@ format.rd_section_field <- function(x, ...) {
 }
 
 #' @export
-roxy_tag_parse.roxy_tag_slot <- function(x) tag_name_description(x)
+roxy_tag_parse.roxy_tag_slot <- function(x) {
+  tag_two_part(x, "a slot name", "a description")
+}
 #' @export
 roxy_tag_rd.roxy_tag_slot <- function(x, base_path, env) {
   value <- setNames(x$val$description, x$val$name)

R/rd-template.R

@@ -5,7 +5,7 @@ roxy_tag_parse.roxy_tag_template <- function(x) {
 
 #' @export
 roxy_tag_parse.roxy_tag_templateVar <- function(x) {
-  tag_name_description(x)
+  tag_two_part(x, "a variable name", "a value")
 }
 
 process_templates <- function(block, base_path) {

R/tag-parser.R

@@ -98,10 +98,14 @@ tag_name <- function(x) {
 #' @param markdown Should the second part be parsed as markdown?
 tag_two_part <- function(x, first, second, required = TRUE, markdown = TRUE) {
   if (str_trim(x$raw) == "") {
-    warn_roxy_tag(x, "requires a value")
+    if (!required) {
+      warn_roxy_tag(x, "requires {first}")
+    } else {
+      warn_roxy_tag(x, "requires two parts: {first} and {second}")
+    }
     NULL
   } else if (required && !str_detect(x$raw, "[[:space:]]+")) {
-    warn_roxy_tag(x, "requires {first} and {second}")
+    warn_roxy_tag(x, "requires two parts: {first} and {second}")
     NULL
   } else if (!rdComplete(x$raw, is_code = FALSE)) {
     warn_roxy_tag(x, "has mismatched braces or quotes")
@@ -118,15 +122,15 @@ tag_two_part <- function(x, first, second, required = TRUE, markdown = TRUE) {
       pieces[, 1],
       trim_docstring(pieces[,2])
     )
-    names(x$val) <- c(first, second)
+    names(x$val) <- c("name", "description")
     x
   }
 }
 
 #' @export
 #' @rdname tag_parsers
 tag_name_description <- function(x) {
-  tag_two_part(x, "name", "description")
+  tag_two_part(x, "a name", "a description")
 }
 
 #' @export

tests/testthat/_snaps/rd-describe-in.md

@@ -1,3 +1,11 @@
+# @describeIn suggests @rdname
+
+    Code
+      . <- roc_proc_text(rd_roclet(), block)
+    Message
+      x <text>:2: @describeIn requires a name and description.
+      i Did you want @rdname instead?
+
 # Multiple @describeIn functions combined into one
 
     Code

tests/testthat/_snaps/rd-include-rmd.md

@@ -1,3 +1,10 @@
+# invalid syntax gives useful warning
+
+    Code
+      . <- roc_proc_text(rd_roclet(), block)
+    Message
+      x <text>:2: @includeRmd requires a path.
+
 # useful warnings
 
     Code

tests/testthat/_snaps/rd-inherit.md

@@ -5,6 +5,14 @@
     "sha1"}
     } 
 
+# invalid syntax gives useful warning
+
+    Code
+      . <- roc_proc_text(rd_roclet(), block)
+    Message
+      x <text>:2: @inheritDotParams requires a source.
+      x <text>:3: @inheritSection requires two parts: a topic name and a section title.
+
 # warns on unknown inherit type
 
     Code

tests/testthat/_snaps/rd-params.md

@@ -3,5 +3,5 @@
     Code
       . <- roc_proc_text(rd_roclet(), block)
     Message
-      x <text>:3: @param requires a value.
+      x <text>:3: @param requires two parts: an argument name and a description.
 

tests/testthat/_snaps/rd-s4.md

@@ -0,0 +1,8 @@
+# invalid syntax generates useful warning
+
+    Code
+      . <- roc_proc_text(rd_roclet(), block)
+    Message
+      x <text>:3: @field requires two parts: a field name and a description.
+      x <text>:4: @slot requires two parts: a slot name and a description.
+

tests/testthat/_snaps/rd-template.md

@@ -1,3 +1,10 @@
+# invalid syntax generates useful warning
+
+    Code
+      . <- roc_proc_text(rd_roclet(), block)
+    Message
+      x <text>:3: @templateVar requires two parts: a variable name and a value.
+
 # templates gives useful error if not found
 
     Code

tests/testthat/_snaps/tag-parser.md

@@ -19,18 +19,12 @@
       <message/rlang_message>
       Message:
       x test.R:1: @test requires a value.
-    Code
-      expect_parse_failure(tag_two_part(tag))
-    Output
-      <message/rlang_message>
-      Message:
-      x test.R:1: @test requires a value.
     Code
       expect_parse_failure(tag_name_description(tag))
     Output
       <message/rlang_message>
       Message:
-      x test.R:1: @test requires a value.
+      x test.R:1: @test requires two parts: a name and a description.
     Code
       expect_parse_failure(tag_code(tag))
     Output
@@ -146,12 +140,18 @@
 # tag_two_part() gives useful warnings
 
     Code
-      tag <- roxy_test_tag("a")
-      expect_parse_failure(tag_two_part(tag, "name", "value"))
+      tag <- roxy_test_tag("")
+      expect_parse_failure(tag_two_part(tag, "a name", "a value", required = FALSE))
+    Output
+      <message/rlang_message>
+      Message:
+      x test.R:1: @test requires a name.
+    Code
+      expect_parse_failure(tag_two_part(tag, "a name", "a value"))
     Output
       <message/rlang_message>
       Message:
-      x test.R:1: @test requires name and value.
+      x test.R:1: @test requires two parts: a name and a value.
 
 # tag_words() gives useful warnings
 

tests/testthat/test-rd-describe-in.R

@@ -1,3 +1,11 @@
+test_that("@describeIn suggests @rdname", {
+  block <- "
+    #' @describeIn foo
+    NULL
+  "
+  expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
+})
+
 test_that("@describeIn generic destination captures s3 method source", {
   out <- roc_proc_text(rd_roclet(), "
     #' Title
@@ -252,3 +260,4 @@ test_that("complains about bad usage", {
   "
   expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
 })
+

tests/testthat/test-rd-include-rmd.R

@@ -1,5 +1,13 @@
 skip_if_not_installed("rmarkdown")
 
+test_that("invalid syntax gives useful warning", {
+  block <- "
+    #' @includeRmd
+    NULL
+  "
+  expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
+})
+
 test_that("markdown file can be included", {
   skip_if_not(rmarkdown::pandoc_available("2.17"))
 

tests/testthat/test-rd-inherit.R

@@ -59,6 +59,15 @@ test_that("relative links converted to absolute", {
 
 # tag parsing -------------------------------------------------------------
 
+test_that("invalid syntax gives useful warning", {
+  block <- "
+    #' @inheritDotParams
+    #' @inheritSection
+    NULL
+  "
+  expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
+})
+
 test_that("warns on unknown inherit type", {
   text <- "
     #' @inherit fun blah

tests/testthat/test-rd-s4.R

@@ -1,3 +1,14 @@
+test_that("invalid syntax generates useful warning", {
+  block <- "
+    #' A
+    #' @field
+    #' @slot
+    a <- function() {}
+  "
+
+  expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
+})
+
 test_that("@fields creates a new section and lists fields", {
   out <- roc_proc_text(rd_roclet(), "
     #' Important class.

tests/testthat/test-rd-template.R

@@ -1,3 +1,13 @@
+test_that("invalid syntax generates useful warning", {
+  block <- "
+    #' A
+    #' @templateVar
+    a <- function() {}
+  "
+
+  expect_snapshot(. <- roc_proc_text(rd_roclet(), block))
+})
+
 test_that("can find template from name", {
   base <- test_path("templates/")
 

tests/testthat/test-tag-parser.R

@@ -4,7 +4,6 @@ test_that("tags containing only whitespace generate warning", {
     expect_parse_failure(tag_value(tag))
     expect_parse_failure(tag_inherit(tag))
     expect_parse_failure(tag_name(tag))
-    expect_parse_failure(tag_two_part(tag))
     expect_parse_failure(tag_name_description(tag))
     expect_parse_failure(tag_code(tag))
     expect_parse_failure(tag_examples(tag))
@@ -54,8 +53,9 @@ test_that("tag_name() checks for valid names", {
 test_that("tag_two_part() gives useful warnings", {
   local_markdown()
   expect_snapshot({
-    tag <- roxy_test_tag("a")
-    expect_parse_failure(tag_two_part(tag, "name", "value"))
+    tag <- roxy_test_tag("")
+    expect_parse_failure(tag_two_part(tag, "a name", "a value", required = FALSE))
+    expect_parse_failure(tag_two_part(tag, "a name", "a value"))
   })
 })