diff --git a/.dev/unused_helpers_test.R b/.dev/unused_helpers_test.R new file mode 100644 index 000000000..413853595 --- /dev/null +++ b/.dev/unused_helpers_test.R @@ -0,0 +1,55 @@ +# Look for unexported objects which are not used internally +# TODO(#1513): delete this after we have a dedicated linter for it + +ns_contents <- parseNamespaceFile("lintr", "..") + +export_names <- ns_contents$exports +registered_s3_methods <- apply(ns_contents$S3methods[, 1L:2L], 1L, paste, collapse = ".") + +pkgload::load_all() + +internal_names <- setdiff( + ls(asNamespace("lintr"), all.names = TRUE), + c(export_names, registered_s3_methods) +) + +# .__global__, etc. +internal_names <- grepv("^[.]__", internal_names, invert = TRUE) +# other generic R names +internal_names <- setdiff(internal_names, c(".onLoad", ".onAttach", ".packageName")) +# known false positives +# - %||% is a mask for certain versions of R +# - addin_lint* are used by RStudio even though they're not exported +# - regexes_rd is used in ?object_name_linter by an \Sexpr execution +internal_names <- setdiff(internal_names, c("%||%", "addin_lint", "addin_lint_package", "regexes_rd")) + +is_operator <- make.names(internal_names) != internal_names + +operator_linter <- undesirable_operator_linter(internal_names[is_operator]) +function_linter <- undesirable_function_linter(internal_names[!is_operator]) + +# suppressWarnings: #nolint for linters not present here +usages <- as.data.frame(suppressWarnings(lint_dir("R", linters = list(operator_linter, function_linter)))) +usages$used_call <- gsub('.*["`]([^"`]*)["`].*', "\\1", usages$message) +# TODO(#2815): these should be excluded by the linter itself +usages <- subset(usages, !mapply(\(nm, l) grepl(sprintf("\\b%s <-", rex::rex(nm)), l), used_call, line)) + +# TODO(#2004): can this just come from get_source_expressions(), and/or will +# the above linters "just work" once we're roxygen2-aware? +options(keep.source = TRUE) +roxy_usage <- character() +for (robj in roxygen2::parse_package()) { + for (tag in robj$tags) { + if (tag$tag == "evalRd") { + roxy_usage <- c(roxy_usage, + subset(getParseData(tag$val), token == "SYMBOL_FUNCTION_CALL", select = "text", drop = TRUE) + ) + } + } +} + +unused_names <- setdiff(internal_names, c(usages$used_call, roxy_usage)) + +if (length(unused_names) > 0L) { + stop("These objects are in the {lintr} namespace but don't have any usage: ", toString(unused_names)) +} diff --git a/.github/workflows/repo-meta-tests.yaml b/.github/workflows/repo-meta-tests.yaml index 057cce7e4..57f92613f 100644 --- a/.github/workflows/repo-meta-tests.yaml +++ b/.github/workflows/repo-meta-tests.yaml @@ -45,3 +45,8 @@ jobs: run: | callr::rscript(".dev/defunct_linters_test.R") shell: Rscript {0} + + - name: Ensure any objects in the namespace are actually needed + run: | + callr::rscript(".dev/unused_helpers_test.R") + shell: Rscript {0} diff --git a/R/methods.R b/R/methods.R index ff5558843..c7bad5be9 100644 --- a/R/methods.R +++ b/R/methods.R @@ -85,39 +85,6 @@ print.lints <- function(x, ...) { invisible(x) } -trim_output <- function(x, max = 65535L) { - # if x is less than the max, just return it - if (length(x) <= 0L || nchar(x) <= max) { - return(x) - } - - # otherwise trim x to the max, then search for the lint starts - x <- substr(x, 1L, max) - - re <- rex( - "[", except_some_of(":"), ":", numbers, ":", numbers, ":", "]", - "(", except_some_of(")"), ")", - space, - "*", or("style", "warning", "error"), ":", "*", - except_some_of("\r\n"), newline, - except_some_of("\r\n"), newline, - except_some_of("\r\n"), newline, - except_some_of("\r\n"), newline, - except_some_of("\r\n"), newline - ) - - lint_starts <- re_matches(x, re, global = TRUE, locations = TRUE)[[1L]] - - # if at least one lint ends before the cutoff, cutoff there, else just use - # the cutoff - last_end <- lint_starts[NROW(lint_starts), "end"] - if (!is.na(last_end)) { - substr(x, 1L, last_end) - } else { - x - } -} - #' @export names.lints <- function(x, ...) { vapply(x, `[[`, character(1L), "filename") diff --git a/R/path_utils.R b/R/path_utils.R index 569333095..220fb206e 100644 --- a/R/path_utils.R +++ b/R/path_utils.R @@ -57,10 +57,6 @@ is_root_path <- function(path) { re_matches(path, root_path_regex) } -is_relative_path <- function(path) { - re_matches(path, relative_path_regex) -} - is_path <- function(path) { re_matches(path, path_regex) } diff --git a/R/utils.R b/R/utils.R index c2b861f47..a6107424c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -166,11 +166,6 @@ read_lines <- function(file, encoding = settings$encoding, ...) { lines } -# nocov start -# support for usethis::use_release_issue(). Make sure to use devtools::load_all() beforehand! -release_bullets <- function() {} -# nocov end - # see issue #923, PR #2455 -- some locales ignore _ when running sort(), others don't. # We want to consistently treat "_" < "n" = "N"; C locale does this, which 'radix' uses. platform_independent_order <- function(x) order(tolower(x), method = "radix") diff --git a/tests/testthat/test-absolute_path_linter.R b/tests/testthat/test-absolute_path_linter.R index bff94520c..84e58c4b8 100644 --- a/tests/testthat/test-absolute_path_linter.R +++ b/tests/testthat/test-absolute_path_linter.R @@ -52,28 +52,6 @@ test_that("is_absolute_path", { expect_identical(f(x), y) }) - -test_that("is_relative_path", { - f <- lintr:::is_relative_path - - x <- character() - y <- logical() - expect_identical(f(x), y) - - x <- c("/", "c:\\", "~/", "foo", "http://rseek.org/", "'./'") - y <- c(FALSE, FALSE, FALSE, FALSE, FALSE, FALSE) - expect_identical(f(x), y) - - x <- c("/foo", "foo/", "foo/bar", "foo//bar", "./foo", "../foo") - y <- c(FALSE, TRUE, TRUE, TRUE, TRUE, TRUE) - expect_identical(f(x), y) - - x <- c("\\\\", "\\foo", "foo\\", "foo\\bar", ".\\foo", "..\\foo", ".", "..", "../") - y <- c(FALSE, FALSE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE) - expect_identical(f(x), y) -}) - - test_that("is_path", { f <- lintr:::is_path diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 91a04cafc..f8aef14e1 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -1,23 +1,3 @@ -test_that("it returns the input if less than the max", { - expect_identical(lintr:::trim_output(character()), character()) - expect_identical(lintr:::trim_output("test", max = 10L), "test") -}) - -test_that("it returns the input trimmed strictly to max if no lints found", { - expect_identical(lintr:::trim_output("testing a longer non_lint string", max = 7L), "testing") -}) - -test_that("it returns the input trimmed to the last full lint if one exists within the max", { - t1 <- readChar(test_path("lints"), file.size(test_path("lints"))) - if (.Platform$OS.type == "windows") { - # Magic numbers expect newlines to be 1 character - t1 <- gsub("\r\n", "\n", t1, fixed = TRUE) - } - expect_identical(lintr:::trim_output(t1, max = 200L), substr(t1, 1L, 195L)) - expect_identical(lintr:::trim_output(t1, max = 400L), substr(t1, 1L, 380L)) - expect_identical(lintr:::trim_output(t1, max = 2000L), substr(t1, 1L, 1930L)) -}) - test_that("as.data.frame.lints", { l1 <- Lint( "dummy.R",