Include line information for tags in roxy_block

[jimhester]

For ropensci/spelling#3 it would be useful to have the start line per tag in the roxy_block() objects which are the result of parse_file(). This can be done with a trivial change

diff --git a/R/block.R b/R/block.R
index 350fd1e..ce2b5cd 100644
--- a/R/block.R
+++ b/R/block.R
@@ -163,6 +163,10 @@ parse_tags <- function(tokens, registry = list(), global_options = list()) {
   # it's what roxygen already uses
   vals <- lapply(tags, `[[`, "val")
   names <- vapply(tags, `[[`, "tag", FUN.VALUE = character(1))
+  for (i in seq_along(tags)) {
+    attr(vals[[i]], "line") <- tags[[i]]$line
+  }
+
   setNames(vals, names)
 }

However doing so reveals that the line numbers do not seem quite correct. Running parse_file() on a simple file reveals a couple of issues.

cat(
"#' Foo
#'
#' A simple
#' multiline description.
#'
#' @param x xyz
#' @export
NULL", "test.R")

res <- roxygen2::parse_file("test.R")

str(res)

#> List of 1
#>  $ :List of 4
#>   ..$ title      : atomic [1:1] Foo
#>   .. ..- attr(*, "line")= int 1
#>   ..$ description: atomic [1:1] A simple
#> multiline description.
#>   .. ..- attr(*, "line")= int 1
#>   ..$ param      :List of 2
#>   .. ..$ name       : chr "x"
#>   .. ..$ description: chr "xyz"
#>   .. ..- attr(*, "line")= int 7
#>   ..$ export     : atomic [1:1]
#>   .. ..- attr(*, "line")= int 8
#>   ..- attr(*, "filename")= chr "test.R"
#>   ..- attr(*, "location")= int [1:8] 8 1 8 4 1 4 8 8
#>   ..- attr(*, "class")= chr "roxy_block"

1. The start line of the description should be at least 2 (ideally 3), it is 1.
2. The start lines of the param and export tags are 1 higher than they should be. They are 7 and 8 when they occur on lines 6 and 7 of the file.

[hadley]

This change also breaks a very large number of unit tests. Probably worth doing, but will require careful planning.

[hadley]

The first problem needs careful thought in parse_description() replacing the use of intro$line everywhere with a more accurate offset. I think cumsum(map_int(paragraphs, str_count, "\n")) gets you 90% of the way there (just need to think about the lines eaten by the paragraph separators).

The second problem seems to mostly be an off by one error in the C++ code (although it needs a little more thought for when the first element isn't a tag).

I think the biggest payoff to roxygen2 will be to replace many uses of block_warning() with something that can give a specific line number. That will also require saving the filename, so maybe it makes sense to do something more like:

for (i in seq_along(tags)) {
  attr(vals[[i]], "tag") <- tags[[i]]
}

Alternatively, it might be worth rethinking the object structure here, and just return a list of tags. This will require touching code in a lot of places, but it's likely to be simple changes, and code coverage is good enough that we're unlikely to miss anything.

[hadley]

Will also need to update extension vignette, and polish roxy_block and roxy_tag print methods. That would also make debugging substantially easier. (Probably needs at least a full day set aside to the fixes)

[hadley]

I've moved the description line number issue to #917, since I can now make a nice reprex for it, and tomorrow I'll be merging a massive commit to resolve this issue.

[jimhester]

Wow, that was a massive commit!

[hadley]

That's what you get for requesting a change to the data structure at the heart of an 8 year old R package 😛