Use CLI and error classes for check_package()

R/check_package.R

@@ -1,37 +1,35 @@
 #' Check Data Package object
 #'
 #' Check if an object is a list describing a Data Package, i.e. it has the
-#' required properties `resources`, `resource_names` and `directory`.
+#' required properties `resources` and `directory`.
 #'
 #' @param package List describing a Data Package.
 #' @return `TRUE` or error.
 #' @family check functions
 #' @noRd
 check_package <- function(package) {
-  msg_invalid <- glue::glue(
-    "`package` must be a list describing a Data Package,",
-    "created with `read_package()` or `create_package()`.",
-    .sep = " "
-  )
-  # Check package is list with required properties
-  assertthat::assert_that(
-    is.list(package) &
-      all(c("resources", "directory") %in% names(package)),
-    msg = msg_invalid
-  )
-
-  # Check package properties have correct class
-  assertthat::assert_that(
-    is.list(package$resources) &
-      is.character(package$directory),
-    msg = msg_invalid
-  )
+  # Check package is a list with resources (list) and directory (character)
+  if (
+    !is.list(package) ||
+    !all(c("resources", "directory") %in% names(package)) ||
+    !is.list(package$resources) ||
+    !is.character(package$directory)
+  ) {
+    cli::cli_abort(
+      "{.arg package} must be a list describing a Data Package created with
+       {.fun read_package} or {.fun create_package}.",
+      class = "frictionless_error_package_incorrect"
+    )
+  }
 
   # Check all resources (if any) have a name
-  assertthat::assert_that(
-    purrr::every(package$resources, ~ !is.null(.x$name)),
-    msg = glue::glue(
-      "All resources in `package` must have property `name`."
+  if (purrr::some(package$resources, ~ is.null(.x$name))) {
+    cli::cli_abort(
+      "All resources in {.arg package} must have a {.field name} property.",
+      class = "frictionless_error_resources_without_name"
     )
-  )
+  }
+
+  # Return TRUE
+  TRUE
 }

tests/testthat/test-add_resource.R

@@ -15,11 +15,7 @@ test_that("add_resource() returns error on incorrect Data Package", {
   df <- data.frame("col_1" = c(1, 2), "col_2" = c("a", "b"))
   expect_error(
     add_resource(list(), "new", df),
-    paste(
-      "`package` must be a list describing a Data Package,",
-      "created with `read_package()` or `create_package()`."
-    ),
-    fixed = TRUE
+    class = "frictionless_error_package_incorrect"
   )
 })
 

tests/testthat/test-check_package.R

@@ -10,34 +10,56 @@ test_that("check_package() returns error on incorrect Data Package", {
   )
   expect_true(check_package(p))
 
-  error_message <- paste(
-    "`package` must be a list describing a Data Package,",
-    "created with `read_package()` or `create_package()`."
+  # Check error message
+  expect_error(
+    check_package("not_a_list"),
+    regexp = paste("`package` must be a list describing a Data Package",
+                   "created with `read_package()` or `create_package()`."),
+    fixed = TRUE
   )
-  # Must be a list
-  expect_error(check_package("not_a_list"), error_message, fixed = TRUE)
 
+  # Must be a list
+  expect_error(
+    check_package("not_a_list"),
+    class = "frictionless_error_package_incorrect"
+  )
   # Must have resources as list
   p_invalid <- p
   p_invalid$resources <- NULL
-  expect_error(check_package(p_invalid), error_message, fixed = TRUE)
+  expect_error(
+    check_package(p_invalid),
+    class = "frictionless_error_package_incorrect"
+  )
   p_invalid$resources <- vector(mode = "character")
-  expect_error(check_package(p_invalid), error_message, fixed = TRUE)
+  expect_error(
+    check_package(p_invalid),
+    class = "frictionless_error_package_incorrect"
+  )
 
   # Must have directory as character
   p_invalid <- p
   p_invalid$directory <- NULL
-  expect_error(check_package(p_invalid), error_message, fixed = TRUE)
+  expect_error(
+    check_package(p_invalid),
+    class = "frictionless_error_package_incorrect"
+  )
   p_invalid$directory <- logical()
-  expect_error(check_package(p_invalid), error_message, fixed = TRUE)
+  expect_error(
+    check_package(p_invalid),
+    class = "frictionless_error_package_incorrect"
+  )
 })
 
 test_that("check_package() returns error if resources have no name", {
   p <- example_package
   p$resources[[2]]$name <- NULL
   expect_error(
     check_package(p),
-    "All resources in `package` must have property `name`",
+    class = "frictionless_error_resources_without_name"
+  )
+  expect_error(
+    check_package(p),
+    regexp = "All resources in `package` must have a name property.",
     fixed = TRUE
   )
 })

tests/testthat/test-read_resource.R

@@ -98,11 +98,7 @@ test_that("read_resource() returns error on column selection not in schema", {
 test_that("read_resource() returns error on incorrect Data Package", {
   expect_error(
     read_resource(list(), "deployments"),
-    paste(
-      "`package` must be a list describing a Data Package,",
-      "created with `read_package()` or `create_package()`."
-    ),
-    fixed = TRUE
+    class = "frictionless_error_package_incorrect"
   )
 })
 

tests/testthat/test-remove_resource.R

@@ -7,11 +7,7 @@ test_that("remove_resource() returns a valid Data Package", {
 test_that("remove_resource() returns error on incorrect Data Package", {
   expect_error(
     remove_resource(list(), "deployments"),
-    paste(
-      "`package` must be a list describing a Data Package,",
-      "created with `read_package()` or `create_package()`."
-    ),
-    fixed = TRUE
+    class = "frictionless_error_package_incorrect"
   )
 })
 

tests/testthat/test-write_package.R

@@ -18,11 +18,7 @@ test_that("write_package() returns output Data Package (invisibly)", {
 test_that("write_package() returns error on incorrect Data Package", {
   expect_error(
     write_package(list()),
-    paste(
-      "`package` must be a list describing a Data Package,",
-      "created with `read_package()` or `create_package()`."
-    ),
-    fixed = TRUE
+    class = "frictionless_error_package_incorrect"
   )
 })