Linter for on.exit()

[MichaelChirico]

I think it makes sense to require on.exit() to be called with explicit add= when called multiple times in a function body:

foo <- function(...) {
  c1 <- file(..1)
  on.exit(close(c1))
  writeLines(readLines(c1))

  c1 <- file(..1)
  on.exit(close(c1))
  writeLines(readLines(c1))

  invisible()
}

Almost surely this should be written with on.exit(close(c2), add = TRUE). Better yet in this case is to use {withr}, but this applies to on.exit() generally.

If the author does intend to overwrite on.exit(), the linter encourages doing so explicitly: on.exit(close(c2), add = FALSE).

The difficulty here will be identifying "multiple times". It may not be possible in general, e.g.

foo <- function(...) {
  if (rnorm(1) > 0) {
    c1 <- file(..1)
    on.exit(close(c1))
    writeLines(readLines(c1))
  } else {
    c2 <- file(..2)
    on.exit(close(c2)) # <- no issue!
    writeLines(readLines(c2))
  }
  invisible()
}

This problem only gets worse as further nesting is allowed. Here's an even trickier example:

foo <- function(...) {
  on.exit(message("done"))
  function(...) {
    on.exit(message("goodbye"))
  }
}

That said, I think we can at least come up with some simple rules to start with a high-precision, low-recall linter that's useful.

[AshesITR]

If the on.exit() calls belong to the same expr, there should be a code path that hits both, right? In that case, a following-sibling::SYMBOL_FUNCTION_CALL based XPath should work well.