Merge pull request #33 from tidyverse/master

Move the check on misuse of data.frame() to tests (tidyverse#3758)

Author: sthagen

R/performance.R

@@ -26,13 +26,6 @@ data_frame <- function(...) {
   new_data_frame(list(...))
 }
 
-data.frame <- function(...) {
-  abort(glue("
-    Please use `data_frame()` or `new_data_frame()` instead of `data.frame()` for better performance.
-    See the vignette 'ggplot2 internal programming guidelines' for details.
-  "))
-}
-
 split_matrix <- function(x, col_names = colnames(x)) {
   force(col_names)
   x <- lapply(seq_len(ncol(x)), function(i) x[, i])

tests/testthat/test-conditions.R …sts/testthat/test-prohibited-functions.Rtests/testthat/test-conditions.R renamed to tests/testthat/test-prohibited-functions.R tests/testthat/test-conditions.R renamed to tests/testthat/test-prohibited-functions.R

@@ -10,6 +10,27 @@ get_n_warning <- function(f) {
   sum(d$token == "SYMBOL_FUNCTION_CALL" & d$text == "warning")
 }
 
+get_n_data.frame <- function(f) {
+  d <- getParseData(parse(f, keep.source = TRUE))
+  sum(d$token == "SYMBOL_FUNCTION_CALL" & d$text == "data.frame")
+}
+
+test_that("`get_n_*() detects number of calls properly", {
+  withr::local_file("tmp.R")
+  writeLines(
+    c(
+      'stop("foo!")',
+      'warning("bar!")',
+      "data.frame(x = 1)"
+    ),
+    "tmp.R"
+  )
+
+  expect_equal(get_n_stop("tmp.R"), 1)
+  expect_equal(get_n_warning("tmp.R"), 1)
+  expect_equal(get_n_data.frame("tmp.R"), 1)
+})
+
 # Pattern is needed filter out files such as ggplot2.rdb, which is created when running covr::package_coverage()
 R_files <- list.files("../../R", pattern = ".*\\.(R|r)$", full.names = TRUE)
 
@@ -22,3 +43,8 @@ test_that("do not use warning()", {
   warnings <- vapply(R_files, get_n_warning, integer(1))
   expect_equal(sum(warnings), 0)
 })
+
+test_that("do not use data.frame(), use `data_frame()` or `new_data_frame()`", {
+  data.frames <- vapply(R_files, get_n_data.frame, integer(1))
+  expect_equal(sum(data.frames), 0)
+})