From e53f149ae28a096405785cec0756a3b71c866f34 Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Mon, 10 Feb 2025 12:51:56 +0800 Subject: [PATCH 1/6] delete cran-comments, there's no need any longer --- cran-comments.md | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100644 cran-comments.md diff --git a/cran-comments.md b/cran-comments.md deleted file mode 100644 index 87dd8b6..0000000 --- a/cran-comments.md +++ /dev/null @@ -1,17 +0,0 @@ -# R CMD check results - -0 errors | 0 warnings | 2 notes - -## Fixes from previous CRAN version - -I had asked that the last version of this package on CRAN be removed, but the -notes indicate it was forcibly removed for "leaving things behind". - -Therefore, in this release, the caching capabilities now follow the new R -standard `tools::R_user_dir(package = "getCRUCLdata")` in spite of the default -behaviour of the underlying CRAN packages, {hoadr} and {rappdirs}, which have -been used since this package was first submitted to CRAN to provide file -caching functionality in this package. I trust that this is sufficient to have -this package reinstated on CRAN. - -- This is a new major release. From fd66773d062ca5fb618d709a0f9c950927e9585d Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Mon, 3 Mar 2025 13:52:31 +0800 Subject: [PATCH 2/6] Format with Air and desc::desc_normalize() --- DESCRIPTION | 8 +- R/create_CRU_df.R | 31 +++--- R/create_CRU_stack.R | 31 +++--- R/get_CRU.R | 60 +++++------ R/get_CRU_df.R | 28 +++--- R/get_CRU_stack.R | 28 +++--- R/internal_functions.R | 132 +++++++++++++------------ R/zzz.R | 3 +- tests/spelling.R | 3 +- tests/testthat/test-create_CRU_stack.R | 44 +++------ tests/testthat/test-create_stack.R | 62 +++++------- tests/testthat/test-get_CRU.R | 1 - tests/testthat/test_caching.R | 8 +- tests/testthat/test_get_local.R | 12 +-- vignettes/precompile.R | 6 +- 15 files changed, 213 insertions(+), 244 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index ecae0ea..588e9e7 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -7,7 +7,7 @@ Authors@R: c( comment = c(ORCID = "0000-0002-0061-8359")), person("Curtin University of Technology", role = "cph", comment = "Provided support through Adam Sparks's time."), - person("Grains Research and Development Corporation", role = c("cph"), + person("Grains Research and Development Corporation", role = "cph", comment = "GRDC Project CUR2210-005OPX (AAGI-CU)") ) Description: Provides functions that automate downloading and importing @@ -49,6 +49,7 @@ Suggests: VignetteBuilder: knitr ByteCompile: TRUE +Config/Needs/build: moodymudskipper/devtag Config/roxyglobals/filename: globals.R Config/roxyglobals/unique: FALSE Config/roxylint: list(linters = roxylint::tidy) @@ -56,11 +57,12 @@ Config/testthat/edition: 3 Config/testthat/parallel: true Encoding: UTF-8 Language: en-US -Roxygen: list(markdown = TRUE, roclets = c("collate", "namespace", "rd", "roxyglobals::global_roclet", "roxylint::roxylint", "devtag::dev_roclet")) +Roxygen: list(markdown = TRUE, roclets = c("collate", "namespace", "rd", + "roxyglobals::global_roclet", "roxylint::roxylint", + "devtag::dev_roclet")) RoxygenNote: 7.3.2 X-schema.org-applicationCategory: Tools X-schema.org-isPartOf: https://ropensci.org X-schema.org-keywords: anglia-cru, climate-data, cru-cl2, temperature, rainfall, elevation, data-access, wind, relative-humidity, solar-radiation, diurnal-temperature, frost -Config/Needs/build: moodymudskipper/devtag diff --git a/R/create_CRU_df.R b/R/create_CRU_df.R index 0535199..1c0ad4e 100644 --- a/R/create_CRU_df.R +++ b/R/create_CRU_df.R @@ -83,19 +83,21 @@ #' #' @export -create_CRU_df <- function(pre = FALSE, - pre_cv = FALSE, - rd0 = FALSE, - tmp = FALSE, - dtr = FALSE, - reh = FALSE, - tmn = FALSE, - tmx = FALSE, - sunp = FALSE, - frs = FALSE, - wnd = FALSE, - elv = FALSE, - dsn) { +create_CRU_df <- function( + pre = FALSE, + pre_cv = FALSE, + rd0 = FALSE, + tmp = FALSE, + dtr = FALSE, + reh = FALSE, + tmn = FALSE, + tmx = FALSE, + sunp = FALSE, + frs = FALSE, + wnd = FALSE, + elv = FALSE, + dsn +) { .check_vars_FALSE( pre, pre_cv, @@ -113,7 +115,8 @@ create_CRU_df <- function(pre = FALSE, .validate_dsn(dsn) - files <- .get_local(pre, + files <- .get_local( + pre, pre_cv, rd0, tmp, diff --git a/R/create_CRU_stack.R b/R/create_CRU_stack.R index b921d1c..cf16b95 100644 --- a/R/create_CRU_stack.R +++ b/R/create_CRU_stack.R @@ -37,19 +37,21 @@ #' #' @export -create_CRU_stack <- function(pre = FALSE, - pre_cv = FALSE, - rd0 = FALSE, - tmp = FALSE, - dtr = FALSE, - reh = FALSE, - tmn = FALSE, - tmx = FALSE, - sunp = FALSE, - frs = FALSE, - wnd = FALSE, - elv = FALSE, - dsn) { +create_CRU_stack <- function( + pre = FALSE, + pre_cv = FALSE, + rd0 = FALSE, + tmp = FALSE, + dtr = FALSE, + reh = FALSE, + tmn = FALSE, + tmx = FALSE, + sunp = FALSE, + frs = FALSE, + wnd = FALSE, + elv = FALSE, + dsn +) { .check_vars_FALSE( pre, pre_cv, @@ -67,7 +69,8 @@ create_CRU_stack <- function(pre = FALSE, .validate_dsn(dsn) - files <- .get_local(pre, + files <- .get_local( + pre, pre_cv, rd0, tmp, diff --git a/R/get_CRU.R b/R/get_CRU.R index a2565ca..d160423 100644 --- a/R/get_CRU.R +++ b/R/get_CRU.R @@ -21,19 +21,21 @@ #' #' @dev .get_CRU <- - function(pre, - pre_cv, - rd0, - tmp, - dtr, - reh, - tmn, - tmx, - sunp, - frs, - wnd, - elv, - cache_dir) { + function( + pre, + pre_cv, + rd0, + tmp, + dtr, + reh, + tmn, + tmx, + sunp, + frs, + wnd, + elv, + cache_dir + ) { dtr_file <- "grid_10min_dtr.dat.gz" tmp_file <- "grid_10min_tmp.dat.gz" reh_file <- "grid_10min_reh.dat.gz" @@ -70,17 +72,17 @@ ) names(files) <- names(object_list) <- - c( - "dtr_file", - "tmp_file", - "reh_file", - "elv_file", - "pre_file", - "sun_file", - "wnd_file", - "frs_file", - "rd0_file" - ) + c( + "dtr_file", + "tmp_file", + "reh_file", + "elv_file", + "pre_file", + "sun_file", + "wnd_file", + "frs_file", + "rd0_file" + ) # filter downloaded ------------------------------------------------------- # which files are being requested? @@ -108,17 +110,17 @@ }, error = function(x) { manage_cache$delete_all() - cli::cli_abort("The file downloads have failed. - Please start the download again.") + cli::cli_abort( + "The file downloads have failed. + Please start the download again." + ) } ) } # filter files from cache directory in case there are local files for which # we do not want data - cache_dir_contents <- as.list(list.files(cache_dir, - pattern = ".dat.gz$" - )) + cache_dir_contents <- as.list(list.files(cache_dir, pattern = ".dat.gz$")) files <- cache_dir_contents[cache_dir_contents %in% files] diff --git a/R/get_CRU_df.R b/R/get_CRU_df.R index 527afb2..b330bf3 100644 --- a/R/get_CRU_df.R +++ b/R/get_CRU_df.R @@ -75,19 +75,21 @@ #' #' @export -get_CRU_df <- function(pre = FALSE, - pre_cv = FALSE, - rd0 = FALSE, - tmp = FALSE, - dtr = FALSE, - reh = FALSE, - tmn = FALSE, - tmx = FALSE, - sunp = FALSE, - frs = FALSE, - wnd = FALSE, - elv = FALSE, - cache = FALSE) { +get_CRU_df <- function( + pre = FALSE, + pre_cv = FALSE, + rd0 = FALSE, + tmp = FALSE, + dtr = FALSE, + reh = FALSE, + tmn = FALSE, + tmx = FALSE, + sunp = FALSE, + frs = FALSE, + wnd = FALSE, + elv = FALSE, + cache = FALSE +) { .check_vars_FALSE( pre, pre_cv, diff --git a/R/get_CRU_stack.R b/R/get_CRU_stack.R index a935273..4281f9c 100644 --- a/R/get_CRU_stack.R +++ b/R/get_CRU_stack.R @@ -29,19 +29,21 @@ #' @export get_CRU_stack <- - function(pre = FALSE, - pre_cv = FALSE, - rd0 = FALSE, - tmp = FALSE, - dtr = FALSE, - reh = FALSE, - tmn = FALSE, - tmx = FALSE, - sunp = FALSE, - frs = FALSE, - wnd = FALSE, - elv = FALSE, - cache = FALSE) { + function( + pre = FALSE, + pre_cv = FALSE, + rd0 = FALSE, + tmp = FALSE, + dtr = FALSE, + reh = FALSE, + tmn = FALSE, + tmx = FALSE, + sunp = FALSE, + frs = FALSE, + wnd = FALSE, + elv = FALSE, + cache = FALSE + ) { .check_vars_FALSE( pre, pre_cv, diff --git a/R/internal_functions.R b/R/internal_functions.R index ef6f8de..13635ce 100644 --- a/R/internal_functions.R +++ b/R/internal_functions.R @@ -44,19 +44,20 @@ #' ) #' @dev - -.check_vars_FALSE <- function(pre, - pre_cv, - rd0, - tmp, - dtr, - reh, - tmn, - tmx, - sunp, - frs, - wnd, - elv) { +.check_vars_FALSE <- function( + pre, + pre_cv, + rd0, + tmp, + dtr, + reh, + tmn, + tmx, + sunp, + frs, + wnd, + elv +) { if (!any(pre, pre_cv, rd0, tmp, dtr, reh, tmn, tmx, sunp, frs, wnd, elv)) { cli::cli_abort( "You must select at least one element for download or import.", @@ -82,14 +83,17 @@ dsn <- trimws(dsn) if (substr(dsn, nchar(dsn) - 1, nchar(dsn)) == "//") { p <- substr(dsn, 1, nchar(dsn) - 2) - } else if (substr(dsn, nchar(dsn), nchar(dsn)) == "/" | - substr(dsn, nchar(dsn), nchar(dsn)) == "\\") { + } else if ( + substr(dsn, nchar(dsn), nchar(dsn)) == "/" | + substr(dsn, nchar(dsn), nchar(dsn)) == "\\" + ) { p <- substr(dsn, 1, nchar(dsn) - 1) } else { p <- dsn } if (!file.exists(p) || !file.exists(dsn)) { - cli::cli_abort("File directory does not exist: {.var dsn}.", + cli::cli_abort( + "File directory does not exist: {.var dsn}.", call = rlang::caller_env() ) } @@ -164,15 +168,21 @@ # lastly merge the data frames into one tidy (large) data frame -------------- if (isFALSE(elv)) { - CRU_df <- Reduce(function(...) { - merge(..., by = c("lat", "lon", "month")) - }, CRU_list) + CRU_df <- Reduce( + function(...) { + merge(..., by = c("lat", "lon", "month")) + }, + CRU_list + ) } else if (elv && length(CRU_list) > 1) { elv_df <- CRU_list[which(names(CRU_list) %in% "elv")] CRU_list[which(names(CRU_list) %in% "elv")] <- NULL - CRU_df <- Reduce(function(...) { - merge(..., by = c("lat", "lon", "month")) - }, CRU_list) + CRU_df <- Reduce( + function(...) { + merge(..., by = c("lat", "lon", "month")) + }, + CRU_list + ) CRU_df <- CRU_df[elv_df$elv, on = c("lat", "lon")] } else if (elv) { @@ -289,7 +299,8 @@ nrows = 930, ncols = 2160, ymin = -65, - ymax = 90, xmin = -180, + ymax = 90, + xmin = -180, xmax = 180 ) @@ -358,11 +369,7 @@ #' #' @autoglobal #' @dev -.create_stack <- function(files, - wrld, - month_names, - pre, - pre_cv) { +.create_stack <- function(files, wrld, month_names, pre, pre_cv) { wvar <- data.frame(data.table::fread( cmd = paste0("gzip -dc ", files[[1]]), @@ -417,12 +424,15 @@ names(y) <- "elv" } - y <- terra::crop(y, terra::ext( - -180, - 180, - -60, - 85 - )) + y <- terra::crop( + y, + terra::ext( + -180, + 180, + -60, + 85 + ) + ) return(y) } @@ -480,19 +490,21 @@ #' the data frame. #' #' @dev -.get_local <- function(pre, - pre_cv, - rd0, - tmp, - dtr, - reh, - tmn, - tmx, - sunp, - frs, - wnd, - elv, - cache_dir) { +.get_local <- function( + pre, + pre_cv, + rd0, + tmp, + dtr, + reh, + tmn, + tmx, + sunp, + frs, + wnd, + elv, + cache_dir +) { # check if pre_cv or tmx/tmn (derived) are true, make sure proper ------------ # parameters set TRUE if (pre_cv) { @@ -529,17 +541,17 @@ ) names(files) <- names(object_list) <- - c( - "dtr_file", - "tmp_file", - "reh_file", - "elv_file", - "pre_file", - "sun_file", - "wnd_file", - "frs_file", - "rd0_file" - ) + c( + "dtr_file", + "tmp_file", + "reh_file", + "elv_file", + "pre_file", + "sun_file", + "wnd_file", + "frs_file", + "rd0_file" + ) # filter files --------------------------------------------------------------- # which files are being requested? @@ -547,9 +559,7 @@ # filter files from cache directory in case there are local files for which # we do not want data - cache_dir_contents <- as.list(list.files(cache_dir, - pattern = ".dat.gz$" - )) + cache_dir_contents <- as.list(list.files(cache_dir, pattern = ".dat.gz$")) files <- cache_dir_contents[cache_dir_contents %in% files] diff --git a/R/zzz.R b/R/zzz.R index c5d539e..a6f992d 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -1,8 +1,7 @@ manage_cache <- NULL # nocov start .onLoad <- - function(libname = find.package("getCRUCLdata"), - pkgname = "getCRUCLdata") { + function(libname = find.package("getCRUCLdata"), pkgname = "getCRUCLdata") { # CRAN Note avoidance if (getRversion() >= "2.15.1") { utils::globalVariables(c(".")) diff --git a/tests/spelling.R b/tests/spelling.R index 13f77d9..d60e024 100644 --- a/tests/spelling.R +++ b/tests/spelling.R @@ -1,6 +1,7 @@ if (requireNamespace("spelling", quietly = TRUE)) { spelling::spell_check_test( - vignettes = TRUE, error = FALSE, + vignettes = TRUE, + error = FALSE, skip_on_cran = TRUE ) } diff --git a/tests/testthat/test-create_CRU_stack.R b/tests/testthat/test-create_CRU_stack.R index 9a6d4d9..82b7c2d 100644 --- a/tests/testthat/test-create_CRU_stack.R +++ b/tests/testthat/test-create_CRU_stack.R @@ -479,19 +479,11 @@ test_that("create_CRU_stack returns a list of terra rast objects", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_pre.dat.gz"), "w") - utils::write.table(pre_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(pre_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) dsn <- tempdir() @@ -837,19 +829,11 @@ test_that("Test that create_stack creates tmx if requested", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_dtr.dat.gz"), "w") - utils::write.table(dtr_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(dtr_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) pre_cv <- FALSE @@ -868,7 +852,9 @@ test_that("Test that create_stack creates tmx if requested", { .create_stacks(tmn, tmx, tmp, dtr, pre, pre_cv, files) expect_named(CRU_stack_list, c("tmx")) - expect_equal(terra::minmax(CRU_stack_list[[1]])[[2]][[1]], 12.9, + expect_equal( + terra::minmax(CRU_stack_list[[1]])[[2]][[1]], + 12.9, tolerance = 0.1 ) unlink(list.files( @@ -1206,19 +1192,11 @@ test_that("Test that create_stack creates tmn if requested", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_dtr.dat.gz"), "w") - utils::write.table(dtr_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(dtr_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) pre_cv <- FALSE @@ -1237,7 +1215,9 @@ test_that("Test that create_stack creates tmn if requested", { .create_stacks(tmn, tmx, tmp, dtr, pre, pre_cv, files) expect_named(CRU_stack_list, c("tmn")) - expect_equal(terra::minmax(CRU_stack_list[[1]])[[2]][[1]], 4.3, + expect_equal( + terra::minmax(CRU_stack_list[[1]])[[2]][[1]], + 4.3, tolerance = 0.1 ) unlink(list.files( diff --git a/tests/testthat/test-create_stack.R b/tests/testthat/test-create_stack.R index 46ee7b8..de7814b 100644 --- a/tests/testthat/test-create_stack.R +++ b/tests/testthat/test-create_stack.R @@ -484,19 +484,11 @@ test_that("create_CRU_stack creates a list of terra rast of pre and tmp", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_pre.dat.gz"), "w") - utils::write.table(pre_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(pre_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) pre_cv <- TRUE @@ -623,14 +615,9 @@ test_that("create_CRU_stack creates a list containing only elv", { ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_elv.dat.gz"), "w") - utils::write.table(elv_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(elv_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) - files <- list.files( path = tempdir(), pattern = ".dat.gz$", @@ -701,20 +688,23 @@ test_that("month names are appropriate", { "dec" ) - expect_identical(month_names, c( - "jan", - "feb", - "mar", - "apr", - "may", - "jun", - "jul", - "aug", - "sep", - "oct", - "nov", - "dec" - )) + expect_identical( + month_names, + c( + "jan", + "feb", + "mar", + "apr", + "may", + "jun", + "jul", + "aug", + "sep", + "oct", + "nov", + "dec" + ) + ) }) # Test that CRU_stack_list returns list of raster stacks with proper names ----- @@ -1174,19 +1164,11 @@ test_that("CRU_stack_list returns list of raster stacks with proper names", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_pre.dat.gz"), "w") - utils::write.table(pre_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(pre_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) files <- diff --git a/tests/testthat/test-get_CRU.R b/tests/testthat/test-get_CRU.R index 4707316..95a8b83 100644 --- a/tests/testthat/test-get_CRU.R +++ b/tests/testthat/test-get_CRU.R @@ -100,7 +100,6 @@ test_that("get_CRU will retrieve diurnal tmp range & tmp files when tmx TRUE", { test_that("get_CRU will set pre to TRUE if pre_cv is TRUE and pre is FALSE", { skip_if_offline() - pre <- FALSE pre_cv <- TRUE diff --git a/tests/testthat/test_caching.R b/tests/testthat/test_caching.R index f0d3c00..8e989b9 100644 --- a/tests/testthat/test_caching.R +++ b/tests/testthat/test_caching.R @@ -24,9 +24,7 @@ test_that("test that set_cache does not create a dir if cache == FALSE", { test_that("cache directory is created if necessary", { # if cache directory exists during testing, remove it - unlink(manage_cache$cache_path_get(), - recursive = TRUE - ) + unlink(manage_cache$cache_path_get(), recursive = TRUE) cache <- TRUE cache_dir <- .set_cache(cache) expect_true(dir.exists(manage_cache$cache_path_get())) @@ -34,9 +32,7 @@ test_that("cache directory is created if necessary", { test_that("caching utils list files in cache and delete when asked", { skip_if_offline() - unlink(list.files(manage_cache$cache_path_get()), - recursive = TRUE - ) + unlink(list.files(manage_cache$cache_path_get()), recursive = TRUE) withr::local_envvar(R_USER_CACHE_DIR = tempdir()) temp_cache <- file.path(tempdir(), "R/getCRUCLdata/") manage_cache <- hoardr::hoard() diff --git a/tests/testthat/test_get_local.R b/tests/testthat/test_get_local.R index d2083eb..db5ca6d 100644 --- a/tests/testthat/test_get_local.R +++ b/tests/testthat/test_get_local.R @@ -458,19 +458,11 @@ test_that("Test that .get_local lists local files", { ) ) gz1 <- gzfile(file.path(tempdir(), "grid_10min_pre.dat.gz"), "w") - utils::write.table(pre_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(pre_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) gz1 <- gzfile(file.path(tempdir(), "grid_10min_tmp.dat.gz"), "w") - utils::write.table(tmp_data, - file = gz1, - col.names = FALSE, - row.names = FALSE - ) + utils::write.table(tmp_data, file = gz1, col.names = FALSE, row.names = FALSE) close(gz1) pre <- TRUE diff --git a/vignettes/precompile.R b/vignettes/precompile.R index b5a725d..0d3040b 100644 --- a/vignettes/precompile.R +++ b/vignettes/precompile.R @@ -15,8 +15,4 @@ devtools::build_vignettes() # move resource files to /doc resources <- list.files("vignettes/", pattern = ".png$", full.names = TRUE) -file.copy(from = resources, - to = "doc", - overwrite = TRUE) - - +file.copy(from = resources, to = "doc", overwrite = TRUE) From 45aefba390d2fb242ea7e21b8c6b6738b0e1e6d1 Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Mon, 3 Mar 2025 17:06:35 +0800 Subject: [PATCH 3/6] fix test that was rightfully not passing --- tests/testthat/test_caching.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test_caching.R b/tests/testthat/test_caching.R index 8e989b9..3d849b2 100644 --- a/tests/testthat/test_caching.R +++ b/tests/testthat/test_caching.R @@ -32,7 +32,6 @@ test_that("cache directory is created if necessary", { test_that("caching utils list files in cache and delete when asked", { skip_if_offline() - unlink(list.files(manage_cache$cache_path_get()), recursive = TRUE) withr::local_envvar(R_USER_CACHE_DIR = tempdir()) temp_cache <- file.path(tempdir(), "R/getCRUCLdata/") manage_cache <- hoardr::hoard() @@ -46,8 +45,10 @@ test_that("caching utils list files in cache and delete when asked", { f <- terra::rast(system.file("ex/test.grd", package = "terra")) cache_dir <- manage_cache$cache_path_get() - terra::writeRaster(f, file.path(cache_dir, "file1.asc")) - terra::writeRaster(f, file.path(cache_dir, "file2.asc")) + manage_cache$delete_all() + + terra::writeRaster(f, file.path(cache_dir, "file1.grd")) + terra::writeRaster(f, file.path(cache_dir, "file2.grd")) # test getCRUCLdata cache list k <- list.files(manage_cache$cache_path_get()) @@ -56,7 +57,7 @@ test_that("caching utils list files in cache and delete when asked", { # test delete one file expect_error(manage_cache$delete("file1.tif")) - manage_cache$delete("file1.asc") + manage_cache$delete("file1.grd") l <- list.files(manage_cache$cache_path_get()) expect_identical(basename(manage_cache$list()), l) From 72f654945d504ce45b56a05c23c509e31d4e410a Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Mon, 3 Mar 2025 17:07:02 +0800 Subject: [PATCH 4/6] update codemeta.json --- codemeta.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codemeta.json b/codemeta.json index b45f3de..f74219d 100644 --- a/codemeta.json +++ b/codemeta.json @@ -8,7 +8,7 @@ "codeRepository": "https://github.com/ropensci/getCRUCLdata", "issueTracker": "https://github.com/ropensci/getCRUCLdata/issues", "license": "https://spdx.org/licenses/MIT", - "version": "1.0.2", + "version": "1.0.3", "programmingLanguage": { "@type": "ComputerLanguage", "name": "R", @@ -254,7 +254,7 @@ "applicationCategory": "Tools", "isPartOf": "https://ropensci.org", "keywords": ["anglia-cru", "climate-data", "cru-cl2", "temperature", "rainfall", "elevation", "data-access", "wind", "relative-humidity", "solar-radiation", "diurnal-temperature", "frost", "cru", "r", "rstats", "r-package", "peer-reviewed"], - "fileSize": "913.618KB", + "fileSize": "911.891KB", "citation": [ { "@type": "ScholarlyArticle", From bc711d2d6500d753c19e3fcb5863f95ff2f8af82 Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Tue, 4 Mar 2025 20:10:24 +0800 Subject: [PATCH 5/6] update CONTRIBUTING.md --- .github/CONTRIBUTING.md | 45 +++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index fbcc9e7..b5e92d8 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -1,56 +1,57 @@ -# CONTRIBUTING # +# CONTRIBUTING ### Fixing typos Small typos or grammatical errors in documentation may be edited directly using the GitHub web interface, so long as the changes are made in the _source_ file. -* YES: you edit a roxygen comment in a `.R` file below `R/`. -* NO: you edit an `.Rd` file below `man/`. +- YES: you edit a roxygen comment in a `.R` file below `R/`. +- NO: you edit an `.Rd` file below `man/`. ### Prerequisites Before you make a substantial pull request, you should always file an issue and make sure someone from the team agrees that it’s a problem. If you’ve found a -bug, create an associated issue and illustrate the bug with a minimal +bug, create an associated issue and illustrate the bug with a minimal [reprex](https://www.tidyverse.org/help/#reprex). ### Pull request process -* We recommend that you create a Git branch for each pull request (PR). -* Look at the Travis and AppVeyor build status before and after making changes. -The `README` should contain badges for any continuous integration services used -by the package. -* We recommend the tidyverse [style guide](http://style.tidyverse.org). -You can use the [styler](https://CRAN.R-project.org/package=styler) package to -apply these styles, but please don't restyle code that has nothing to do with -your PR. -* We use [roxygen2](https://cran.r-project.org/package=roxygen2). -* We use [testthat](https://cran.r-project.org/package=testthat). Contributions -with test cases included are easier to accept. -* For user-facing changes, add a bullet to the top of `NEWS.md` below the -current development version header describing the changes made followed by your -GitHub username, and links to relevant issue(s)/PR(s). +- We recommend that you create a Git branch for each pull request (PR). +- Look at the Travis and AppVeyor build status before and after making changes. + The `README` should contain badges for any continuous integration services used + by the package. +- We recommend the tidyverse [style guide](http://style.tidyverse.org). + You can use the [styler](https://CRAN.R-project.org/package=styler) package or + the [Air formatter](https://posit-dev.github.io/air/formatter.html) to apply + these styles, but please don't restyle code that has nothing to do with your PR. +- We use [roxygen2](https://cran.r-project.org/package=roxygen2). +- We use [testthat](https://cran.r-project.org/package=testthat). Contributions + with test cases included are easier to accept. +- For user-facing changes, add a bullet to the top of `NEWS.md` below the + current development version header describing the changes made followed by your + GitHub username, and links to relevant issue(s)/PR(s). ### Code of Conduct -Please note that the getCRUCLdata project is released with a +Please note that the nasapower project is released with a [Contributor Code of Conduct](CODE_OF_CONDUCT.md). By contributing to this project you agree to abide by its terms. ### See rOpenSci [contributing guide](https://ropensci.github.io/dev_guide/contributingguide.html) + for further details. ### Discussion forum Check out our [discussion forum](https://discuss.ropensci.org) if you think your issue requires a longer form discussion. -### Prefer to Email? +### Prefer to Email? Email the person listed as maintainer in the `DESCRIPTION` file of this repo. Though note that private discussions over email don't help others - of course email is totally warranted if it's a sensitive problem of any kind. -### Thanks for contributing! +### Thanks for contributing -This contributing guide is adapted from the tidyverse contributing guide available at https://raw.githubusercontent.com/r-lib/usethis/master/inst/templates/tidy-contributing.md +This contributing guide is adapted from the tidyverse contributing guide available at From 1555031f739979bed196c923e2a3a261ea98c1ff Mon Sep 17 00:00:00 2001 From: "Adam H. Sparks" Date: Thu, 6 Mar 2025 16:12:13 +0800 Subject: [PATCH 6/6] set up flir --- .Rbuildignore | 4 + .github/workflows/flir.yaml | 37 +++++ flir/cache_file_state.rds | Bin 0 -> 44 bytes flir/config.yml | 51 +++++++ flir/rules/builtin/T_and_F_symbol.yml | 97 +++++++++++++ flir/rules/builtin/absolute_path.yml | 13 ++ flir/rules/builtin/any_duplicated.yml | 91 +++++++++++++ flir/rules/builtin/any_is_na.yml | 25 ++++ flir/rules/builtin/class_equals.yml | 42 ++++++ flir/rules/builtin/condition_message.yml | 23 ++++ flir/rules/builtin/double_assignment.yml | 23 ++++ flir/rules/builtin/duplicate_argument.yml | 46 +++++++ flir/rules/builtin/empty_assignment.yml | 15 +++ flir/rules/builtin/equal_assignment.yml | 10 ++ flir/rules/builtin/equals_na.yml | 37 +++++ flir/rules/builtin/expect_comparison.yml | 37 +++++ flir/rules/builtin/expect_identical.yml | 42 ++++++ flir/rules/builtin/expect_length.yml | 15 +++ flir/rules/builtin/expect_named.yml | 79 +++++++++++ flir/rules/builtin/expect_not.yml | 23 ++++ flir/rules/builtin/expect_null.yml | 22 +++ flir/rules/builtin/expect_true_false.yml | 28 ++++ flir/rules/builtin/expect_type.yml | 51 +++++++ flir/rules/builtin/for_loop_index.yml | 27 ++++ flir/rules/builtin/function_return.yml | 11 ++ flir/rules/builtin/implicit_assignment.yml | 69 ++++++++++ flir/rules/builtin/is_numeric.yml | 25 ++++ flir/rules/builtin/length_levels.yml | 7 + flir/rules/builtin/length_test.yml | 59 ++++++++ flir/rules/builtin/lengths.yml | 59 ++++++++ flir/rules/builtin/library_call.yml | 26 ++++ flir/rules/builtin/list_comparison.yml | 18 +++ flir/rules/builtin/literal_coercion.yml | 89 ++++++++++++ flir/rules/builtin/matrix_apply.yml | 142 ++++++++++++++++++++ flir/rules/builtin/missing_argument.yml | 44 ++++++ flir/rules/builtin/nested_ifelse.yml | 29 ++++ flir/rules/builtin/numeric_leading_zero.yml | 11 ++ flir/rules/builtin/outer_negation.yml | 29 ++++ flir/rules/builtin/package_hooks.yml | 127 +++++++++++++++++ flir/rules/builtin/paste.yml | 75 +++++++++++ flir/rules/builtin/redundant_equals.yml | 29 ++++ flir/rules/builtin/redundant_ifelse.yml | 67 +++++++++ flir/rules/builtin/rep_len.yml | 7 + flir/rules/builtin/right_assignment.yml | 10 ++ flir/rules/builtin/sample_int.yml | 43 ++++++ flir/rules/builtin/semicolon.yml | 10 ++ flir/rules/builtin/seq.yml | 121 +++++++++++++++++ flir/rules/builtin/sort.yml | 85 ++++++++++++ flir/rules/builtin/stopifnot_all.yml | 24 ++++ flir/rules/builtin/todo_comment.yml | 7 + flir/rules/builtin/undesirable_function.yml | 13 ++ flir/rules/builtin/undesirable_operator.yml | 29 ++++ flir/rules/builtin/unnecessary_nesting.yml | 36 +++++ flir/rules/builtin/unreachable_code.yml | 64 +++++++++ flir/rules/builtin/which_grepl.yml | 7 + getCRUCLdata.Rproj | 21 +++ 56 files changed, 2231 insertions(+) create mode 100644 .github/workflows/flir.yaml create mode 100644 flir/cache_file_state.rds create mode 100644 flir/config.yml create mode 100644 flir/rules/builtin/T_and_F_symbol.yml create mode 100644 flir/rules/builtin/absolute_path.yml create mode 100644 flir/rules/builtin/any_duplicated.yml create mode 100644 flir/rules/builtin/any_is_na.yml create mode 100644 flir/rules/builtin/class_equals.yml create mode 100644 flir/rules/builtin/condition_message.yml create mode 100644 flir/rules/builtin/double_assignment.yml create mode 100644 flir/rules/builtin/duplicate_argument.yml create mode 100644 flir/rules/builtin/empty_assignment.yml create mode 100644 flir/rules/builtin/equal_assignment.yml create mode 100644 flir/rules/builtin/equals_na.yml create mode 100644 flir/rules/builtin/expect_comparison.yml create mode 100644 flir/rules/builtin/expect_identical.yml create mode 100644 flir/rules/builtin/expect_length.yml create mode 100644 flir/rules/builtin/expect_named.yml create mode 100644 flir/rules/builtin/expect_not.yml create mode 100644 flir/rules/builtin/expect_null.yml create mode 100644 flir/rules/builtin/expect_true_false.yml create mode 100644 flir/rules/builtin/expect_type.yml create mode 100644 flir/rules/builtin/for_loop_index.yml create mode 100644 flir/rules/builtin/function_return.yml create mode 100644 flir/rules/builtin/implicit_assignment.yml create mode 100644 flir/rules/builtin/is_numeric.yml create mode 100644 flir/rules/builtin/length_levels.yml create mode 100644 flir/rules/builtin/length_test.yml create mode 100644 flir/rules/builtin/lengths.yml create mode 100644 flir/rules/builtin/library_call.yml create mode 100644 flir/rules/builtin/list_comparison.yml create mode 100644 flir/rules/builtin/literal_coercion.yml create mode 100644 flir/rules/builtin/matrix_apply.yml create mode 100644 flir/rules/builtin/missing_argument.yml create mode 100644 flir/rules/builtin/nested_ifelse.yml create mode 100644 flir/rules/builtin/numeric_leading_zero.yml create mode 100644 flir/rules/builtin/outer_negation.yml create mode 100644 flir/rules/builtin/package_hooks.yml create mode 100644 flir/rules/builtin/paste.yml create mode 100644 flir/rules/builtin/redundant_equals.yml create mode 100644 flir/rules/builtin/redundant_ifelse.yml create mode 100644 flir/rules/builtin/rep_len.yml create mode 100644 flir/rules/builtin/right_assignment.yml create mode 100644 flir/rules/builtin/sample_int.yml create mode 100644 flir/rules/builtin/semicolon.yml create mode 100644 flir/rules/builtin/seq.yml create mode 100644 flir/rules/builtin/sort.yml create mode 100644 flir/rules/builtin/stopifnot_all.yml create mode 100644 flir/rules/builtin/todo_comment.yml create mode 100644 flir/rules/builtin/undesirable_function.yml create mode 100644 flir/rules/builtin/undesirable_operator.yml create mode 100644 flir/rules/builtin/unnecessary_nesting.yml create mode 100644 flir/rules/builtin/unreachable_code.yml create mode 100644 flir/rules/builtin/which_grepl.yml create mode 100644 getCRUCLdata.Rproj diff --git a/.Rbuildignore b/.Rbuildignore index 49ed843..6901b71 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -31,3 +31,7 @@ ^man/dot-create_stack\.Rd$ ^man/dot-set_cache\.Rd$ ^man/dot-get_local\.Rd$ + + +# flir files +^flir$ diff --git a/.github/workflows/flir.yaml b/.github/workflows/flir.yaml new file mode 100644 index 0000000..a82408f --- /dev/null +++ b/.github/workflows/flir.yaml @@ -0,0 +1,37 @@ +# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples +# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help +on: + push: + branches: [main, master] + pull_request: + branches: [main, master] + release: + types: [published] + workflow_dispatch: + +name: flir + +jobs: + flir: + runs-on: macOS-latest + # Only restrict concurrency for non-PR jobs + concurrency: + group: flir-${{ github.event_name != 'pull_request' || github.run_id }} + env: + GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }} + permissions: + contents: write + steps: + - uses: actions/checkout@v4 + + - uses: r-lib/actions/setup-r@v2 + + - name: Install flir + run: install.packages("flir", repos = c("https://etiennebacher.r-universe.dev/", getOption("repos"))) + shell: Rscript {0} + + - name: Run flir + run: flir::lint() + shell: Rscript {0} + env: + FLIR_ERROR_ON_LINT: true diff --git a/flir/cache_file_state.rds b/flir/cache_file_state.rds new file mode 100644 index 0000000000000000000000000000000000000000..b8cafa45bcdfa1d213bd233dd6c72780e7314417 GIT binary patch literal 44 vcmb2|=3oE==I#ec2?+^l35jWG32CfGk`d0%cS>{{dg>Xr@2!@Q1}Xvo3Mve= literal 0 HcmV?d00001 diff --git a/flir/config.yml b/flir/config.yml new file mode 100644 index 0000000..6909752 --- /dev/null +++ b/flir/config.yml @@ -0,0 +1,51 @@ +keep: + - any_duplicated + - any_is_na + - class_equals + - condition_message + - double_assignment + - duplicate_argument + - empty_assignment + - equal_assignment + - equals_na + - expect_comparison + - expect_identical + - expect_length + - expect_named + - expect_not + - expect_null + - expect_true_false + - expect_type + - for_loop_index + - function_return + - implicit_assignment + - is_numeric + - length_levels + - length_test + - lengths + - library_call + - list_comparison + - literal_coercion + - matrix_apply + - missing_argument + - nested_ifelse + - numeric_leading_zero + - outer_negation + - package_hooks + - paste + - redundant_equals + - redundant_ifelse + - rep_len + - right_assignment + - sample_int + - semicolon + - seq + - sort + - stopifnot_all + - T_and_F_symbol + - todo_comment + - undesirable_function + - undesirable_operator + - unnecessary_nesting + - unreachable_code + - which_grepl diff --git a/flir/rules/builtin/T_and_F_symbol.yml b/flir/rules/builtin/T_and_F_symbol.yml new file mode 100644 index 0000000..ebe54ba --- /dev/null +++ b/flir/rules/builtin/T_and_F_symbol.yml @@ -0,0 +1,97 @@ +id: true_false_symbol +language: r +severity: warning +rule: + pattern: T + kind: identifier + not: + any: + - precedes: + any: + - pattern: <- + - pattern: = + - regex: ^~$ + - follows: + any: + - pattern: $ + - regex: ^~$ + - inside: + any: + - kind: parameter + - kind: call + - kind: binary_operator + follows: + regex: ^~$ + stopBy: end + stopBy: + kind: + argument +fix: TRUE +message: Use TRUE instead of the symbol T. + +--- + +id: true_false_symbol-2 +language: r +severity: warning +rule: + pattern: F + kind: identifier + not: + any: + - precedes: + any: + - pattern: <- + - pattern: = + - regex: ^~$ + - follows: + any: + - pattern: $ + - regex: ^~$ + - inside: + any: + - kind: parameter + - kind: call + - kind: binary_operator + follows: + regex: ^~$ + stopBy: end + stopBy: + kind: + argument +fix: FALSE +message: Use FALSE instead of the symbol F. + +--- + +id: true_false_symbol-3 +language: r +severity: warning +rule: + pattern: T + kind: identifier + precedes: + any: + - pattern: <- + - pattern: = + not: + inside: + kind: argument +message: Don't use T as a variable name, as it can break code relying on T being TRUE. + +--- + +id: true_false_symbol-4 +language: r +severity: warning +rule: + pattern: F + kind: identifier + precedes: + any: + - pattern: <- + - pattern: = + not: + inside: + kind: argument +message: Don't use F as a variable name, as it can break code relying on F being FALSE. diff --git a/flir/rules/builtin/absolute_path.yml b/flir/rules/builtin/absolute_path.yml new file mode 100644 index 0000000..2c5bb3d --- /dev/null +++ b/flir/rules/builtin/absolute_path.yml @@ -0,0 +1,13 @@ +# id: absolute_path-1 +# language: r +# severity: warning +# rule: +# kind: string_content +# any: +# - regex: '^~[[:alpha:]]' +# - regex: '^~/[[:alpha:]]' +# - regex: '^[[:alpha:]]:' +# - regex: '^(/|~)$' +# - regex: '^/[[:alpha:]]' +# - regex: '^\\' +# message: Do not use absolute paths. diff --git a/flir/rules/builtin/any_duplicated.yml b/flir/rules/builtin/any_duplicated.yml new file mode 100644 index 0000000..514b028 --- /dev/null +++ b/flir/rules/builtin/any_duplicated.yml @@ -0,0 +1,91 @@ +id: any_duplicated-1 +language: r +severity: warning +rule: + pattern: any($$$ duplicated($MYVAR) $$$) +fix: anyDuplicated(~~MYVAR~~) > 0 +message: anyDuplicated(x, ...) > 0 is better than any(duplicated(x), ...). + +--- + +id: any_duplicated-2 +language: r +severity: warning +rule: + any: + - pattern: length(unique($MYVAR)) == length($MYVAR) + - pattern: length($MYVAR) == length(unique($MYVAR)) +fix: anyDuplicated(~~MYVAR~~) == 0L +message: anyDuplicated(x) == 0L is better than length(unique(x)) == length(x). + +--- + +id: any_duplicated-3 +language: r +severity: warning +rule: + pattern: length(unique($MYVAR)) != length($MYVAR) +fix: anyDuplicated(~~MYVAR~~) != 0L +message: | + Use anyDuplicated(x) != 0L (or > or <) instead of length(unique(x)) != length(x) + (or > or <). + +--- + +id: any_duplicated-4 +language: r +severity: warning +rule: + any: + - pattern: nrow($DATA) != length(unique($DATA$µCOL)) + - pattern: length(unique($DATA$µCOL)) != nrow($DATA) +fix: anyDuplicated(~~DATA~~$~~COL~~) != 0L +message: | + anyDuplicated(DF$col) != 0L is better than length(unique(DF$col)) != nrow(DF) + +--- + +# id: any_duplicated-5 +# language: r +# severity: warning +# rule: +# any: +# - pattern: +# context: nrow($DATA) != length(unique($DATA[["µCOL"]])) +# strictness: ast +# - pattern: +# context: length(unique($DATA[["µCOL"]])) != nrow($DATA) +# strictness: ast +# fix: anyDuplicated(~~DATA~~[["~~COL~~"]]) != 0L +# message: | +# anyDuplicated(DF[["col"]]) != 0L is better than length(unique(DF[["col"]])) != nrow(DF) +# +# --- + +id: any_duplicated-6 +language: r +severity: warning +rule: + any: + - pattern: nrow($DATA) == length(unique($DATA$µCOL)) + - pattern: length(unique($DATA$µCOL)) == nrow($DATA) +fix: anyDuplicated(~~DATA~~$~~COL~~) == 0L +message: | + anyDuplicated(DF$col) == 0L is better than length(unique(DF$col)) == nrow(DF) + +# --- +# +# id: any_duplicated-7 +# language: r +# severity: warning +# rule: +# any: +# - pattern: +# context: nrow($DATA) == length(unique($DATA[["µCOL"]])) +# strictness: ast +# - pattern: +# context: length(unique($DATA[["µCOL"]])) == nrow($DATA) +# strictness: ast +# fix: anyDuplicated(~~DATA~~[["~~COL~~"]]) == 0L +# message: | +# anyDuplicated(DF[["col"]]) == 0L is better than length(unique(DF[["col"]])) == nrow(DF) diff --git a/flir/rules/builtin/any_is_na.yml b/flir/rules/builtin/any_is_na.yml new file mode 100644 index 0000000..68acfae --- /dev/null +++ b/flir/rules/builtin/any_is_na.yml @@ -0,0 +1,25 @@ +id: any_na-1 +language: r +severity: warning +rule: + any: + - pattern: any(is.na($MYVAR)) + - pattern: any(na.rm = $NARM, is.na($MYVAR)) + - pattern: any(is.na($MYVAR), na.rm = $NARM) +fix: anyNA(~~MYVAR~~) +message: anyNA(x) is better than any(is.na(x)). + +--- + +id: any_na-2 +language: r +severity: warning +rule: + any: + - pattern: NA %in% $ELEM + - pattern: NA_real_ %in% $ELEM + - pattern: NA_logical_ %in% $ELEM + - pattern: NA_character_ %in% $ELEM + - pattern: NA_complex_ %in% $ELEM +fix: anyNA(~~ELEM~~) +message: anyNA(x) is better than NA %in% x. diff --git a/flir/rules/builtin/class_equals.yml b/flir/rules/builtin/class_equals.yml new file mode 100644 index 0000000..2b7eb4c --- /dev/null +++ b/flir/rules/builtin/class_equals.yml @@ -0,0 +1,42 @@ +id: class_equals-1 +language: r +severity: warning +rule: + any: + - pattern: class($VAR) == $CLASSNAME + - pattern: $CLASSNAME == class($VAR) + not: + inside: + kind: argument +fix: inherits(~~VAR~~, ~~CLASSNAME~~) +message: Instead of comparing class(x) with ==, use inherits(x, 'class-name') or is. or is(x, 'class') + +--- + +id: class_equals-2 +language: r +severity: warning +rule: + any: + - pattern: class($VAR) != $CLASSNAME + - pattern: $CLASSNAME != class($VAR) + not: + inside: + kind: argument +fix: "!inherits(~~VAR~~, ~~CLASSNAME~~)" +message: "Instead of comparing class(x) with !=, use !inherits(x, 'class-name') or is. or is(x, 'class')" + +--- + +id: class_equals-3 +language: r +severity: warning +rule: + any: + - pattern: $CLASSNAME %in% class($VAR) + - pattern: class($VAR) %in% $CLASSNAME +constraints: + CLASSNAME: + kind: string +fix: inherits(~~VAR~~, ~~CLASSNAME~~) +message: Instead of comparing class(x) with %in%, use inherits(x, 'class-name') or is. or is(x, 'class') diff --git a/flir/rules/builtin/condition_message.yml b/flir/rules/builtin/condition_message.yml new file mode 100644 index 0000000..eb32466 --- /dev/null +++ b/flir/rules/builtin/condition_message.yml @@ -0,0 +1,23 @@ +id: condition_message-1 +language: r +severity: warning +rule: + pattern: $FUN($$$ paste0($$$MSG) $$$) + kind: call + not: + any: + - has: + kind: extract_operator + - has: + stopBy: end + kind: argument + has: + field: name + regex: "^collapse|recycle0$" + stopBy: end +constraints: + FUN: + regex: "^(packageStartupMessage|stop|warning)$" +fix: ~~FUN~~(~~MSG~~) +message: | + ~~FUN~~(paste0(...)) can be rewritten as ~~FUN~~(...). diff --git a/flir/rules/builtin/double_assignment.yml b/flir/rules/builtin/double_assignment.yml new file mode 100644 index 0000000..60a04e2 --- /dev/null +++ b/flir/rules/builtin/double_assignment.yml @@ -0,0 +1,23 @@ +id: right_double_assignment +language: r +severity: hint +rule: + pattern: $RHS ->> $LHS + has: + field: rhs + kind: identifier +message: ->> can have hard-to-predict behavior; prefer assigning to a + specific environment instead (with assign() or <-). + +--- + +id: left_double_assignment +language: r +severity: hint +rule: + pattern: $LHS <<- $RHS + has: + field: lhs + kind: identifier +message: <<- can have hard-to-predict behavior; prefer assigning to a + specific environment instead (with assign() or <-). diff --git a/flir/rules/builtin/duplicate_argument.yml b/flir/rules/builtin/duplicate_argument.yml new file mode 100644 index 0000000..415b9ff --- /dev/null +++ b/flir/rules/builtin/duplicate_argument.yml @@ -0,0 +1,46 @@ +id: duplicate_argument-1 +language: r +severity: warning +rule: + # Look for a function argument... + kind: argument + any: + - has: + kind: identifier + field: name + pattern: $OBJ + - has: + kind: string_content + pattern: $OBJ + stopBy: end + + # ... that follows other argument(s) with the same name... + follows: + kind: argument + stopBy: end + has: + stopBy: end + kind: identifier + field: name + pattern: $OBJ + + # ... inside a function call (or a subset environment for data.table)... + inside: + kind: arguments + follows: + any: + - kind: identifier + pattern: $FUN + - kind: string + inside: + any: + - kind: call + - kind: subset + +# ... that is not a function listed below. +constraints: + FUN: + not: + regex: ^(mutate|transmute)$ + +message: Avoid duplicate arguments in function calls. diff --git a/flir/rules/builtin/empty_assignment.yml b/flir/rules/builtin/empty_assignment.yml new file mode 100644 index 0000000..ccc995f --- /dev/null +++ b/flir/rules/builtin/empty_assignment.yml @@ -0,0 +1,15 @@ +id: empty_assignment-1 +language: r +severity: warning +rule: + any: + - pattern: $OBJ <- {} + - pattern: $OBJ <- {$CONTENT} + - pattern: $OBJ = {} + - pattern: $OBJ = {$CONTENT} +constraints: + CONTENT: + regex: ^\s+$ +message: | + Assign NULL explicitly or, whenever possible, allocate the empty object with + the right type and size. diff --git a/flir/rules/builtin/equal_assignment.yml b/flir/rules/builtin/equal_assignment.yml new file mode 100644 index 0000000..77074fe --- /dev/null +++ b/flir/rules/builtin/equal_assignment.yml @@ -0,0 +1,10 @@ +id: equal_assignment +language: r +severity: hint +rule: + pattern: $LHS = $RHS + has: + field: lhs + kind: identifier +fix: ~~LHS~~ <- ~~RHS~~ +message: Use <-, not =, for assignment. diff --git a/flir/rules/builtin/equals_na.yml b/flir/rules/builtin/equals_na.yml new file mode 100644 index 0000000..64e7454 --- /dev/null +++ b/flir/rules/builtin/equals_na.yml @@ -0,0 +1,37 @@ +id: equals_na +language: r +severity: warning +rule: + any: + - pattern: $MYVAR == NA + - pattern: $MYVAR == NA_integer_ + - pattern: $MYVAR == NA_real_ + - pattern: $MYVAR == NA_complex_ + - pattern: $MYVAR == NA_character_ + - pattern: NA == $MYVAR + - pattern: NA_integer_ == $MYVAR + - pattern: NA_real_ == $MYVAR + - pattern: NA_complex_ == $MYVAR + - pattern: NA_character_ == $MYVAR +fix: is.na(~~MYVAR~~) +message: Use is.na for comparisons to NA (not == or !=). + +--- + +id: equals_na-2 +language: r +severity: warning +rule: + any: + - pattern: $MYVAR != NA + - pattern: $MYVAR != NA_integer_ + - pattern: $MYVAR != NA_real_ + - pattern: $MYVAR != NA_complex_ + - pattern: $MYVAR != NA_character_ + - pattern: NA != $MYVAR + - pattern: NA_integer_ != $MYVAR + - pattern: NA_real_ != $MYVAR + - pattern: NA_complex_ != $MYVAR + - pattern: NA_character_ != $MYVAR +fix: is.na(~~MYVAR~~) +message: Use is.na for comparisons to NA (not == or !=). diff --git a/flir/rules/builtin/expect_comparison.yml b/flir/rules/builtin/expect_comparison.yml new file mode 100644 index 0000000..6af9bb1 --- /dev/null +++ b/flir/rules/builtin/expect_comparison.yml @@ -0,0 +1,37 @@ +id: expect_comparison-1 +language: r +severity: warning +rule: + pattern: expect_true($X > $Y) +fix: expect_gt(~~X~~, ~~Y~~) +message: expect_gt(x, y) is better than expect_true(x > y). + +--- + +id: expect_comparison-2 +language: r +severity: warning +rule: + pattern: expect_true($X >= $Y) +fix: expect_gte(~~X~~, ~~Y~~) +message: expect_gte(x, y) is better than expect_true(x >= y). + +--- + +id: expect_comparison-3 +language: r +severity: warning +rule: + pattern: expect_true($X < $Y) +fix: expect_lt(~~X~~, ~~Y~~) +message: expect_lt(x, y) is better than expect_true(x < y). + +--- + +id: expect_comparison-4 +language: r +severity: warning +rule: + pattern: expect_true($X <= $Y) +fix: expect_lte(~~X~~, ~~Y~~) +message: expect_lte(x, y) is better than expect_true(x <= y). diff --git a/flir/rules/builtin/expect_identical.yml b/flir/rules/builtin/expect_identical.yml new file mode 100644 index 0000000..3af5a85 --- /dev/null +++ b/flir/rules/builtin/expect_identical.yml @@ -0,0 +1,42 @@ +id: expect_identical-1 +language: r +severity: warning +rule: + pattern: expect_true(identical($VAL1, $VAL2)) +fix: expect_identical(~~VAL1~~, ~~VAL2~~) +message: Use expect_identical(x, y) instead of expect_true(identical(x, y)). + +--- + +id: expect_identical-2 +language: r +severity: warning +rule: + pattern: expect_equal($VAL1, $VAL2) +fix: expect_identical(~~VAL1~~, ~~VAL2~~) +constraints: + VAL1: + all: + - not: + has: + stopBy: end + kind: float + regex: \. + - not: + regex: ^typeof + - not: + pattern: NULL + VAL2: + all: + - not: + has: + stopBy: end + kind: float + regex: \. + - not: + regex: ^typeof + - not: + pattern: NULL +message: | + Use expect_identical(x, y) by default; resort to expect_equal() only when + needed, e.g. when setting ignore_attr= or tolerance=. diff --git a/flir/rules/builtin/expect_length.yml b/flir/rules/builtin/expect_length.yml new file mode 100644 index 0000000..fe233a7 --- /dev/null +++ b/flir/rules/builtin/expect_length.yml @@ -0,0 +1,15 @@ +id: expect_length-1 +language: r +severity: warning +rule: + any: + - pattern: $FUN(length($OBJ), $VALUE) + - pattern: $FUN($VALUE, length($OBJ)) +constraints: + FUN: + regex: ^(expect_identical|expect_equal)$ + VALUE: + not: + regex: length\( +fix: expect_length(~~OBJ~~, ~~VALUE~~) +message: expect_length(x, n) is better than ~~FUN~~(length(x), n). diff --git a/flir/rules/builtin/expect_named.yml b/flir/rules/builtin/expect_named.yml new file mode 100644 index 0000000..7d0691e --- /dev/null +++ b/flir/rules/builtin/expect_named.yml @@ -0,0 +1,79 @@ +id: expect_named-1 +language: r +severity: warning +rule: + any: + - pattern: + context: expect_identical(names($OBJ), $VALUES) + strictness: ast + - pattern: + context: expect_identical($VALUES, names($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: ^(colnames\(|rownames\(|dimnames\(|NULL|names\() + has: + kind: null +fix: expect_named(~~OBJ~~, ~~VALUES~~) +message: expect_named(x, n) is better than expect_identical(names(x), n). + +--- + +id: expect_named-2 +language: r +severity: warning +rule: + any: + - pattern: + context: expect_equal(names($OBJ), $VALUES) + strictness: ast + - pattern: + context: expect_equal($VALUES, names($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: ^(colnames\(|rownames\(|dimnames\(|NULL|names\() +fix: expect_named(~~OBJ~~, ~~VALUES~~) +message: expect_named(x, n) is better than expect_equal(names(x), n). + +--- + +id: expect_named-3 +language: r +severity: warning +rule: + any: + - pattern: + context: testthat::expect_identical(names($OBJ), $VALUES) + strictness: ast + - pattern: + context: testthat::expect_identical($VALUES, names($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: ^(colnames\(|rownames\(|dimnames\(|NULL|names\() +fix: testthat::expect_named(~~OBJ~~, ~~VALUES~~) +message: expect_named(x, n) is better than expect_identical(names(x), n). + +--- + +id: expect_named-4 +language: r +severity: warning +rule: + any: + - pattern: + context: testthat::expect_equal(names($OBJ), $VALUES) + strictness: ast + - pattern: + context: testthat::expect_equal($VALUES, names($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: ^(colnames\(|rownames\(|dimnames\(|NULL|names\() +fix: testthat::expect_named(~~OBJ~~, ~~VALUES~~) +message: expect_named(x, n) is better than expect_equal(names(x), n). diff --git a/flir/rules/builtin/expect_not.yml b/flir/rules/builtin/expect_not.yml new file mode 100644 index 0000000..3a23e9f --- /dev/null +++ b/flir/rules/builtin/expect_not.yml @@ -0,0 +1,23 @@ +id: expect_not-1 +language: r +severity: warning +rule: + all: + - pattern: expect_true(!$COND) + - not: + regex: '^expect_true\(!!' +fix: expect_false(~~COND~~) +message: expect_false(x) is better than expect_true(!x), and vice versa. + +--- + +id: expect_not-2 +language: r +severity: warning +rule: + all: + - pattern: expect_false(!$COND) + - not: + regex: '^expect_false\(!!' +fix: expect_true(~~COND~~) +message: expect_false(x) is better than expect_true(!x), and vice versa. diff --git a/flir/rules/builtin/expect_null.yml b/flir/rules/builtin/expect_null.yml new file mode 100644 index 0000000..7888fdb --- /dev/null +++ b/flir/rules/builtin/expect_null.yml @@ -0,0 +1,22 @@ +id: expect_null-1 +language: r +severity: warning +rule: + any: + - pattern: $FUN(NULL, $VALUES) + - pattern: $FUN($VALUES, NULL) +constraints: + FUN: + regex: ^(expect_identical|expect_equal)$ +fix: expect_null(~~VALUES~~) +message: expect_null(x) is better than ~~FUN~~(x, NULL). + +--- + +id: expect_null-2 +language: r +severity: warning +rule: + pattern: expect_true(is.null($VALUES)) +fix: expect_null(~~VALUES~~) +message: expect_null(x) is better than expect_true(is.null(x)). diff --git a/flir/rules/builtin/expect_true_false.yml b/flir/rules/builtin/expect_true_false.yml new file mode 100644 index 0000000..784843d --- /dev/null +++ b/flir/rules/builtin/expect_true_false.yml @@ -0,0 +1,28 @@ +id: expect_true_false-1 +language: r +severity: warning +rule: + any: + - pattern: $FUN(TRUE, $VALUES) + - pattern: $FUN($VALUES, TRUE) +constraints: + FUN: + regex: ^(expect_identical|expect_equal)$ +fix: expect_true(~~VALUES~~) +message: expect_true(x) is better than ~~FUN~~(x, TRUE). + +--- + +id: expect_true_false-2 +language: r +severity: warning +rule: + any: + - pattern: $FUN(FALSE, $VALUES) + - pattern: $FUN($VALUES, FALSE) +constraints: + FUN: + regex: ^(expect_identical|expect_equal)$ +fix: expect_false(~~VALUES~~) +message: expect_false(x) is better than ~~FUN~~(x, FALSE). + diff --git a/flir/rules/builtin/expect_type.yml b/flir/rules/builtin/expect_type.yml new file mode 100644 index 0000000..1ab20ec --- /dev/null +++ b/flir/rules/builtin/expect_type.yml @@ -0,0 +1,51 @@ +id: expect_type-1 +language: r +severity: warning +rule: + any: + - pattern: + context: expect_identical(typeof($OBJ), $VALUES) + strictness: ast + - pattern: + context: expect_identical($VALUES, typeof($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: typeof +fix: expect_type(~~OBJ~~, ~~VALUES~~) +message: expect_type(x, t) is better than expect_identical(typeof(x), t). + +--- + +id: expect_type-2 +language: r +severity: warning +rule: + any: + - pattern: + context: expect_equal(typeof($OBJ), $VALUES) + strictness: ast + - pattern: + context: expect_equal($VALUES, typeof($OBJ)) + strictness: ast +constraints: + VALUES: + not: + regex: typeof +fix: expect_type(~~OBJ~~, ~~VALUES~~) +message: expect_type(x, t) is better than expect_equal(typeof(x), t). + +--- + +id: expect_type-3 +language: r +severity: warning +rule: + pattern: expect_true($FUN($OBJ)) +constraints: + FUN: + regex: ^is\. + not: + regex: data\.frame$ +message: expect_type(x, t) is better than expect_true(is.(x)). diff --git a/flir/rules/builtin/for_loop_index.yml b/flir/rules/builtin/for_loop_index.yml new file mode 100644 index 0000000..e5b28b4 --- /dev/null +++ b/flir/rules/builtin/for_loop_index.yml @@ -0,0 +1,27 @@ +id: for_loop_index-1 +language: r +severity: warning +rule: + pattern: for ($IDX in $IDX) +message: Don't re-use any sequence symbols as the index symbol in a for loop. + +--- + +id: for_loop_index-2 +language: r +severity: warning +rule: + pattern: for ($IDX in $SEQ) +constraints: + SEQ: + kind: call + has: + kind: arguments + has: + kind: argument + stopBy: end + has: + kind: identifier + field: value + pattern: $IDX +message: Don't re-use any sequence symbols as the index symbol in a for loop. diff --git a/flir/rules/builtin/function_return.yml b/flir/rules/builtin/function_return.yml new file mode 100644 index 0000000..31ba46b --- /dev/null +++ b/flir/rules/builtin/function_return.yml @@ -0,0 +1,11 @@ +id: function_return-1 +language: r +severity: warning +rule: + any: + - pattern: return($OBJ <- $VAL) + - pattern: return($OBJ <<- $VAL) + - pattern: return($VAL -> $OBJ) + - pattern: return($VAL ->> $OBJ) +message: | + Move the assignment outside of the return() clause, or skip assignment altogether. diff --git a/flir/rules/builtin/implicit_assignment.yml b/flir/rules/builtin/implicit_assignment.yml new file mode 100644 index 0000000..133a40e --- /dev/null +++ b/flir/rules/builtin/implicit_assignment.yml @@ -0,0 +1,69 @@ +id: implicit_assignment-1 +language: r +severity: warning +rule: + any: + - pattern: $RECEIVER <- $VALUE + - pattern: $RECEIVER <<- $VALUE + - pattern: $VALUE -> $RECEIVER + - pattern: $VALUE ->> $RECEIVER + inside: + any: + - kind: if_statement + - kind: while_statement + field: condition + stopBy: end + strictness: cst +message: | + Avoid implicit assignments in function calls. For example, instead of + `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`. + +--- + +id: implicit_assignment-2 +language: r +severity: warning +rule: + any: + - pattern: $RECEIVER <- $VALUE + - pattern: $RECEIVER <<- $VALUE + - pattern: $VALUE -> $RECEIVER + - pattern: $VALUE ->> $RECEIVER + inside: + kind: for_statement + field: sequence + stopBy: end + strictness: cst +message: | + Avoid implicit assignments in function calls. For example, instead of + `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`. + +# --- +# +# id: implicit_assignment-3 +# language: r +# severity: warning +# rule: +# any: +# - pattern: $RECEIVER <- $VALUE +# - pattern: $RECEIVER <<- $VALUE +# - pattern: $VALUE -> $RECEIVER +# - pattern: $VALUE ->> $RECEIVER +# inside: +# kind: argument +# field: value +# strictness: cst +# stopBy: end +# not: +# inside: +# kind: call +# field: function +# has: +# kind: identifier +# regex: ^(lapply)$ +# stopBy: end +# strictness: cst +# message: | +# Avoid implicit assignments in function calls. For example, instead of +# `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`. + diff --git a/flir/rules/builtin/is_numeric.yml b/flir/rules/builtin/is_numeric.yml new file mode 100644 index 0000000..0a76b08 --- /dev/null +++ b/flir/rules/builtin/is_numeric.yml @@ -0,0 +1,25 @@ +id: is_numeric-1 +language: r +severity: warning +rule: + any: + - pattern: is.numeric($VAR) || is.integer($VAR) + - pattern: is.integer($VAR) || is.numeric($VAR) +message: is.numeric(x) || is.integer(x) can be simplified to is.numeric(x). Use + is.double(x) to test for objects stored as 64-bit floating point. + +--- + +id: is_numeric-2 +language: r +severity: warning +rule: + any: + - pattern: + context: class($VAR) %in% c("numeric", "integer") + strictness: ast + - pattern: + context: class($VAR) %in% c("integer", "numeric") + strictness: ast +message: class(x) %in% c("numeric", "integer") can be simplified to is.numeric(x). Use + is.double(x) to test for objects stored as 64-bit floating point. diff --git a/flir/rules/builtin/length_levels.yml b/flir/rules/builtin/length_levels.yml new file mode 100644 index 0000000..8e02978 --- /dev/null +++ b/flir/rules/builtin/length_levels.yml @@ -0,0 +1,7 @@ +id: length_levels-1 +language: r +severity: warning +rule: + pattern: length(levels($VAR)) +fix: nlevels(~~VAR~~) +message: nlevels(x) is better than length(levels(x)). df diff --git a/flir/rules/builtin/length_test.yml b/flir/rules/builtin/length_test.yml new file mode 100644 index 0000000..f9ba9ed --- /dev/null +++ b/flir/rules/builtin/length_test.yml @@ -0,0 +1,59 @@ +# Strangely, having something like pattern: length($VAR $OP $VAR2) doesn't work + +id: length_test-1 +language: r +severity: warning +rule: + pattern: length($VAR == $VAR2) +fix: length(~~VAR~~) == ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. + +--- + +id: length_test-2 +language: r +severity: warning +rule: + pattern: length($VAR != $VAR2) +fix: length(~~VAR~~) != ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. + +--- + +id: length_test-3 +language: r +severity: warning +rule: + pattern: length($VAR > $VAR2) +fix: length(~~VAR~~) > ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. + +--- + +id: length_test-4 +language: r +severity: warning +rule: + pattern: length($VAR >= $VAR2) +fix: length(~~VAR~~) >= ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. + +--- + +id: length_test-5 +language: r +severity: warning +rule: + pattern: length($VAR < $VAR2) +fix: length(~~VAR~~) < ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. + +--- + +id: length_test-6 +language: r +severity: warning +rule: + pattern: length($VAR <= $VAR2) +fix: length(~~VAR~~) <= ~~VAR2~~ +message: Checking the length of a logical vector is likely a mistake. diff --git a/flir/rules/builtin/lengths.yml b/flir/rules/builtin/lengths.yml new file mode 100644 index 0000000..a416440 --- /dev/null +++ b/flir/rules/builtin/lengths.yml @@ -0,0 +1,59 @@ +id: sapply_lengths-1 +language: r +severity: warning +rule: + any: + - pattern: sapply($MYVAR, length) + - pattern: sapply(FUN = length, $MYVAR) + - pattern: sapply($MYVAR, FUN = length) + - pattern: vapply($MYVAR, length $$$) + + - pattern: map_dbl($MYVAR, length) + - pattern: map_dbl($MYVAR, .f = length) + - pattern: map_dbl(.f = length, $MYVAR) + - pattern: map_int($MYVAR, length) + - pattern: map_int($MYVAR, .f = length) + - pattern: map_int(.f = length, $MYVAR) + + - pattern: purrr::map_dbl($MYVAR, length) + - pattern: purrr::map_dbl($MYVAR, .f = length) + - pattern: purrr::map_dbl(.f = length, $MYVAR) + - pattern: purrr::map_int($MYVAR, length) + - pattern: purrr::map_int($MYVAR, .f = length) + - pattern: purrr::map_int(.f = length, $MYVAR) +fix: lengths(~~MYVAR~~) +message: Use lengths() to find the length of each element in a list. + +--- + +id: sapply_lengths-2 +language: r +severity: warning +rule: + any: + - pattern: $MYVAR |> sapply(length) + - pattern: $MYVAR |> sapply(FUN = length) + - pattern: $MYVAR |> vapply(length $$$) + - pattern: $MYVAR |> map_int(length) + - pattern: $MYVAR |> map_int(length $$$) + - pattern: $MYVAR |> purrr::map_int(length) + - pattern: $MYVAR |> purrr::map_int(length $$$) +fix: ~~MYVAR~~ |> lengths() +message: Use lengths() to find the length of each element in a list. + +--- + +id: sapply_lengths-3 +language: r +severity: warning +rule: + any: + - pattern: $MYVAR %>% sapply(length) + - pattern: $MYVAR %>% sapply(FUN = length) + - pattern: $MYVAR %>% vapply(length $$$) + - pattern: $MYVAR %>% map_int(length) + - pattern: $MYVAR %>% map_int(length $$$) + - pattern: $MYVAR %>% purrr::map_int(length) + - pattern: $MYVAR %>% purrr::map_int(length $$$) +fix: ~~MYVAR~~ %>% lengths() +message: Use lengths() to find the length of each element in a list. diff --git a/flir/rules/builtin/library_call.yml b/flir/rules/builtin/library_call.yml new file mode 100644 index 0000000..4b578fe --- /dev/null +++ b/flir/rules/builtin/library_call.yml @@ -0,0 +1,26 @@ +id: library_call +language: r +severity: warning +rule: + kind: call + has: + regex: ^library|require$ + kind: identifier + follows: + not: + any: + - kind: call + has: + regex: ^library|require$ + kind: identifier + - kind: comment + not: + inside: + stopBy: end + any: + - kind: function_definition + - kind: call + has: + pattern: suppressPackageStartupMessages + kind: identifier +message: Move all library/require calls to the top of the script. diff --git a/flir/rules/builtin/list_comparison.yml b/flir/rules/builtin/list_comparison.yml new file mode 100644 index 0000000..772d433 --- /dev/null +++ b/flir/rules/builtin/list_comparison.yml @@ -0,0 +1,18 @@ +id: list_comparison-1 +language: r +severity: warning +rule: + any: + - pattern: $FUN($$$) > $$$ + - pattern: $FUN($$$) >= $$$ + - pattern: $FUN($$$) < $$$ + - pattern: $FUN($$$) <= $$$ + - pattern: $FUN($$$) == $$$ + - pattern: $FUN($$$) != $$$ +constraints: + FUN: + regex: ^(lapply|map|Map|\.mapply)$ +message: | + The output of ~~FUN~~(), a list, is being coerced for comparison. + Instead, use a mapper that generates a vector with the correct type directly, + for example vapply(x, FUN, character(1L)) if the output is a string. diff --git a/flir/rules/builtin/literal_coercion.yml b/flir/rules/builtin/literal_coercion.yml new file mode 100644 index 0000000..e61fb24 --- /dev/null +++ b/flir/rules/builtin/literal_coercion.yml @@ -0,0 +1,89 @@ +id: literal_coercion-1 +language: r +severity: warning +rule: + pattern: $FUN($VALUE) +constraints: + VALUE: + kind: argument + has: + kind: float + not: + regex: 'e' + FUN: + regex: ^(int|as\.integer)$ +fix: ~~VALUE~~L +message: | + Use ~~VALUE~~L instead of ~~FUN~~(~~VALUE~~), i.e., use literals directly + where possible, instead of coercion. + +--- + +id: literal_coercion-2 +language: r +severity: warning +rule: + pattern: as.character(NA) +fix: NA_character_ +message: | + Use NA_character_ instead of as.character(NA), i.e., use literals directly + where possible, instead of coercion. + +--- + +id: literal_coercion-3 +language: r +severity: warning +rule: + pattern: as.logical($VAR) +constraints: + VAR: + kind: argument + has: + any: + - regex: ^1L$ + - regex: ^1$ + - regex: 'true' +fix: TRUE +message: Use TRUE instead of as.logical(~~VAR~~). + +--- + +id: literal_coercion-4 +language: r +severity: warning +rule: + pattern: $FUN($VAR) +constraints: + VAR: + kind: argument + has: + kind: float + FUN: + regex: ^(as\.numeric|as\.double)$ +fix: ~~VAR~~ +message: Use ~~VAR~~ instead of ~~FUN~~(~~VAR~~). + +--- + +id: literal_coercion-5 +language: r +severity: warning +rule: + pattern: as.integer(NA) +fix: NA_integer_ +message: Use NA_integer_ instead of as.integer(NA). + +--- + +id: literal_coercion-6 +language: r +severity: warning +rule: + pattern: $FUN(NA) +constraints: + FUN: + regex: ^(as\.numeric|as\.double)$ +fix: NA_real_ +message: Use NA_real_ instead of ~~FUN~~(NA). + diff --git a/flir/rules/builtin/matrix_apply.yml b/flir/rules/builtin/matrix_apply.yml new file mode 100644 index 0000000..7eaa40c --- /dev/null +++ b/flir/rules/builtin/matrix_apply.yml @@ -0,0 +1,142 @@ +id: matrix_apply-1 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, sum) + - pattern: apply($INPUT, MARGIN = $MARG, sum) + - pattern: apply($INPUT, $MARG, FUN = sum) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = sum) +constraints: + MARG: + has: + regex: ^(2|2L)$ +fix: colSums(~~INPUT~~) +message: Use colSums(x) rather than apply(x, 2, sum) + +--- + +id: matrix_apply-2 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, sum, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, sum, na.rm = $NARM) + - pattern: apply($INPUT, $MARG, FUN = sum, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = sum, na.rm = $NARM) +constraints: + MARG: + has: + regex: ^(2|2L)$ +fix: colSums(~~INPUT~~, na.rm = ~~NARM~~) +message: Use colSums(x, na.rm = ~~NARM~~) rather than apply(x, 2, sum, na.rm = ~~NARM~~). + +--- + +id: matrix_apply-3 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, sum) + - pattern: apply($INPUT, MARGIN = $MARG, sum) + - pattern: apply($INPUT, $MARG, FUN = sum) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = sum) +constraints: + MARG: + has: + regex: ^(1|1L)$ +fix: rowSums(~~INPUT~~) +message: Use rowSums(x) rather than apply(x, 1, sum) + +--- + +id: matrix_apply-4 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, sum, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, sum, na.rm = $NARM) + - pattern: apply($INPUT, $MARG, FUN = sum, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = sum, na.rm = $NARM) +constraints: + MARG: + has: + regex: ^(1|1L)$ +fix: rowSums(~~INPUT~~, na.rm = ~~NARM~~) +message: Use rowSums(x, na.rm = ~~NARM~~) rather than apply(x, 1, sum, na.rm = ~~NARM~~). + +--- + +id: matrix_apply-5 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, mean) + - pattern: apply($INPUT, MARGIN = $MARG, mean) + - pattern: apply($INPUT, $MARG, FUN = mean) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = mean) +constraints: + MARG: + has: + regex: ^(1|1L)$ +fix: rowMeans(~~INPUT~~) +message: Use rowMeans(x) rather than apply(x, 1, mean). + +--- + +id: matrix_apply-6 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, mean, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, mean, na.rm = $NARM) + - pattern: apply($INPUT, $MARG, FUN = mean, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = mean, na.rm = $NARM) +constraints: + MARG: + has: + regex: ^(1|1L)$ +fix: rowMeans(~~INPUT~~, na.rm = ~~NARM~~) +message: Use rowMeans(x, na.rm = ~~NARM~~) rather than apply(x, 1, mean, na.rm = ~~NARM~~). + +--- + +id: matrix_apply-7 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, mean) + - pattern: apply($INPUT, MARGIN = $MARG, mean) + - pattern: apply($INPUT, $MARG, FUN = mean) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = mean) +constraints: + MARG: + has: + regex: ^(2|2L)$ +fix: colMeans(~~INPUT~~) +message: Use colMeans(x) rather than apply(x, 2, mean). + +--- + +id: matrix_apply-8 +language: r +severity: warning +rule: + any: + - pattern: apply($INPUT, $MARG, mean, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, mean, na.rm = $NARM) + - pattern: apply($INPUT, $MARG, FUN = mean, na.rm = $NARM) + - pattern: apply($INPUT, MARGIN = $MARG, FUN = mean, na.rm = $NARM) +constraints: + MARG: + has: + regex: ^(2|2L)$ +fix: colMeans(~~INPUT~~, na.rm = ~~NARM~~) +message: Use colMeans(x, na.rm = ~~NARM~~) rather than apply(x, 2, mean, na.rm = ~~NARM~~). + diff --git a/flir/rules/builtin/missing_argument.yml b/flir/rules/builtin/missing_argument.yml new file mode 100644 index 0000000..9f47d17 --- /dev/null +++ b/flir/rules/builtin/missing_argument.yml @@ -0,0 +1,44 @@ +id: missing_argument-1 +language: r +severity: warning +rule: + kind: arguments + has: + kind: comma + any: + - precedes: + stopBy: neighbor + any: + - regex: '^\)$' + - kind: comma + - follows: + any: + - regex: '^\($' + - kind: argument + regex: '=$' + follows: + kind: identifier + not: + regex: '^(quote|switch|alist)$' + inside: + kind: call +message: Missing argument in function call. + +--- + +id: missing_argument-2 +language: r +severity: warning +rule: + kind: arguments + regex: '=(\s+|)\)$' + follows: + any: + - kind: identifier + - kind: extract_operator + - kind: namespace_operator + not: + regex: '^(quote|switch|alist)$' + inside: + kind: call +message: Missing argument in function call. diff --git a/flir/rules/builtin/nested_ifelse.yml b/flir/rules/builtin/nested_ifelse.yml new file mode 100644 index 0000000..64dcb08 --- /dev/null +++ b/flir/rules/builtin/nested_ifelse.yml @@ -0,0 +1,29 @@ +id: nested_ifelse-1 +language: r +severity: warning +rule: + pattern: $FUN($COND, $TRUE, $FALSE) +constraints: + FALSE: + regex: ^(ifelse|if_else|fifelse) + FUN: + regex: ^(ifelse|if_else|fifelse) +message: | + Don't use nested ~~FUN~~() calls; instead, try (1) data.table::fcase; + (2) dplyr::case_when; or (3) using a lookup table. + +--- + +id: nested_ifelse-2 +language: r +severity: warning +rule: + pattern: $FUN($COND, $TRUE, $FALSE) +constraints: + TRUE: + regex: ^(ifelse|if_else|fifelse) + FUN: + regex: ^(ifelse|if_else|fifelse) +message: | + Don't use nested ~~FUN~~() calls; instead, try (1) data.table::fcase; + (2) dplyr::case_when; or (3) using a lookup table. diff --git a/flir/rules/builtin/numeric_leading_zero.yml b/flir/rules/builtin/numeric_leading_zero.yml new file mode 100644 index 0000000..1d6762d --- /dev/null +++ b/flir/rules/builtin/numeric_leading_zero.yml @@ -0,0 +1,11 @@ +id: numeric_leading_zero-1 +language: r +severity: warning +rule: + pattern: $VALUE + any: + - kind: float + - kind: identifier + regex: ^\.[0-9] +fix: 0~~VALUE~~ +message: Include the leading zero for fractional numeric constants. diff --git a/flir/rules/builtin/outer_negation.yml b/flir/rules/builtin/outer_negation.yml new file mode 100644 index 0000000..ca377de --- /dev/null +++ b/flir/rules/builtin/outer_negation.yml @@ -0,0 +1,29 @@ +id: outer_negation-1 +language: r +severity: warning +rule: + pattern: all(!$VAR) +constraints: + VAR: + not: + regex: '^!' +fix: '!any(~~VAR~~)' +message: | + !any(x) is better than all(!x). The former applies negation only once after + aggregation instead of many times for each element of x. + +--- + +id: outer_negation-2 +language: r +severity: warning +rule: + pattern: any(! $VAR) +constraints: + VAR: + not: + regex: '^!' +fix: '!all(~~VAR~~)' +message: | + !all(x) is better than any(!x). The former applies negation only once after + aggregation instead of many times for each element of x. diff --git a/flir/rules/builtin/package_hooks.yml b/flir/rules/builtin/package_hooks.yml new file mode 100644 index 0000000..f4dfa75 --- /dev/null +++ b/flir/rules/builtin/package_hooks.yml @@ -0,0 +1,127 @@ +id: package_hooks-1 +language: r +severity: warning +rule: + pattern: packageStartupMessage($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: .onLoad +message: Put packageStartupMessage() calls in .onAttach(), not .onLoad(). + +--- + +id: package_hooks-2 +language: r +severity: warning +rule: + pattern: library.dynam($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: .onAttach +message: Put library.dynam() calls in .onLoad(), not .onAttach(). + +--- + +id: package_hooks-3 +language: r +severity: warning +rule: + pattern: $FN($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: .onLoad +constraints: + FN: + regex: '^(cat|installed.packages|message|packageStartupMessage|print|writeLines)$' +message: Don't use ~~FN~~() in .onLoad(). + +--- + +id: package_hooks-4 +language: r +severity: warning +rule: + pattern: $FN($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: .onAttach +constraints: + FN: + # library.dynam already has its own linter + regex: '^(cat|installed.packages|message|print|writeLines)$' +message: Don't use ~~FN~~() in .onAttach(). + +--- + +id: package_hooks-5 +language: r +severity: warning +rule: + pattern: $FN($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: $LOAD +constraints: + LOAD: + regex: '^(\.onAttach|\.onLoad)$' + FN: + regex: '^(require|library)$' +message: Don't alter the search() path in ~~LOAD~~() by calling ~~FN~~(). + +--- + +id: package_hooks-6 +language: r +severity: warning +rule: + pattern: installed.packages($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: $LOAD +constraints: + LOAD: + regex: '^(\.onAttach|\.onLoad)$' +message: Don't slow down package load by running installed.packages() in ~~LOAD~~(). + +--- + +id: package_hooks-7 +language: r +severity: warning +rule: + pattern: library.dynam.unload($$$) + inside: + stopBy: end + kind: binary_operator + has: + stopBy: end + field: lhs + pattern: $LOAD +constraints: + LOAD: + regex: '^(\.onDetach|\.Last\.lib)$' +message: Use library.dynam.unload() calls in .onUnload(), not ~~LOAD~~(). diff --git a/flir/rules/builtin/paste.yml b/flir/rules/builtin/paste.yml new file mode 100644 index 0000000..800b676 --- /dev/null +++ b/flir/rules/builtin/paste.yml @@ -0,0 +1,75 @@ +id: paste-1 +language: r +severity: warning +rule: + pattern: + context: paste($$$CONTENT sep = "" $$$CONTENT2) + strictness: ast +# fix: paste0($$$CONTENT) +message: paste0(...) is better than paste(..., sep = ""). + +--- + +id: paste-2 +language: r +severity: warning +rule: + any: + - pattern: + context: paste($CONTENT, collapse = ", ") + strictness: ast + - pattern: + context: paste(collapse = ", ", $CONTENT) + strictness: ast +# fix: paste0($$$CONTENT) +message: toString(.) is more expressive than paste(., collapse = ", "). + +--- + +id: paste-3 +language: r +severity: warning +rule: + pattern: + context: paste0($$$CONTENT sep = $USELESS $$$CONTENT2) + strictness: ast +# fix: paste0($$$CONTENT) +message: | + sep= is not a formal argument to paste0(); did you mean to use paste(), or + collapse=? + +--- + +id: paste-4 +language: r +severity: warning +rule: + any: + - pattern: + context: paste0($CONTENT, collapse = $FOO) + strictness: ast + - pattern: + context: paste0(collapse = $FOO, $CONTENT) + strictness: ast + not: + has: + regex: sep + kind: argument +# fix: paste0($$$CONTENT) +message: | + Use paste(), not paste0(), to collapse a character vector when sep= is not used. + +# --- +# +# id: paste-5 +# language: r +# severity: warning +# rule: +# pattern: +# context: paste0(rep($VAR, $TIMES), collapse = "") +# strictness: ast +# constraints: +# VAR: +# kind: string +# fix: strrep(~~VAR~~, ~~TIMES~~) +# message: strrep(x, times) is better than paste0(rep(x, times), collapse = ""). diff --git a/flir/rules/builtin/redundant_equals.yml b/flir/rules/builtin/redundant_equals.yml new file mode 100644 index 0000000..79a6abc --- /dev/null +++ b/flir/rules/builtin/redundant_equals.yml @@ -0,0 +1,29 @@ +id: redundant_equals-1 +language: r +severity: warning +rule: + any: + - pattern: $VAR == TRUE + - pattern: TRUE == $VAR + - pattern: $VAR == FALSE + - pattern: FALSE == $VAR +message: | + Using == on a logical vector is redundant. Well-named logical vectors can be + used directly in filtering. For data.table's `i` argument, wrap the column + name in (), like `DT[(is_treatment)]`. + +--- + +id: redundant_equals-2 +language: r +severity: warning +rule: + any: + - pattern: $VAR != TRUE + - pattern: TRUE != $VAR + - pattern: $VAR != FALSE + - pattern: FALSE != $VAR +message: | + Using != on a logical vector is redundant. Well-named logical vectors can be + used directly in filtering. For data.table's `i` argument, wrap the column + name in (), like `DT[(is_treatment)]`. diff --git a/flir/rules/builtin/redundant_ifelse.yml b/flir/rules/builtin/redundant_ifelse.yml new file mode 100644 index 0000000..8f252e6 --- /dev/null +++ b/flir/rules/builtin/redundant_ifelse.yml @@ -0,0 +1,67 @@ +id: redundant_ifelse-1 +language: r +severity: warning +rule: + pattern: $FUN($COND, $VAL1, $VAL2) +constraints: + VAL1: + regex: ^TRUE$ + VAL2: + regex: ^FALSE$ + FUN: + regex: ^(ifelse|fifelse|if_else)$ +fix: ~~COND~~ +message: | + Use ~~COND~~ directly instead of calling ~~FUN~~(~~COND~~, TRUE, FALSE). + +--- + +id: redundant_ifelse-2 +language: r +severity: warning +rule: + pattern: $FUN($COND, $VAL1, $VAL2) +constraints: + VAL1: + regex: ^FALSE$ + VAL2: + regex: ^TRUE$ + FUN: + regex: ^(ifelse|fifelse|if_else)$ +fix: '!(~~COND~~)' +message: | + Use !(~~COND~~) directly instead of calling ~~FUN~~(~~COND~~, FALSE, TRUE). + +--- + +id: redundant_ifelse-3 +language: r +severity: warning +rule: + pattern: $FUN($COND, $VAL1, $VAL2) +constraints: + VAL1: + regex: ^(1|1L)$ + VAL2: + regex: ^(0|0L)$ + FUN: + regex: ^(ifelse|fifelse|if_else)$ +fix: as.integer(~~COND~~) +message: Prefer as.integer(~~COND~~) to ~~FUN~~(~~COND~~, ~~VAL1~~, ~~VAL2~~). + +--- + +id: redundant_ifelse-4 +language: r +severity: warning +rule: + pattern: $FUN($COND, $VAL1, $VAL2) +constraints: + VAL1: + regex: ^(0|0L)$ + VAL2: + regex: ^(1|1L)$ + FUN: + regex: ^(ifelse|fifelse|if_else)$ +fix: as.integer(!(~~COND~~)) +message: Prefer as.integer(!(~~COND~~)) to ~~FUN~~(~~COND~~, ~~VAL1~~, ~~VAL2~~). diff --git a/flir/rules/builtin/rep_len.yml b/flir/rules/builtin/rep_len.yml new file mode 100644 index 0000000..d4d78e5 --- /dev/null +++ b/flir/rules/builtin/rep_len.yml @@ -0,0 +1,7 @@ +id: rep_len-1 +language: r +severity: warning +rule: + pattern: rep($OBJ, length.out = $LEN) +fix: rep_len(~~OBJ~~, ~~LEN~~) +message: Use rep_len(x, n) instead of rep(x, length.out = n). diff --git a/flir/rules/builtin/right_assignment.yml b/flir/rules/builtin/right_assignment.yml new file mode 100644 index 0000000..76b736e --- /dev/null +++ b/flir/rules/builtin/right_assignment.yml @@ -0,0 +1,10 @@ +id: right_assignment +language: r +severity: hint +rule: + pattern: $RHS -> $LHS + has: + field: rhs + kind: identifier +fix: ~~LHS~~<- ~~RHS~~ +message: Use <-, not ->, for assignment. diff --git a/flir/rules/builtin/sample_int.yml b/flir/rules/builtin/sample_int.yml new file mode 100644 index 0000000..091825c --- /dev/null +++ b/flir/rules/builtin/sample_int.yml @@ -0,0 +1,43 @@ +id: sample_int-1 +language: r +severity: warning +rule: + any: + - pattern: sample(1:$N, $$$OTHER) + - pattern: sample(1L:$N, $$$OTHER) +fix: sample.int(~~N~~, ~~OTHER~~) +message: sample.int(n, m, ...) is preferable to sample(1:n, m, ...). + +--- + +id: sample_int-2 +language: r +severity: warning +rule: + pattern: sample(seq($N), $$$OTHER) +fix: sample.int(~~N~~, ~~OTHER~~) +message: sample.int(n, m, ...) is preferable to sample(seq(n), m, ...). + +--- + +id: sample_int-3 +language: r +severity: warning +rule: + pattern: sample(seq_len($N), $$$OTHER) +fix: sample.int(~~N~~, ~~OTHER~~) +message: sample.int(n, m, ...) is preferable to sample(seq_len(n), m, ...). + +--- + +# Strangely this panicks if I rename FIRST to N +id: sample_int-4 +language: r +severity: warning +rule: + pattern: sample($FIRST, $$$OTHER) +constraints: + FIRST: + regex: ^\d+(L|)$ +fix: sample.int(~~N~~, ~~OTHER~~) +message: sample.int(n, m, ...) is preferable to sample(n, m, ...). diff --git a/flir/rules/builtin/semicolon.yml b/flir/rules/builtin/semicolon.yml new file mode 100644 index 0000000..fb5dd75 --- /dev/null +++ b/flir/rules/builtin/semicolon.yml @@ -0,0 +1,10 @@ +id: semicolon-1 +language: r +severity: warning +rule: + regex: ;\s+$ + not: + inside: + kind: string + stopBy: end +message: Trailing semicolons are not needed. diff --git a/flir/rules/builtin/seq.yml b/flir/rules/builtin/seq.yml new file mode 100644 index 0000000..c199c5d --- /dev/null +++ b/flir/rules/builtin/seq.yml @@ -0,0 +1,121 @@ +id: seq-1 +language: r +severity: warning +rule: + pattern: seq(length($VAR)) +fix: seq_along(~~VAR~~) +message: | + seq(length(...)) is likely to be wrong in the empty edge case. Use seq_along(...) instead. + +--- + +id: seq-2 +language: r +severity: warning +rule: + any: + - pattern: 1:nrow($VAR) + - pattern: 1L:nrow($VAR) + regex: ^1 +fix: seq_len(nrow(~~VAR~~)) +message: | + 1:nrow(...) is likely to be wrong in the empty edge case. Use seq_len(nrow(...)) instead. + +--- + +id: seq-3 +language: r +severity: warning +rule: + any: + - pattern: 1:n() + - pattern: 1L:n() + regex: ^1 +fix: seq_len(n()) +message: | + 1:n() is likely to be wrong in the empty edge case. Use seq_len(n()) instead. + +--- + +id: seq-4 +language: r +severity: warning +rule: + pattern: seq(nrow($VAR)) +fix: seq_len(nrow(~~VAR~~)) +message: | + seq(nrow(...)) is likely to be wrong in the empty edge case. Use seq_len(nrow(...)) instead. + +--- + +id: seq-5 +language: r +severity: warning +rule: + any: + - pattern: 1:length($VAR) + - pattern: 1L:length($VAR) + regex: ^1 +fix: seq_along(~~VAR~~) +message: | + 1:length(...) is likely to be wrong in the empty edge case. Use seq_along(...) instead. + +--- + +id: seq-6 +language: r +severity: warning +rule: + any: + - pattern: 1:ncol($VAR) + - pattern: 1L:ncol($VAR) + regex: ^1 +fix: seq_len(ncol(~~VAR~~)) +message: | + 1:ncol(...) is likely to be wrong in the empty edge case. Use seq_len(ncol(...)) instead. + +--- + +id: seq-7 +language: r +severity: warning +rule: + any: + - pattern: 1:NCOL($VAR) + - pattern: 1L:NCOL($VAR) + regex: ^1 +fix: seq_len(NCOL(~~VAR~~)) +message: | + 1:NCOL(...) is likely to be wrong in the empty edge case. Use seq_len(NCOL(...)) instead. + +--- + +id: seq-8 +language: r +severity: warning +rule: + any: + - pattern: 1:NROW($VAR) + - pattern: 1L:NROW($VAR) + regex: ^1 +fix: seq_len(NROW(~~VAR~~)) +message: | + 1:NROW(...) is likely to be wrong in the empty edge case. Use seq_len(NROW(...)) instead. + + +--- + +id: seq-9 +language: r +severity: warning +rule: + pattern: seq(1, $VAL) + not: + pattern: seq(1, 0) +constraints: + VAL: + regex: ^\d+(|L)$ +fix: seq_len(~~VAL~~) +message: seq_len(~~VAL~~) is more efficient than seq(1, ~~VAL~~). + + diff --git a/flir/rules/builtin/sort.yml b/flir/rules/builtin/sort.yml new file mode 100644 index 0000000..930f5c6 --- /dev/null +++ b/flir/rules/builtin/sort.yml @@ -0,0 +1,85 @@ +id: sort-1 +language: r +severity: warning +rule: + pattern: $OBJ[order($OBJ)] +fix: sort(~~OBJ~~, na.last = TRUE) +message: sort(~~OBJ~~, na.last = TRUE) is better than ~~OBJ~~[order(~~OBJ~~)]. + +--- + +id: sort-2 +language: r +severity: warning +rule: + any: + - pattern: $OBJ[order($OBJ, decreasing = $DECREASING)] + - pattern: $OBJ[order(decreasing = $DECREASING, $OBJ)] +constraints: + DECREASING: + regex: ^(TRUE|FALSE)$ +fix: sort(~~OBJ~~, decreasing = ~~DECREASING~~, na.last = TRUE) +message: | + sort(~~OBJ~~, decreasing = ~~DECREASING~~, na.last = TRUE) is better than + ~~OBJ~~[order(~~OBJ~~, decreasing = ~~DECREASING~~)]. + +--- + +id: sort-3 +language: r +severity: warning +rule: + any: + - pattern: $OBJ[order($OBJ, na.last = $NALAST)] + - pattern: $OBJ[order(na.last = $NALAST, $OBJ)] +constraints: + NALAST: + regex: ^(TRUE|FALSE)$ +fix: sort(~~OBJ~~, na.last = ~~NALAST~~, na.last = TRUE) +message: | + sort(~~OBJ~~, na.last = ~~NALAST~~, na.last = TRUE) is better than + ~~OBJ~~[order(~~OBJ~~, na.last = ~~NALAST~~)]. + +--- + +id: sort-4 +language: r +severity: warning +rule: + any: + - pattern: $OBJ[order($OBJ, decreasing = TRUE, na.last = FALSE)] + - pattern: $OBJ[order($OBJ, na.last = FALSE, decreasing = TRUE)] + - pattern: $OBJ[order(decreasing = TRUE, $OBJ, na.last = FALSE)] + - pattern: $OBJ[order(decreasing = TRUE, na.last = FALSE, $OBJ)] + - pattern: $OBJ[order(na.last = FALSE, decreasing = TRUE, $OBJ)] + - pattern: $OBJ[order(na.last = FALSE, $OBJ, decreasing = TRUE)] +fix: sort(~~OBJ~~, decreasing = TRUE, na.last = FALSE) +message: | + sort(~~OBJ~~, decreasing = TRUE, na.last = FALSE) is better than + ~~OBJ~~[order(~~OBJ~~, na.last = FALSE, decreasing = TRUE)]. + +--- + +id: sort-5 +language: r +severity: warning +rule: + any: + - pattern: sort($OBJ) == $OBJ + - pattern: $OBJ == sort($OBJ) +fix: !is.unsorted(~~OBJ~~) +message: | + Use !is.unsorted(~~OBJ~~) to test the sortedness of a vector. + +--- + +id: sort-6 +language: r +severity: warning +rule: + any: + - pattern: sort($OBJ) != $OBJ + - pattern: $OBJ != sort($OBJ) +fix: is.unsorted(~~OBJ~~) +message: | + Use is.unsorted(~~OBJ~~) to test the unsortedness of a vector. diff --git a/flir/rules/builtin/stopifnot_all.yml b/flir/rules/builtin/stopifnot_all.yml new file mode 100644 index 0000000..6f6619b --- /dev/null +++ b/flir/rules/builtin/stopifnot_all.yml @@ -0,0 +1,24 @@ +id: stopifnot_all-1 +language: r +severity: warning +rule: + pattern: stopifnot(all($$$CODE)) +fix: stopifnot(~~CODE~~) +message: | + Use stopifnot(x) instead of stopifnot(all(x)). stopifnot(x) runs all() 'under + the hood' and provides a better error message in case of failure. + +--- + +id: stopifnot_all-2 +language: r +severity: warning +rule: + pattern: stopifnot(exprs = { all($$$CODE) }) +fix: | + stopifnot(exprs = { + ~~CODE~~ + }) +message: | + Use stopifnot(x) instead of stopifnot(all(x)). stopifnot(x) runs all() 'under + the hood' and provides a better error message in case of failure. diff --git a/flir/rules/builtin/todo_comment.yml b/flir/rules/builtin/todo_comment.yml new file mode 100644 index 0000000..83d86ed --- /dev/null +++ b/flir/rules/builtin/todo_comment.yml @@ -0,0 +1,7 @@ +id: todo_comment-1 +language: r +severity: warning +rule: + kind: comment + regex: '(?i)#(|\s+)\b(todo|fixme)\b' +message: Remove TODO comments. diff --git a/flir/rules/builtin/undesirable_function.yml b/flir/rules/builtin/undesirable_function.yml new file mode 100644 index 0000000..c9b1756 --- /dev/null +++ b/flir/rules/builtin/undesirable_function.yml @@ -0,0 +1,13 @@ +id: undesirable_function-1 +language: r +severity: warning +rule: + pattern: $FUN + kind: identifier + not: + inside: + kind: argument +constraints: + FUN: + regex: ^(\.libPaths|attach|browser|debug|debugcall|debugonce|detach|par|setwd|structure|Sys\.setenv|Sys\.setlocale|trace|undebug|untrace)$ +message: Function "~~FUN~~()" is undesirable. diff --git a/flir/rules/builtin/undesirable_operator.yml b/flir/rules/builtin/undesirable_operator.yml new file mode 100644 index 0000000..7d63513 --- /dev/null +++ b/flir/rules/builtin/undesirable_operator.yml @@ -0,0 +1,29 @@ +id: undesirable_operator-1 +language: r +severity: warning +rule: + any: + - pattern: $X <<- $Y + - pattern: $X ->> $Y +message: | + Avoid undesirable operators `<<-` and `->>`. They assign outside the current + environment in a way that can be hard to reason about. Prefer fully-encapsulated + functions wherever possible, or, if necessary, assign to a specific environment + with assign(). Recall that you can create an environment at the desired scope + with new.env(). + +--- + +id: undesirable_operator-2 +language: r +severity: warning +rule: + kind: namespace_operator + has: + pattern: ':::' +message: | + Operator `:::` is undesirable. It accesses non-exported functions inside + packages. Code relying on these is likely to break in future versions of the + package because the functions are not part of the public interface and may be + changed or removed by the maintainers without notice. Use public functions + via :: instead. diff --git a/flir/rules/builtin/unnecessary_nesting.yml b/flir/rules/builtin/unnecessary_nesting.yml new file mode 100644 index 0000000..b56b467 --- /dev/null +++ b/flir/rules/builtin/unnecessary_nesting.yml @@ -0,0 +1,36 @@ +id: unnecessary_nesting-1 +language: r +severity: warning +rule: + kind: if_statement + any: + - has: + kind: 'braced_expression' + field: consequence + has: + kind: if_statement + stopBy: neighbor + not: + has: + kind: 'braced_expression' + field: alternative + stopBy: end + not: + any: + - has: + nthChild: 2 + - precedes: + regex: "^else$" + - has: + kind: if_statement + field: consequence + stopBy: neighbor + # Can be in if(), but not else if() + not: + inside: + field: alternative + kind: if_statement +message: | + Don't use nested `if` statements, where a single `if` with the combined + conditional expression will do. For example, instead of `if (x) { if (y) { ... }}`, + use `if (x && y) { ... }`. diff --git a/flir/rules/builtin/unreachable_code.yml b/flir/rules/builtin/unreachable_code.yml new file mode 100644 index 0000000..56eb497 --- /dev/null +++ b/flir/rules/builtin/unreachable_code.yml @@ -0,0 +1,64 @@ +id: unreachable_code-1 +language: r +severity: warning +rule: + regex: '[^}]+' + not: + regex: 'else' + follows: + any: + - pattern: return($$$A) + - pattern: stop($$$A) + not: + precedes: + regex: 'else' + stopBy: end +message: Code and comments coming after a return() or stop() should be removed. + +--- + +id: unreachable_code-2 +language: r +severity: warning +rule: + regex: '[^}]+' + not: + regex: 'else' + follows: + any: + - pattern: next + - pattern: break + stopBy: end +message: Remove code and comments coming after `next` or `break` + +--- + +id: unreachable_code-3 +language: r +severity: warning +rule: + inside: + any: + - kind: if_statement + pattern: if (FALSE) + - kind: while_statement + pattern: while (FALSE) + stopBy: end +message: Remove code inside a conditional loop with a deterministically false condition. + +--- + +id: unreachable_code-4 +language: r +severity: warning +rule: + inside: + any: + - kind: if_statement + pattern: if (TRUE) + - kind: while_statement + pattern: while (TRUE) + stopBy: end +message: | + One branch has a a deterministically true condition. The other branches can + be removed. diff --git a/flir/rules/builtin/which_grepl.yml b/flir/rules/builtin/which_grepl.yml new file mode 100644 index 0000000..81e30f2 --- /dev/null +++ b/flir/rules/builtin/which_grepl.yml @@ -0,0 +1,7 @@ +id: which_grepl-1 +language: r +severity: warning +rule: + pattern: which(grepl($$$ARGS)) +fix: grep(~~ARGS~~) +message: grep(pattern, x) is better than which(grepl(pattern, x)). diff --git a/getCRUCLdata.Rproj b/getCRUCLdata.Rproj new file mode 100644 index 0000000..25074f7 --- /dev/null +++ b/getCRUCLdata.Rproj @@ -0,0 +1,21 @@ +Version: 1.0 +ProjectId: b876aac1-663c-4e95-b4b7-a129e578fd9b + +RestoreWorkspace: Default +SaveWorkspace: Default +AlwaysSaveHistory: Default + +EnableCodeIndexing: Yes +UseSpacesForTab: Yes +NumSpacesForTab: 2 +Encoding: UTF-8 + +RnwWeave: Sweave +LaTeX: pdfLaTeX + +AutoAppendNewline: Yes +StripTrailingWhitespace: Yes + +BuildType: Package +PackageUseDevtools: Yes +PackageInstallArgs: --no-multiarch --with-keep.source