fix corner cases of wb_dims + standardize error messages

NEWS.md

@@ -9,6 +9,14 @@
 
 * Allow writing data frames with zero rows. [987](https://github.com/JanMarvin/openxlsx2/pull/987)
 
+* `wb_dims()` has been improved and is safer on 0-length inputs. In particular, it will error for a case where a `cols` doesn't exist in `x`
+
+```r
+# Previously created a wrong dims
+wb_dims(x = mtcars, cols = "non-existent-col")
+# Now errors
+```
+
 
 ***************************************************************************
 

R/utils.R

@@ -280,9 +280,9 @@ check_wb_dims_args <- function(args, select = NULL) {
 
   x_has_colnames <- !is.null(colnames(args$x))
 
-  if (x_has_colnames && !is.null(args$rows) && is.character(args$rows)) {
+  if (is.character(args$rows) && !is.null(args$rows) && x_has_colnames) {
     # Not checking whether it's a row name, not supported.
-    is_rows_a_colname <- args$row %in% colnames(args$x)
+    is_rows_a_colname <- args$rows %in% colnames(args$x)
 
     if (any(is_rows_a_colname)) {
       stop(
@@ -292,6 +292,15 @@ check_wb_dims_args <- function(args, select = NULL) {
       )
     }
   }
+
+  if (is.character(args$cols) && x_has_colnames && !all(args$cols %in% colnames(args$x))) {
+    # Checking whether cols is character, and error if it is not the col names of x
+    stop(
+      "`cols` must be an integer or an existing column name of `x`, not ", args$cols,
+      call. = FALSE
+    )
+  }
+
   invisible(NULL)
 }
 
@@ -699,7 +708,7 @@ wb_dims <- function(..., select = NULL) {
   # After this point, all unnamed problems are solved ;)
   x <- args$x
   if (!is.null(select) && is.null(args$x)) {
-    stop("`select` must only be provided with `x`.")
+    stop("Can't supply `select` when `x` is absent.")
   }
 
   # little helper that streamlines which inputs cannot be
@@ -731,7 +740,7 @@ wb_dims <- function(..., select = NULL) {
   col_names <- args$col_names %||% x_has_named_dims
 
   if (!cnam_null && !x_has_named_dims) {
-    stop("Supplying `col_names` when `x` is a vector is not supported.")
+    stop("Can't supply `col_names` when `x` is a vector.\n", "Transform `x` to a data.frame")
   }
 
   row_names <- args$row_names %||% FALSE
@@ -842,6 +851,16 @@ wb_dims <- function(..., select = NULL) {
     }
   }
 
+  # Correct for empty data frame?
+  # patch so that we will want to span at least 1 row and one col
+  if (identical(nrow_to_span, 0L)) {
+    nrow_to_span <- 1L
+  }
+
+  if (identical(ncol_to_span, 0L)) {
+    ncol_to_span <- 1L
+  }
+
   row_span <- frow + seq_len(nrow_to_span) - 1L
   col_span <- fcol + seq_len(ncol_to_span) - 1L
 

tests/testthat/test-utils.R

@@ -115,7 +115,7 @@ test_that("wb_dims() works when not supplying `x`.", {
   expect_error(wb_dims(3, 0))
   expect_error(wb_dims(1, 1, col_names = TRUE))
   expect_error(wb_dims(1, 1, row_names = FALSE), "`row_names`")
-  expect_error(wb_dims(2, 10, select = "col_names"), "`select` must only be provided with `x`")
+  expect_error(wb_dims(2, 10, select = "col_names"), "Can't supply `select` when `x` is absent")
 
 })
 
@@ -181,7 +181,7 @@ test_that("`wb_dims()` works when Supplying an object `x`.", {
   # doesn't work
   expect_equal(wb_dims(x = letters), "A1:A26")
   expect_equal(wb_dims(x = t(letters), col_names = TRUE), "A1:Z2")
-  expect_error(wb_dims(x = letters, col_names = TRUE), "Supplying `col_names` when `x` is a vector is not supported.") # don't want this error anymore.
+  expect_error(wb_dims(x = letters, col_names = TRUE), "Can't supply `col_names` when `x` is a vector") # don't want this error anymore.
 
   expect_equal(wb_dims(x = mtcars, rows = 5, from_col = "C"), "C6:M6")
 
@@ -214,6 +214,8 @@ test_that("`wb_dims()` works when Supplying an object `x`.", {
   # use 1 column name works
 
   expect_error(wb_dims(cols = "hp"), "Supplying a single argument")
+  # using non-existing character column doesn't work
+  expect_error(wb_dims(x = mtcars, cols = "A"), "`cols` must be an integer or an existing column name of `x`.")
   expect_error(
     wb_dims(x = mtcars, cols = c("hp", "vs")),
     regexp = "Supplying multiple column names is not supported"
@@ -262,6 +264,10 @@ test_that("`wb_dims()` handles row_names = TRUE consistenly.", {
   # Style row names of an object
 })
 
+test_that("wb_dims() handles empty data frames", {
+  expect_equal(wb_dims(x = data.frame(), from_col = "B"), "B1")
+})
+
 test_that("wb_dims() handles `from_dims`", {
   expect_equal(
     wb_dims(from_dims = "A3"),