From 556d5dfb34ff75760e664470303c45054b6191af Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 12 Jun 2024 21:49:44 -0700 Subject: [PATCH 1/4] Assume stringsAsFactors=FALSE --- R/get_source_expressions.R | 3 +-- R/linter_tags.R | 8 ++------ R/methods.R | 5 ++--- R/namespace.R | 12 ++++-------- R/object_overwrite_linter.R | 3 +-- R/shared_constants.R | 2 +- tests/testthat/test-fixed_regex_linter.R | 2 +- tests/testthat/test-linter_tags.R | 9 ++------- tests/testthat/test-with.R | 7 +------ 9 files changed, 15 insertions(+), 36 deletions(-) diff --git a/R/get_source_expressions.R b/R/get_source_expressions.R index bc83712d53..ec8a4d4065 100644 --- a/R/get_source_expressions.R +++ b/R/get_source_expressions.R @@ -636,8 +636,7 @@ fix_eq_assigns <- function(pc) { parent = integer(n_expr), token = character(n_expr), terminal = logical(n_expr), - text = character(n_expr), - stringsAsFactors = FALSE + text = character(n_expr) ) for (i in seq_len(n_expr)) { diff --git a/R/linter_tags.R b/R/linter_tags.R index c8cce3fecd..451ddd5651 100644 --- a/R/linter_tags.R +++ b/R/linter_tags.R @@ -82,11 +82,7 @@ available_linters <- function(packages = "lintr", tags = NULL, exclude_tags = "d } build_available_linters <- function(available, package, tags, exclude_tags) { - available_df <- data.frame( - linter = available[["linter"]], - package, - stringsAsFactors = FALSE - ) + available_df <- data.frame(linter = available[["linter"]], package) available_df$tags <- strsplit(available[["tags"]], split = " ", fixed = TRUE) if (!is.null(tags)) { matches_tags <- vapply(available_df$tags, function(linter_tags) any(linter_tags %in% tags), logical(1L)) @@ -108,7 +104,7 @@ build_available_linters <- function(available, package, tags, exclude_tags) { #' `data.frame` constructors don't handle zero-row list-columns properly, so supply `tags` afterwards. #' @noRd empty_linters <- function() { - empty_df <- data.frame(linter = character(), package = character(), stringsAsFactors = FALSE) + empty_df <- data.frame(linter = character(), package = character()) empty_df$tags <- list() empty_df } diff --git a/R/methods.R b/R/methods.R index 4ff71c0a13..46e8ed8045 100644 --- a/R/methods.R +++ b/R/methods.R @@ -163,8 +163,7 @@ as.data.frame.lints <- function(x, row.names = NULL, optional = FALSE, ...) { # type = vapply(x, `[[`, character(1L), "type"), message = vapply(x, `[[`, character(1L), "message"), line = vapply(x, `[[`, character(1L), "line"), - linter = vapply(x, `[[`, character(1L), "linter"), - stringsAsFactors = FALSE + linter = vapply(x, `[[`, character(1L), "linter") ) } @@ -198,7 +197,7 @@ summary.lints <- function(object, ...) { ) tbl <- table(filenames, types) filenames <- rownames(tbl) - res <- as.data.frame.matrix(tbl, stringsAsFactors = FALSE, row.names = NULL) + res <- as.data.frame.matrix(tbl, row.names = NULL) res$filenames <- filenames %||% character() nms <- colnames(res) res[order(res$filenames), c("filenames", nms[nms != "filenames"])] diff --git a/R/namespace.R b/R/namespace.R index a3fba31465..01ae563282 100644 --- a/R/namespace.R +++ b/R/namespace.R @@ -24,18 +24,18 @@ safe_get_exports <- function(ns) { # importFrom directives appear as list(ns, imported_funs) if (length(ns) > 1L) { - return(data.frame(pkg = ns[[1L]], fun = ns[[2L]], stringsAsFactors = FALSE)) + return(data.frame(pkg = ns[[1L]], fun = ns[[2L]])) } # relevant only if there are any exported objects fun <- getNamespaceExports(ns) if (length(fun) > 0L) { - data.frame(pkg = ns, fun = fun, stringsAsFactors = FALSE) + data.frame(pkg = ns, fun = fun) } } empty_namespace_data <- function() { - data.frame(pkg = character(), fun = character(), stringsAsFactors = FALSE) + data.frame(pkg = character(), fun = character()) } # filter namespace_imports() for S3 generics @@ -64,11 +64,7 @@ exported_s3_generics <- function(path = find_package(".")) { return(empty_namespace_data()) } - data.frame( - pkg = basename(path), - fun = unique(namespace_data$S3methods[, 1L]), - stringsAsFactors = FALSE - ) + data.frame(pkg = basename(path), fun = unique(namespace_data$S3methods[, 1L])) } is_s3_generic <- function(fun) { diff --git a/R/object_overwrite_linter.R b/R/object_overwrite_linter.R index 3c068503dd..099129ed52 100644 --- a/R/object_overwrite_linter.R +++ b/R/object_overwrite_linter.R @@ -66,8 +66,7 @@ object_overwrite_linter <- function( ) pkg_exports <- data.frame( package = rep(packages, lengths(pkg_exports)), - name = unlist(pkg_exports), - stringsAsFactors = FALSE + name = unlist(pkg_exports) ) # Take the first among duplicate names, e.g. 'plot' resolves to base::plot, not graphics::plot diff --git a/R/shared_constants.R b/R/shared_constants.R index 5f13715840..3dc20d615b 100644 --- a/R/shared_constants.R +++ b/R/shared_constants.R @@ -115,7 +115,7 @@ get_token_replacement <- function(token_content, token_type) { # r_string gives the operator as you would write it in R code. # styler: off -infix_metadata <- data.frame(stringsAsFactors = FALSE, matrix(byrow = TRUE, ncol = 2L, c( +infix_metadata <- data.frame(matrix(byrow = TRUE, ncol = 2L, c( "OP-PLUS", "+", "OP-MINUS", "-", "OP-TILDE", "~", diff --git a/tests/testthat/test-fixed_regex_linter.R b/tests/testthat/test-fixed_regex_linter.R index 3718482005..7f4075809d 100644 --- a/tests/testthat/test-fixed_regex_linter.R +++ b/tests/testthat/test-fixed_regex_linter.R @@ -68,7 +68,7 @@ patrick::with_parameters_test_that( "|", "[", "]", "\\\\", "<", ">", "=", ":", ";", "/", "_", "-", "!", "@", "#", "%", "&", "~" ) - data.frame(char = char, .test_name = char, stringsAsFactors = FALSE) + data.frame(char = char, .test_name = char) }) ) diff --git a/tests/testthat/test-linter_tags.R b/tests/testthat/test-linter_tags.R index 985cdb30d9..624407ea23 100644 --- a/tests/testthat/test-linter_tags.R +++ b/tests/testthat/test-linter_tags.R @@ -13,11 +13,7 @@ test_that("validate_linter_db works as expected", { ) expect_false(suppressWarnings(lintr:::validate_linter_db(df_empty, "mypkg"))) - df <- data.frame( - linter = "absolute_path_linter", - tags = "robustness", - stringsAsFactors = FALSE - ) + df <- data.frame(linter = "absolute_path_linter", tags = "robustness") expect_true(lintr:::validate_linter_db(df, "mypkg")) }) @@ -162,8 +158,7 @@ test_that("lintr help files are up to date", { # Counts of tags from available_linters() db_tag_table <- as.data.frame( table(tag = unlist(lintr_db$tags)), - responseName = "n_linters", - stringsAsFactors = FALSE + responseName = "n_linters" ) # In ?linters, entries in the enumeration of tags look like # \item{\link[=${TAG}_linters]{${TAG}} (${N_LINTERS_WITH_TAG} linters)} diff --git a/tests/testthat/test-with.R b/tests/testthat/test-with.R index 5ab2560ade..fff11bced4 100644 --- a/tests/testthat/test-with.R +++ b/tests/testthat/test-with.R @@ -24,12 +24,7 @@ test_that("linters_with_defaults warns on unused NULLs", { test_that("linters_with_tags() verifies the output of available_linters()", { local_mocked_bindings( available_linters = function(...) { - data.frame( - linter = c("fake_linter", "very_fake_linter"), - package = "lintr", - tags = "", - stringsAsFactors = FALSE - ) + data.frame(linter = c("fake_linter", "very_fake_linter"), package = "lintr", tags = "") } ) expect_error( From 06e3412a5a6f77876951d8c6a8cb1699f363c8da Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 12 Jun 2024 21:55:41 -0700 Subject: [PATCH 2/4] disable corresponding linter --- .lintr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.lintr b/.lintr index 0542b9eeec..0d4e1af9f7 100644 --- a/.lintr +++ b/.lintr @@ -28,6 +28,8 @@ linters: all_linters( absolute_path_linter = NULL, library_call_linter = NULL, nonportable_path_linter = NULL, + # We now require R>=4.0.0 + strings_as_factors_linter = NULL, todo_comment_linter = NULL, # TODO(#2327): Enable this. unreachable_code_linter = NULL From 6bcfc5a5f1d12c7bec988bc1ca1153e77a2bda4a Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 12 Jun 2024 22:02:14 -0700 Subject: [PATCH 3/4] revert one change, as.data.frame.table keeps old sAF=TRUE --- tests/testthat/test-linter_tags.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-linter_tags.R b/tests/testthat/test-linter_tags.R index 624407ea23..2d7f538106 100644 --- a/tests/testthat/test-linter_tags.R +++ b/tests/testthat/test-linter_tags.R @@ -156,9 +156,11 @@ test_that("lintr help files are up to date", { ) # Counts of tags from available_linters() + # NB: as.data.frame.table returns stringsAsFactors=TRUE default in R>4 db_tag_table <- as.data.frame( table(tag = unlist(lintr_db$tags)), - responseName = "n_linters" + responseName = "n_linters", + stringsAsFactors = FALSE ) # In ?linters, entries in the enumeration of tags look like # \item{\link[=${TAG}_linters]{${TAG}} (${N_LINTERS_WITH_TAG} linters)} From 6e394649accea8760b75b19ac5ec628d6243df99 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 14 Jun 2024 22:40:13 -0700 Subject: [PATCH 4/4] more call sites --- tests/testthat/test-methods.R | 5 ++--- vignettes/lintr.Rmd | 8 ++------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/testthat/test-methods.R b/tests/testthat/test-methods.R index 9f454c1f47..172b9692d4 100644 --- a/tests/testthat/test-methods.R +++ b/tests/testthat/test-methods.R @@ -56,14 +56,13 @@ test_that("as.data.frame.lints", { expect_s3_class(df, "data.frame") exp <- data.frame( - filename = rep("dummy.R", 2L), + filename = "dummy.R", line_number = c(1L, 2L), column_number = c(1L, 6L), type = c("style", "error"), message = c("", "Under no circumstances is the use of foobar allowed."), line = c("", "a <- 1"), - linter = c(NA_character_, NA_character_), # These are assigned in lint() now. - stringsAsFactors = FALSE + linter = NA_character_ # These are assigned in lint() now. ) expect_identical(df, exp) diff --git a/vignettes/lintr.Rmd b/vignettes/lintr.Rmd index e6c4a1da03..b819c9a12c 100644 --- a/vignettes/lintr.Rmd +++ b/vignettes/lintr.Rmd @@ -140,10 +140,7 @@ make_string <- function(x) { } } -defaults_table <- data.frame( - default = vapply(default_settings, make_string, character(1L)), - stringsAsFactors = FALSE -) +defaults_table <- data.frame(default = vapply(default_settings, make_string, character(1L))) # avoid conflict when loading lintr in echo=TRUE cell below rm(default_settings) @@ -196,8 +193,7 @@ make_setting_string <- function(linter_name) { defaults_table <- data.frame( row.names = names(default_linters), - settings = vapply(names(default_linters), make_setting_string, character(1L)), - stringsAsFactors = FALSE + settings = vapply(names(default_linters), make_setting_string, character(1L)) ) knitr::kable(defaults_table)