Skip to content

Commit b178d22

Browse files
Fix tests/testthat/test-prohibited-functions.R (#4446)
* Do not raise error for `base::data.frame()` * Add a failing test * Add a path for R CMD check
1 parent eb91826 commit b178d22

File tree

1 file changed

+20
-4
lines changed

1 file changed

+20
-4
lines changed

tests/testthat/test-prohibited-functions.R

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,14 @@ get_n_warning <- function(f) {
1212

1313
get_n_data.frame <- function(f) {
1414
d <- getParseData(parse(f, keep.source = TRUE))
15-
sum(d$token == "SYMBOL_FUNCTION_CALL" & d$text == "data.frame")
15+
16+
idx_base <- d$token == "SYMBOL_PACKAGE" & d$text == "base"
17+
idx_colons <- d$token == "NS_GET" & d$text == "::"
18+
# exclude the case when the `data.frame` is prefixed with `base::`
19+
idx_base_prefixed <- c(FALSE, FALSE, idx_base[1:(nrow(d) - 2)]) & c(FALSE, idx_colons[1:(nrow(d) - 1)])
20+
21+
idx_data.frame <- d$token == "SYMBOL_FUNCTION_CALL" & d$text == "data.frame"
22+
sum(idx_data.frame & !idx_base_prefixed)
1623
}
1724

1825
test_that("`get_n_*() detects number of calls properly", {
@@ -21,7 +28,8 @@ test_that("`get_n_*() detects number of calls properly", {
2128
c(
2229
'stop("foo!")',
2330
'warning("bar!")',
24-
"data.frame(x = 1)"
31+
"data.frame(x = 1)",
32+
"base::data.frame(x = 1)" # this is not counted
2533
),
2634
"tmp.R"
2735
)
@@ -32,7 +40,15 @@ test_that("`get_n_*() detects number of calls properly", {
3240
})
3341

3442
# Pattern is needed filter out files such as ggplot2.rdb, which is created when running covr::package_coverage()
35-
R_files <- list.files("../../R", pattern = ".*\\.(R|r)$", full.names = TRUE)
43+
R_paths <- c(
44+
"../../R", # in the case of devtools::test()
45+
"../../00_pkg_src/ggplot2/R" # in the case of R CMD check
46+
)
47+
R_files <- list.files(R_paths, pattern = ".*\\.(R|r)$", full.names = TRUE)
48+
49+
test_that("list up R files properly", {
50+
expect_true(length(R_files) > 0)
51+
})
3652

3753
test_that("do not use stop()", {
3854
stops <- vapply(R_files, get_n_stop, integer(1))
@@ -44,7 +60,7 @@ test_that("do not use warning()", {
4460
expect_equal(sum(warnings), 0)
4561
})
4662

47-
test_that("do not use data.frame(), use `data_frame()` or `new_data_frame()`", {
63+
test_that("do not use data.frame(), use `data_frame()` or `new_data_frame()`, or add `base::` prefix", {
4864
data.frames <- vapply(R_files, get_n_data.frame, integer(1))
4965
expect_equal(sum(data.frames), 0)
5066
})

0 commit comments

Comments
 (0)