Drop parse_all() default method; add tests

NAMESPACE

@@ -4,7 +4,6 @@ S3method(parse_all,"function")
 S3method(parse_all,call)
 S3method(parse_all,character)
 S3method(parse_all,connection)
-S3method(parse_all,default)
 S3method(print,evaluate_evaluation)
 S3method(replay,character)
 S3method(replay,condition)

NEWS.md

@@ -1,5 +1,6 @@
 # evaluate (development version)
 
+* `parse_all()` no longer has a default method, which will generate better errors if you pass in something unexpectected.
 * The package now depends on R 4.0.0 in order to decrease our maintenance burden.
 * `evaluate()` automatically strips calls from conditions emitted by top-level code (these incorrectly get calls because they're wrapped inside `eval()`) (#150).
 * `evalute(include_timing)` has been deprecated. I can't find any use of it on GitHub, and it adds substantial code complexity for little gain.

R/parse.R

@@ -131,12 +131,12 @@ append_break <- function(x) {
 #' @export
 parse_all.connection <- function(x, filename = NULL, ...) {
   if (!isOpen(x, "r")) {
-      open(x, "r")
-      on.exit(close(x))
+    open(x, "r")
+    defer(close(x))
   }
   text <- readLines(x)
-  if (is.null(filename))
-    filename <- summary(x)$description
+  filename <- filename %||% summary(x)$description
+
   parse_all(text, filename, ...)
 }
 
@@ -146,19 +146,10 @@ parse_all.function <- function(x, filename = NULL, ...) {
   parse_all(find_function_body(x), filename = filename, ...)
 }
 
-#' @export
-parse_all.default <- function(x, filename = NULL, ...) {
-  if (is.null(filename))
-    filename <- "<expression>"
-  parse_all(deparse(x), filename, ...)
-}
-
 # Calls are already parsed and always length one
 #' @export
 parse_all.call <- function(x, filename = NULL, ...) {
-  out <- parse_all.default(x, filename = filename, ...)
-  out$expr <- list(as.expression(x))
-  out
+  parse_all(deparse(x), filename = filename, ...)
 }
 
 find_function_body <- function(f) {

tests/testthat/test-parse.R

@@ -30,6 +30,49 @@ test_that("can ignore parse errors", {
   expect_error(evaluate('x <-', stop_on_error = 0), NA)
 })
 
+# input types ------------------------------------------------------------------
+
+test_that("can parse a call", {
+  out <- parse_all(quote(f(a, b, c)))
+  expect_equal(out$src, "f(a, b, c)")
+  expect_equal(
+    out$expr,
+    I(list(expression(f(a, b, c)))),
+    ignore_attr = "srcref"
+  )
+})
+
+test_that("can parse a connection", {
+  path <- withr::local_tempfile(lines = c("# 1", "1 + 1"))
+  cur_cons <- getAllConnections()
+
+  con <- file(path)
+  out <- parse_all(con)
+
+  expect_equal(out$src, c("# 1\n", "1 + 1"))
+  expect_equal(
+    out$expr,
+    I(list(expression(), expression(1 + 1))),
+    ignore_attr = "srcref"
+  )
+
+  # Doesn't leave any connections around
+  expect_equal(getAllConnections(), cur_cons)
+})
+
+test_that("can parse a function", {
+  out <- parse_all(function() {
+    # Hi
+    1 + 1
+  })
+  expect_equal(out$src, c("# Hi\n", "1 + 1"))
+  expect_equal(
+    out$expr,
+    I(list(expression(), expression(1 + 1))),
+    ignore_attr = "srcref"
+  )
+})
+
 # find_function_body -----------------------------------------------------------
 
 test_that("parsing a function parses its body", {