Update tests + various lints (#922)

* Lints in R/ + avoid partial matching

* Use more snake_case in tests

* Have the table name related warnings and errors to show `table_name`.

* Test lints especially yoda linter

* Other test lints

* additional snake_case

* More lints

* lints

* address comments

* Forgotten

* oops

R/class-workbook.R

@@ -4784,7 +4784,7 @@ wbWorkbook <- R6::R6Class(
       pos <- '<xdr:pos x="0" y="0" />'
 
       drawingsXML <- stri_join(
-        '<xdr:absoluteAnchor>',
+        "<xdr:absoluteAnchor>",
         pos,
         sprintf('<xdr:ext cx="%s" cy="%s"/>', width, height),
         genBasePic(imageNo, next_id),

R/class-worksheet.R

@@ -174,7 +174,7 @@ wbWorksheet <- R6::R6Class(
 
       if (!is.null(tab_color)) {
         tab_color <- xml_node_create("tabColor", xml_attributes = tab_color)
-        tabColor <- sprintf('<sheetPr>%s</sheetPr>', tab_color)
+        tabColor <- sprintf("<sheetPr>%s</sheetPr>", tab_color)
       } else {
         tabColor <- character()
       }
@@ -280,7 +280,7 @@ wbWorksheet <- R6::R6Class(
         if (length(self$cols_attr)) {
           paste(c("<cols>", self$cols_attr, "</cols>"), collapse = "")
         },
-        '</worksheet>',
+        "</worksheet>",
         sep = ""
       )
     },

R/helper-functions.R

@@ -66,7 +66,7 @@ create_hyperlink <- function(sheet, row = 1, col = 1, text = NULL, file = NULL)
     }
 
     if (is.null(text)) {
-      str <- sprintf('=HYPERLINK(%s)', dest)
+      str <- sprintf("=HYPERLINK(%s)", dest)
     } else {
       str <- sprintf('=HYPERLINK(%s, \"%s\")', dest, text)
     }

R/read.R

@@ -331,8 +331,8 @@ wb_to_df <- function(
   cc_tab <- unique(cc$c_t)
 
   # bool
-  if (any(cc_tab == c("b"))) {
-    sel <- cc$c_t %in% c("b")
+  if (any(cc_tab == "b")) {
+    sel <- cc$c_t %in% "b"
     cc$val[sel] <- as.logical(as.numeric(cc$v[sel]))
     cc$typ[sel] <- "b"
   }

R/utils.R

@@ -798,6 +798,7 @@ read_xml_files <- function(x) {
 #' @noRd
 un_list <- function(x) {
 
+  # TODO can use `lengths()` when depending on R 4.0
   names <- vapply(x, length, NA_integer_)
   nams <- NULL
   for (i in seq_along(names)) {

tests/testthat/test-class-comment.R

@@ -181,10 +181,9 @@ test_that("print comment", {
 
   c2 <- create_comment(text = "this is another comment",
                        author = "Marco Polo")
-
-  exp <- "Author: Marco Polo\nText:\n Marco Polo:\nthis is another comment\n\nStyle:\n\n\n\n\nFont name: Aptos Narrow\nFont size: 11\nFont color: #000000\n\n"
   got <- capture_output(print(c2), print = TRUE)
-  expect_equal(exp, got)
+  exp <- "Author: Marco Polo\nText:\n Marco Polo:\nthis is another comment\n\nStyle:\n\n\n\n\nFont name: Aptos Narrow\nFont size: 11\nFont color: #000000\n\n"
+  expect_equal(got, exp)
 
 })
 
@@ -330,11 +329,11 @@ test_that("background images work", {
   wb$add_comment(dims = "G12", comment = c1, file = tmp2)
   wb$add_comment(dims = "G12", sheet = 1, comment = c1, file = tmp2)
 
-  expect_equal(3, length(wb$vml))
-  expect_equal(3, length(wb$vml_rels))
-  expect_equal(2, length(wb$vml_rels[[1]]))
-  expect_true(is.null(wb$vml_rels[[2]]))
-  expect_equal(1, length(wb$vml_rels[[3]]))
+  expect_length(wb$vml, 3)
+  expect_length(wb$vml_rels, 3)
+  expect_length(wb$vml_rels[[1]], 2)
+  expect_null(wb$vml_rels[[2]])
+  expect_length(wb$vml_rels[[3]], 1)
 
 })
 

tests/testthat/test-class-workbook.R

@@ -16,8 +16,8 @@ test_that("wb_set_col_widths", {
   # set column width to 12
   expect_silent(wb$set_col_widths("test", widths = 12L, cols = seq_along(mtcars)))
   expect_equal(
-    "<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"12.711\"/>",
-    wb$worksheets[[1]]$cols_attr
+    wb$worksheets[[1]]$cols_attr,
+    "<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"12.711\"/>"
   )
 
   # wrong sheet
@@ -26,8 +26,8 @@ test_that("wb_set_col_widths", {
   # reset the column with, we do not provide an option ot remove the column entry
   expect_silent(wb$set_col_widths("test", cols = seq_along(mtcars)))
   expect_equal(
-    "<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"9.141\"/>",
-    wb$worksheets[[1]]$cols_attr
+    wb$worksheets[[1]]$cols_attr,
+    "<col min=\"1\" max=\"11\" bestFit=\"1\" customWidth=\"1\" hidden=\"false\" width=\"9.141\"/>"
   )
 
   # create column width for column 25
@@ -360,7 +360,7 @@ test_that("clone worksheet", {
   wb <- wb_load(fl)
   # wb$get_sheet_names() # chartsheet has no named name?
   expect_silent(wb$clone_worksheet(1, "Clone 1"))
-  expect_true(inherits(wb$worksheets[[5]], "wbChartSheet"))
+  expect_s3_class(wb$worksheets[[5]], "wbChartSheet")
   # wb$open()
 
   # clone pivot table and drawing -----------------------------------------
@@ -502,7 +502,7 @@ test_that("add_drawing works", {
     add_drawing(xml = tmp, dims = NULL)$
     add_drawing(xml = tmp, dims = "L19")
 
-  expect_equal(1L, length(wb$drawings))
+  expect_length(wb$drawings, 1L)
 
 })
 
@@ -563,7 +563,7 @@ test_that("add_drawing works", {
   wb <- wb %>%
     wb_add_mschart(dims = "F4:L20", graph = scatter_plot)
 
-  expect_equal(1L, NROW(wb$charts))
+  expect_equal(NROW(wb$charts), 1L)
 
   chart_01 <- ms_linechart(
     data = us_indus_prod,
@@ -644,7 +644,7 @@ test_that("add_chartsheet works", {
 
   wb$add_mschart(graph = data_plot)
 
-  expect_equal(1, nrow(wb$charts))
+  expect_equal(nrow(wb$charts), 1)
 
   expect_true(grepl("A &amp; B", wb$charts$chart))
 
@@ -661,7 +661,7 @@ test_that("add_chartsheet works", {
   )
   wb$add_mschart(sheet = 2, graph = data_plot)
 
-  expect_equal(2L, nrow(wb$charts))
+  expect_equal(nrow(wb$charts), 2L)
 
   exp <- "xdr:absoluteAnchor"
   got <- xml_node_name(unlist(wb$drawings), "xdr:wsDr")
@@ -844,7 +844,7 @@ test_that("numfmt in pivot tables works", {
   ## Create the workbook and the pivot table
   wb <- wb_workbook()$
     add_worksheet("Data")$
-    add_data(x = df, startCol = 1, startRow = 2)
+    add_data(x = df, start_col = 1, start_row = 2)
 
   df <- wb_data(wb)
   wb$add_pivot_table(df, dims = "A3", rows = "cyl", cols = "gear",
@@ -883,7 +883,7 @@ test_that("numfmt in pivot tables works", {
   ## Create the workbook and the pivot table
   wb <- wb_workbook()$
     add_worksheet("Data")$
-    add_data(x = df, startCol = 1, startRow = 2)
+    add_data(x = df, start_col = 1, start_row = 2)
 
   df <- wb_data(wb)
   wb$add_pivot_table(
@@ -954,7 +954,9 @@ test_that("genBaseWorkbook() works", {
     "smartTagTypes", "webPublishing", "fileRecoveryPr", "webPublishObjects",
     "extLst"
   )
-  got <- names(genBaseWorkbook())
-  expect_equal(exp, got)
+  expect_equal(
+    names(genBaseWorkbook()),
+    exp
+  )
 
 })

tests/testthat/test-cloneWorksheet.R

@@ -31,8 +31,8 @@ test_that("clone Worksheet with table", {
 
   wb <- wb_workbook()
   wb$add_worksheet("Sheet 1")
-  wb$add_data_table(sheet = "Sheet 1", x = iris, tableName = "iris")
-  wb$add_data_table(sheet = 1, x = mtcars, tableName = "mtcars", startCol = 10)
+  wb$add_data_table(sheet = "Sheet 1", x = iris, table_name = "iris")
+  wb$add_data_table(sheet = 1, x = mtcars, table_name = "mtcars", start_col = 10)
 
   # FIXME a wild drawing2.xml appears in wb$Content_Types
   wb$clone_worksheet("Sheet 1", "Clone 1")
@@ -67,7 +67,7 @@ test_that("copy cells", {
     add_data(x = mtcars)$
     add_fill(dims = "A1:F1", color = wb_color("yellow"))
 
-  dat <- wb_data(wb, dims = "A1:D4", colNames = FALSE)
+  dat <- wb_data(wb, dims = "A1:D4", col_names = FALSE)
 
   # FIXME there is a bug with next_sheet() in clone_worksheets()
   wb$
@@ -87,20 +87,20 @@ test_that("copy cells", {
     clone_worksheet(old = 1, new = "Clone5")$
     copy_cells(data = dat, dims = "A20", as_value = TRUE, as_ref = FALSE, transpose = TRUE)
 
-  got <- wb_data(wb, sheet = 2, dims = "M2:P5", colNames = FALSE)
+  got <- wb_data(wb, sheet = 2, dims = "M2:P5", col_names = FALSE)
   expect_equal(dat, got, ignore_attr = TRUE)
 
-  got <- wb_data(wb, sheet = 3, dims = "A20:D23", colNames = FALSE)
+  got <- wb_data(wb, sheet = 3, dims = "A20:D23", col_names = FALSE)
   expect_equal(unlist(t(dat)), unlist(got), ignore_attr = TRUE)
 
   exp <- c("'Sheet 1'!A1", "'Sheet 1'!B1", "'Sheet 1'!C1", "'Sheet 1'!D1")
-  got <- wb_data(wb, sheet = 4, dims = "A20:D23", colNames = FALSE, showFormula = TRUE)[[1]]
+  got <- wb_data(wb, sheet = 4, dims = "A20:D23", col_names = FALSE, show_formula = TRUE)[[1]]
   expect_equal(exp, got)
 
-  got <- wb_data(wb, sheet = 5, dims = "A20:D23", colNames = FALSE)
+  got <- wb_data(wb, sheet = 5, dims = "A20:D23", col_names = FALSE)
   expect_equal(dat, got, ignore_attr = TRUE)
 
-  got <- wb_data(wb, sheet = 6, dims = "A20:D23", colNames = FALSE)
+  got <- wb_data(wb, sheet = 6, dims = "A20:D23", col_names = FALSE)
   expect_equal(unlist(t(dat)), unlist(got), ignore_attr = TRUE)
 
 })
@@ -170,7 +170,7 @@ test_that("wb_set_header_footer() works", {
 test_that("cloning from workbooks works", {
 
   ## FIXME these tests should be improved, right now they only check the
-  ## existance of a worksheet
+  ## existence of a worksheet
 
   # create a second workbook
   wb <- wb_workbook()$
@@ -184,9 +184,8 @@ test_that("cloning from workbooks works", {
   wb_in <- wb_load(fl)
 
   wb$clone_worksheet(old = "SUM", new = "SUM", from = wb_in)
-  exp <- c("NOT_SUM", "SUM")
   got <- wb$get_sheet_names() %>% unname()
-  expect_equal(exp, got)
+  expect_equal(got, c("NOT_SUM", "SUM"))
 
   wb$clone_worksheet(old = "SUM", new = "SUM_clone")
   exp <- c("NOT_SUM", "SUM", "SUM_clone")
@@ -284,7 +283,8 @@ test_that("cloning column and row styles works", {
   wb$worksheets[[1]]$sheet_data$row_attr[2, "customFormat"] <- "1"
   wb$worksheets[[1]]$sheet_data$row_attr[2, "s"] <- wb$styles_mgr$get_xf_id("new_styles")
 
-  cols <- openxlsx2:::wb_create_columns(wb, sheet = 1, cols = seq_along(mtcars))
+  # wb_create_columns is an internal function in openxlsx2.
+  cols <- wb_create_columns(wb, sheet = 1, cols = seq_along(mtcars))
   cols[cols$min == 11, "style"] <- "1"
   wb$worksheets[[1]]$fold_cols(cols)
 

tests/testthat/test-conditional_formatting.R

@@ -695,8 +695,8 @@ test_that("containsBlanks works", {
 
   wb <- wb_workbook()
   wb$add_worksheet()
-  wb$add_data(x = c(NA, 1, 2, ""), colNames = FALSE, na.strings = NULL)
-  wb$add_data(x = c(NA, 1, 2, ""), colNames = FALSE, na.strings = NULL, startCol = 2)
+  wb$add_data(x = c(NA, 1, 2, ""), col_names = FALSE, na.strings = NULL)
+  wb$add_data(x = c(NA, 1, 2, ""), col_names = FALSE, na.strings = NULL, start_col = 2)
   wb$add_conditional_formatting(cols = 1, rows = 1:4, type = "containsBlanks")
   wb$add_conditional_formatting(cols = 2, rows = 1:4, type = "notContainsBlanks")
 
@@ -715,7 +715,7 @@ test_that("warning on cols > 2 and dims", {
     wb$add_conditional_formatting(
       rows = seq_len(nrow(mtcars)),
       cols = c(2, 4, 6),
-      type = 'between',
+      type = "between",
       rule = c(2, 4)
     ),
     "cols > 2, will create range from min to max."
@@ -724,7 +724,7 @@ test_that("warning on cols > 2 and dims", {
   wb <- wb_workbook()$add_worksheet()$add_data(x = mtcars)
   wb$add_conditional_formatting(
     dims = "B2:F5",
-    type = 'between',
+    type = "between",
     rule = c(2, 4)
   )
 

tests/testthat/test-converters.R

@@ -1,34 +1,28 @@
 test_that("int2col", {
 
-  exp <- LETTERS[1:10]
-  got <- int2col(1:10)
-  expect_equal(exp, got)
-
-  expect_error(int2col("a"),
-               "x must be numeric.")
-
+  expect_equal(int2col(1:10), LETTERS[1:10])
+  expect_error(int2col("a"), "x must be numeric.")
 })
 
 test_that("col2int", {
   expect_null(col2int(NULL))
 
-  expect_equal(1, col2int("a"))
-  expect_equal(1, col2int(1))
+  expect_equal(col2int("a"), 1)
+  expect_equal(col2int(1), 1)
   expect_error(col2int(list()), "x must be character")
 
-  expect_equal(1, col2int("A"))
-  expect_equal(c(1, 3, 4), col2int(c("A", "C:D")))
-  expect_equal(c(1, 3, 4, 11), col2int(c("A", "C:D", "K")))
-  expect_equal(c(1, 3, 4, 11, 27, 28, 29, 30), col2int(c("A", "C:D", "K", "AA:AD")))
+  expect_equal(col2int("A"), 1)
+  expect_equal(col2int(c("A", "C:D")), c(1, 3, 4))
+  expect_equal(col2int(c("A", "C:D", "K")), c(1, 3, 4, 11))
+  expect_equal(col2int(c("A", "C:D", "K", "AA:AD")), c(1, 3, 4, 11, 27, 28, 29, 30))
   expect_error(col2int(c("a", NA_character_, "c")), "x contains NA")
 
 })
 
 test_that("get_cell_refs", {
 
-  exp <- c("B1", "C2", "D3")
   got <- get_cell_refs(data.frame(1:3, 2:4))
-  expect_equal(exp, got)
+  expect_equal(got, c("B1", "C2", "D3"))
 
   expect_error(get_cell_refs(data.frame("a", "a")),
                "cellCoords must only contain integers")
@@ -57,12 +51,10 @@ test_that("", {
   )
   on.exit(options(op), add = TRUE)
 
-  exp <- 10
   got <- calc_col_width(wb_workbook()$get_base_font(), 11)
-  expect_equal(exp, got)
+  expect_equal(got, 10)
 
-  exp <- 8
   got <- calc_col_width(wb_workbook()$get_base_font(), 7)
-  expect_equal(exp, got)
+  expect_equal(got, 8)
 
 })