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

[yutannihilation]

#3526 introduced the static analysis of codes here.

ggplot2/tests/testthat/test-conditions.R

Lines 3 to 21 in 214f314

	get_n_stop <- function(f) {
	d <- getParseData(parse(f, keep.source = TRUE))
	sum(d$token == "SYMBOL_FUNCTION_CALL" & d$text == "stop")
	}
	get_n_warning <- function(f) {
	d <- getParseData(parse(f, keep.source = TRUE))
	sum(d$token == "SYMBOL_FUNCTION_CALL" & d$text == "warning")
	}
	test_that("do not use stop()", {
	stops <- vapply(list.files("../../R", full.names = TRUE), get_n_stop, integer(1))
	expect_equal(sum(stops), 0)
	})
	test_that("do not use warning()", {
	warnings <- vapply(list.files("../../R", full.names = TRUE), get_n_warning, integer(1))
	expect_equal(sum(warnings), 0)
	})

This approach seems very clean and extensive because this catches the misuse of the prohibited function even if it's on the rare code path. I think the check on data.frame() should be done in the same way instead of overwriting the function. One bonus of this is that we'll be less likely to face such mysterious issues as #3745.

[clauswilke]

I like this approach, but I'd like to suggest that we should also test the test (i.e., include a true positive). Add a file to the testthat directory that contains the prohibited functions and check that the counts of prohibited functions for that file are correct.

[yutannihilation]

Thanks, I added some simple tests.

[thomasp85]

@yutannihilation can we merge this in in its current form?

[yutannihilation]

Yes, since this is just about testing, I think it's safe to merge this.

[thomasp85]

LGTM

[yutannihilation]

Thanks