From 956815e150522ae9e4dc085037d6003c520b341e Mon Sep 17 00:00:00 2001 From: Jan Marvin Garbuszus Date: Fri, 14 Jul 2023 09:40:28 +0200 Subject: [PATCH] warn on unused arguments in standardize_case_names() --- R/class-workbook.R | 6 ++++-- R/standardize.R | 15 +++++++++++++-- R/wb_load.R | 2 +- R/wb_styles.R | 3 ++- R/write_xlsx.R | 14 ++++++++++++-- tests/testthat/test-class-hyperlink.R | 4 ++-- tests/testthat/test-class-workbook.R | 10 +++++----- tests/testthat/test-class-worksheet.R | 6 +++--- tests/testthat/test-save.R | 6 +++--- tests/testthat/test-standardize.R | 1 + 10 files changed, 46 insertions(+), 21 deletions(-) diff --git a/R/class-workbook.R b/R/class-workbook.R index 81c6ed372..e12a1ebb6 100644 --- a/R/class-workbook.R +++ b/R/class-workbook.R @@ -4069,7 +4069,8 @@ wbWorkbook <- R6::R6Class( ... ) { - standardize_case_names(...) + arguments <- c(ls(), "start_row", "start_col") + standardize_case_names(..., arguments = arguments) if (exists("start_row") && !is.null(start_row) && exists("start_col") && !is.null(start_col)) { @@ -5391,7 +5392,8 @@ wbWorkbook <- R6::R6Class( ... ) { - standardize_case_names(...) + arguments <- c(ls(), "rows", "cols") + standardize_case_names(..., arguments = arguments) sheet <- private$get_sheet_index(sheet) diff --git a/R/standardize.R b/R/standardize.R index 3c422f009..4104e2684 100644 --- a/R/standardize.R +++ b/R/standardize.R @@ -36,7 +36,11 @@ standardize_color_names <- function(..., return = FALSE) { #' @param ... ... #' @returns void. assigns an object in the parent frame #' @noRd -standardize_case_names <- function(..., return = FALSE) { +standardize_case_names <- function(..., return = FALSE, arguments = NULL) { + + if (is.null(arguments)) { + arguments <- ls(envir = parent.frame()) + } # since R 4.1.0: ...names() args <- list(...) @@ -58,6 +62,7 @@ standardize_case_names <- function(..., return = FALSE) { x = camel_case, perl = TRUE ) + got[got_camel_case] <- name_camel_case # since R 3.5.0: ...elt(got_col) if (return) { names(args)[got_camel_case] <- name_camel_case @@ -68,6 +73,11 @@ standardize_case_names <- function(..., return = FALSE) { } } + sel <- !got %in% arguments + if (any(sel)) { + warning("unused arguments (", paste(got[sel], collapse = ", "), ")") + } + if (return) args } @@ -79,9 +89,10 @@ standardize_case_names <- function(..., return = FALSE) { standardize <- function(...) { nms <- list(...) + arguments <- ls(envir = parent.frame()) rtns <- standardize_color_names(nms, return = TRUE) - rtns <- standardize_case_names(rtns, return = TRUE) + rtns <- standardize_case_names(rtns, return = TRUE, arguments = arguments) nms <- names(rtns) for (i in seq_along(nms)) { diff --git a/R/wb_load.R b/R/wb_load.R index 3ac4019a0..0c6c7b634 100644 --- a/R/wb_load.R +++ b/R/wb_load.R @@ -308,7 +308,7 @@ wb_load <- function( overrideAttr <- as.data.frame(do.call("rbind", override)) xmls <- basename(unlist(overrideAttr$PartName)) drawings <- grep("drawing", xmls, value = TRUE) - wb$add_worksheet(sheets$name[i], visible = is_visible[i], hasDrawing = !is.na(drawings[i])) + wb$add_worksheet(sheets$name[i], visible = is_visible[i], has_drawing = !is.na(drawings[i])) } } diff --git a/R/wb_styles.R b/R/wb_styles.R index 22e54a82e..533cbce3c 100644 --- a/R/wb_styles.R +++ b/R/wb_styles.R @@ -449,8 +449,9 @@ create_cell_style <- function( ) { n <- length(num_fmt_id) + arguments <- c(ls(), "is_cell_style_xf") + standardize_case_names(..., arguments = arguments) args <- list(...) - standardize_case_names(...) is_cell_style_xf <- isTRUE(args$is_cell_style_xf) diff --git a/R/write_xlsx.R b/R/write_xlsx.R index dd71ccfa9..677605ed1 100644 --- a/R/write_xlsx.R +++ b/R/write_xlsx.R @@ -97,13 +97,23 @@ write_xlsx <- function(x, file, as_table = FALSE, ...) { ## set scientific notation penalty + arguments <- c(ls(), "creator", "sheet_name", "grid_lines", + "tab_color", "tab_colour", + "zoom", "header", "footer", "even_header", "even_footer", "first_header", + "first_footer", "start_col", "start_row", + "col.names", "row.names", "col_names", "row_names", "table_style", + "table_name", "with_filter", "first_active_row", "first_active_col", + "first_row", "first_col", "col_widths", "na.strings", + "overwrite", "title", "subject", "category" + ) + params <- list(...) # we need them in params - params <- standardize_case_names(params, return = TRUE) + params <- standardize_case_names(params, arguments = arguments, return = TRUE) # and in global env for `asTable` - standardize_case_names(...) + standardize_case_names(..., arguments = arguments) ## Possible parameters diff --git a/tests/testthat/test-class-hyperlink.R b/tests/testthat/test-class-hyperlink.R index 9b9664177..199c3ee99 100644 --- a/tests/testthat/test-class-hyperlink.R +++ b/tests/testthat/test-class-hyperlink.R @@ -12,7 +12,7 @@ test_that("encode Hyperlink works", { add_worksheet("Tab_1", zoom = 80, gridLines = FALSE)$ add_data(x = rbind(2016:2019), dims = "C1:F1", colNames = FALSE)$ add_data(x = 2017, dims = "A1", colNames = FALSE)$ - add_data_validation(rows = 1, col = 1, type = "list", value = '"2016,2017,2018,2019"')$ + add_data_validation(dims = "A1", type = "list", value = '"2016,2017,2018,2019"')$ add_formula(dims = "B1", x = formula_old)$ add_formula(dims = "B2", x = formula_new) @@ -32,7 +32,7 @@ test_that("formulas with hyperlinks works", { add_worksheet("Tab_1", zoom = 80, gridLines = FALSE)$ add_data(dims = "C1:F1", x = rbind(2016:2019), colNames = FALSE)$ add_data(x = 2017, startCol = 1, startRow = 1, colNames = FALSE)$ - add_data_validation(rows = 1, col = 1, type = "list", value = '"2016,2017,2018,2019"')$ + add_data_validation(dims = "A1", type = "list", value = '"2016,2017,2018,2019"')$ add_formula(dims = "B1", x = '=HYPERLINK("#Tab_1!" & CELL("address", INDEX(C1:F1, MATCH(A1, C1:F1, 0))), "Go to the selected column")')$ add_formula(dims = "B2", x = '=IF(2017 = VALUE(A1), HYPERLINK("github.com","github.com"), A1)') diff --git a/tests/testthat/test-class-workbook.R b/tests/testthat/test-class-workbook.R index 407ac2bb9..ebb20e138 100644 --- a/tests/testthat/test-class-workbook.R +++ b/tests/testthat/test-class-workbook.R @@ -235,7 +235,7 @@ test_that("data validation", { wb <- wb_workbook()$ add_worksheet("Sheet 1")$ add_data_table(x = head(iris))$ - add_data_validation(col = "A", rows = 2:151, type = "whole", + add_data_validation(dims = "A2:A151", type = "whole", operator = "between", value = c(1, 9, 19) ), "length <= 2" @@ -246,7 +246,7 @@ test_that("data validation", { wb <- wb_workbook()$ add_worksheet("Sheet 1")$ add_data_table(x = head(iris))$ - add_data_validation(col = "A", rows = 2:151, type = "even", + add_data_validation(dims = "A2:A151", type = "even", operator = "between", value = c(1, 9) ), "Invalid 'type' argument!" @@ -257,7 +257,7 @@ test_that("data validation", { wb <- wb_workbook()$ add_worksheet("Sheet 1")$ add_data_table(x = head(iris))$ - add_data_validation(col = "A", rows = 2:151, type = "whole", + add_data_validation(dims = "A2:A151", type = "whole", operator = "lower", value = c(1, 9) ), "Invalid 'operator' argument!" @@ -269,7 +269,7 @@ test_that("data validation", { add_worksheet("Sheet 1")$ add_data_table(x = head(iris))$ # whole numbers are fine - add_data_validation(col = 1, rows = 2:12, type = "date", + add_data_validation(dims = "A2:A12", type = "date", operator = "greaterThanOrEqual", value = 7 ), "If type == 'date' value argument must be a Date vector" @@ -281,7 +281,7 @@ test_that("data validation", { add_worksheet("Sheet 1")$ add_data_table(x = head(iris))$ # whole numbers are fine - add_data_validation(col = 1, rows = 2:12, type = "time", + add_data_validation(dims = "A2:A12", type = "time", operator = "greaterThanOrEqual", value = 7 ), "If type == 'time' value argument must be a POSIXct or POSIXlt vector." diff --git a/tests/testthat/test-class-worksheet.R b/tests/testthat/test-class-worksheet.R index cf1dc2f0d..e75338112 100644 --- a/tests/testthat/test-class-worksheet.R +++ b/tests/testthat/test-class-worksheet.R @@ -14,9 +14,9 @@ test_that("test data validation list and sparklines", { wb <- wb_workbook()$ add_worksheet()$add_data(x = iris[1:30, ])$ add_worksheet()$add_data(sheet = 2, x = sample(iris$Sepal.Length, 10))$ - add_data_validation(sheet = 1, col = 1, rows = 2:11, type = "list", value = '"O1,O2"')$ + add_data_validation(sheet = 1, dims = "A2:A11", type = "list", value = '"O1,O2"')$ add_sparklines(sheet = 1, sparklines = s1)$ - add_data_validation(sheet = 1, col = 1, rows = 12:21, type = "list", value = '"O2,O3"')$ + add_data_validation(sheet = 1, dims = "A12:A21", type = "list", value = '"O2,O3"')$ add_sparklines(sheet = 1, sparklines = s2) exp <- c( @@ -35,7 +35,7 @@ test_that("old and new data validations", { add_worksheet()$ add_data(x = sample(c("O1", "O2"), 10, TRUE))$ add_data(dims = "B1", x = sample(c("O1", "O2"), 10, TRUE))$ - add_data_validation(sheet = 1, col = 2, rows = 1:10, type = "list", value = '"O1,O2"') + add_data_validation(sheet = 1, dims = "B1:B10", type = "list", value = '"O1,O2"') # add data validations list as x14. this was the default in openxlsx and openxlsx2 <= 0.3 wb$worksheets[[1]]$extLst <- "\"O1,O2\"A2:A11" diff --git a/tests/testthat/test-save.R b/tests/testthat/test-save.R index e09c3d7c3..c6ddd54ea 100644 --- a/tests/testthat/test-save.R +++ b/tests/testthat/test-save.R @@ -17,11 +17,11 @@ test_that("regression test for #248", { tempFile <- temp_xlsx() # no formatting - expect_silent(write_xlsx(df, tempFile, borders = "columns", overwrite = TRUE)) + expect_silent(write_xlsx(df, tempFile, overwrite = TRUE)) # Change column class to percentage class(df$percent) <- "percentage" - expect_silent(write_xlsx(df, tempFile, borders = "columns", overwrite = TRUE)) + expect_silent(write_xlsx(df, tempFile, overwrite = TRUE)) }) @@ -133,7 +133,7 @@ test_that("write xlsx", { expect_silent(write_xlsx(df, tmp, row.names = TRUE)) expect_error(write_xlsx(df, tmp, rowNames = "NO")) expect_silent(write_xlsx(df, tmp, rowNames = TRUE)) - expect_silent(write_xlsx(df, tmp, colWidth = "auto")) + expect_silent(write_xlsx(df, tmp, colWidths = "auto")) expect_silent(write_xlsx(list(df, df), tmp, firstActiveCol = 2, firstActiveRow = 2)) expect_silent(write_xlsx(list(df, df), tmp, firstCol = FALSE, firstRow = FALSE)) expect_silent(write_xlsx(list(df, df), tmp, firstCol = TRUE, firstRow = TRUE)) diff --git a/tests/testthat/test-standardize.R b/tests/testthat/test-standardize.R index c8a04c605..03aee7b56 100644 --- a/tests/testthat/test-standardize.R +++ b/tests/testthat/test-standardize.R @@ -9,6 +9,7 @@ test_that("standardize works", { expect_equal(get("tabColor"), "green") camelCase <- NULL + camel_case <- NULL standardize_case_names(camelCase = "green") expect_equal(get("camel_case"), "green")