refactor: replace all abort() with cli::cli_abort() (#2404)

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
Co-authored-by: Copilot <Copilot@users.noreply.github.com>

Author: 3 people

R/build_copy_queries.R

@@ -36,7 +36,7 @@ build_copy_queries <- function(
             .x,
             "no_action" = "",
             "cascade" = " ON DELETE CASCADE",
-            abort(glue('`on_delete = "{.x}"` not supported'))
+            cli::cli_abort('{.code on_delete = {.val {.x}}} is not supported.')
           )
         }
       )

R/check-cardinalities.R

@@ -257,6 +257,7 @@ check_card_api <- function(
   call = caller_env(),
   target = list
 ) {
+  dm_local_error_call(call)
   if (dots_n(...) >= 2) {
     name <- as.character(frame_call(call)[[1]] %||% "check_card_api")
     deprecate_warn(
@@ -315,8 +316,9 @@ check_card_api_impl <- function(
   if (!isTRUE(by_position)) {
     y_idx <- match(colnames(parent_table), colnames(child_table))
     if (anyNA(y_idx)) {
-      abort(
-        "`by_position = FALSE` or `by_position = NULL` require column names in `x` to match those in `y`."
+      cli::cli_abort(
+        "{.code by_position = FALSE} or {.code by_position = NULL} require column names in {.arg x} to match those in {.arg y}.",
+        call = dm_error_call()
       )
     }
 

R/db-interface.R

@@ -299,7 +299,7 @@ db_append_table <- function(
           # Can't use dbAppendTable(): https://github.com/r-dbi/odbc/issues/480
           sql <- DBI::sqlAppendTable(con, remote_table_id, values, row.names = FALSE)
           if (length(autoinc) > 1L) {
-            abort("more than one autoincrement key in one table")
+            cli::cli_abort("More than one autoincrement key in one table.")
           }
           if (!is_empty(autoinc) && autoinc) {
             sql <- DBI::SQL(paste0(

R/deconstruct.R

@@ -309,17 +309,23 @@ keyed_by <- function(x, y) {
   fks_df <- fks_df_from_keys_info(list(x = x, y = y))
 
   if (nrow(fks_df) == 0) {
-    abort("Can't infer `by`: foreign key information lost?")
+    cli::cli_abort("Can't infer {.arg by}: foreign key information lost?", call = dm_error_call())
   }
 
   stopifnot(map_int(fks_df$fks, NROW) > 0)
 
   if (nrow(fks_df) > 1) {
-    abort("Can't infer `by`: foreign key available in both directions")
+    cli::cli_abort(
+      "Can't infer {.arg by}: foreign key available in both directions.",
+      call = dm_error_call()
+    )
   }
 
   if (nrow(fks_df$fks[[1]]) > 1) {
-    abort("Can't infer `by`: multiple foreign keys available")
+    cli::cli_abort(
+      "Can't infer {.arg by}: multiple foreign keys available.",
+      call = dm_error_call()
+    )
   }
 
   fk <- fks_df$fks[[1]][1, ]

R/dm.R

@@ -63,15 +63,15 @@ dm <- function(
 
   for (i in which(is_dm)) {
     if (names[[i]] != "") {
-      abort(c(
-        "All dm objects passed to `dm()` must be unnamed.",
+      cli::cli_abort(c(
+        "All dm objects passed to {.fun dm} must be unnamed.",
         i = paste0("Argument ", i, " has name ", tick(names[[i]]), ".")
       ))
     }
 
     if (is_zoomed(dots[[i]])) {
-      abort(c(
-        "All dm objects passed to `dm()` must be unzoomed.",
+      cli::cli_abort(c(
+        "All dm objects passed to {.fun dm} must be unzoomed.",
         i = paste0("Argument ", i, " is a zoomed dm.")
       ))
     }
@@ -313,7 +313,7 @@ as_dm.default <- function(x, ...) {
   check_dots_empty()
 
   if (!is.list(x) || is.object(x)) {
-    abort(paste0("Can't coerce <", class(x)[[1]], "> to <dm>."))
+    cli::cli_abort("Can't coerce {.cls {class(x)[[1]]}} to {.cls dm}.")
   }
 
   # Automatic name repair
@@ -699,7 +699,7 @@ src_tbls_impl <- function(dm, quiet = FALSE) {
 #'   class()
 compute.dm <- function(x, ..., temporary = TRUE) {
   if (!isTRUE(temporary)) {
-    abort("`compute.dm()` does not support `temporary = FALSE`.")
+    cli::cli_abort("{.fun compute.dm} does not support {.code temporary = FALSE}.")
   }
 
   # for both dm and dm_zoomed
@@ -842,12 +842,16 @@ pull_tbl.dm <- function(dm, table, ..., keyed = FALSE) {
 
 #' @export
 pull_tbl.dm_zoomed <- function(dm, table, ..., keyed = FALSE) {
+  dm_local_error_call()
+
   if (isTRUE(keyed)) {
-    abort("`keyed = TRUE` not supported for zoomed dm objects.")
+    cli::cli_abort(
+      "{.code keyed = TRUE} not supported for zoomed dm objects.",
+      call = dm_error_call()
+    )
   }
 
   check_dots_empty()
-  dm_local_error_call()
 
   table_name <- as_string(enexpr(table))
   zoomed <- dm_get_zoom(dm)

R/dm_nest_tbl.R

@@ -25,6 +25,8 @@
 #'
 #' nested_dm$airlines
 dm_nest_tbl <- function(dm, child_table, into = NULL) {
+  dm_local_error_call()
+
   # process args
   into <- enquo(into)
   # FIXME: Rename table_name to child_tables_name
@@ -52,27 +54,24 @@ dm_nest_tbl <- function(dm, child_table, into = NULL) {
 
   # make sure we have a terminal child
   if (length(children) || !length(parent_name) || length(parent_name) > 1) {
-    if (length(parent_name)) {
-      parent_msg <- paste0("\nparents: ", toString(paste0("`", parent_name, "`")))
-    } else {
-      parent_msg <- ""
-    }
-    if (length(children)) {
-      children_msg <- paste0("\nchildren: ", toString(paste0("`", children, "`")))
-    } else {
-      children_msg <- ""
-    }
-    abort(glue(
-      "`{table_name}` can't be nested because it is not a terminal child table.",
-      "{parent_msg}{children_msg}"
-    ))
+    cli::cli_abort(
+      c(
+        "{.val {table_name}} can't be nested because it is not a terminal child table.",
+        if (length(parent_name)) "parents: {.val {parent_name}}",
+        if (length(children)) "children: {.val {children}}"
+      ),
+      call = dm_error_call()
+    )
   }
 
   # check consistency of `into` if relevant
   if (!quo_is_null(into)) {
     into <- dm_tbl_name(dm, !!into)
     if (into != parent_name) {
-      abort(glue("`{table_name}` can only be packed into `{child_name}`"))
+      cli::cli_abort(
+        "{.val {table_name}} can only be packed into {.val {child_name}}.",
+        call = dm_error_call()
+      )
     }
   }
 
@@ -126,6 +125,8 @@ dm_nest_tbl <- function(dm, child_table, into = NULL) {
 #'
 #' dm_packed$flights$planes
 dm_pack_tbl <- function(dm, parent_table, into = NULL) {
+  dm_local_error_call()
+
   # process args
   into <- enquo(into)
   table_name <- dm_tbl_name(dm, {{ parent_table }})
@@ -147,7 +148,10 @@ dm_pack_tbl <- function(dm, parent_table, into = NULL) {
   if (!quo_is_null(into)) {
     into <- dm_tbl_name(dm, !!into)
     if (into != child_name) {
-      abort(glue("`{table_name}` can only be packed into `{child_name}`"))
+      cli::cli_abort(
+        "{.val {table_name}} can only be packed into {.val {child_name}}.",
+        call = dm_error_call()
+      )
     }
   }
 
@@ -181,21 +185,14 @@ check_table_can_be_packed <- function(table_name, children_names, fks) {
   table_has_one_child <- length(children_names) == 1
   table_is_terminal_parent <- table_has_one_child && !table_has_parents
   if (!table_is_terminal_parent) {
-    if (table_has_parents) {
-      parent_msg <- paste0("\nparents : ", toString(paste0("`", parents, "`")))
-    } else {
-      parent_msg <- ""
-    }
-    table_has_children <- length(children_names) > 0
-    if (table_has_children) {
-      children_msg <- paste0("\nchildren: ", toString(paste0("`", children_names, "`")))
-    } else {
-      children_msg <- ""
-    }
-    abort(glue(
-      "`{table_name}` can't be packed because it is not a terminal parent table.",
-      "{parent_msg}{children_msg}"
-    ))
+    cli::cli_abort(
+      c(
+        "{.val {table_name}} can't be packed because it is not a terminal parent table.",
+        if (length(parents) > 0) "parents : {.val {parents}}",
+        if (length(children_names) > 0) "children: {.val {children_names}}"
+      ),
+      call = dm_error_call()
+    )
   }
   invisible(NULL)
 }

R/dm_sql.R

@@ -487,7 +487,7 @@ ddl_get_fk_defs <- function(fks, con, table_names) {
             .x,
             "no_action" = "",
             "cascade" = " ON DELETE CASCADE",
-            abort(glue('`on_delete = "{.x}"` not supported'))
+            cli::cli_abort('{.code on_delete = {.val {.x}}} is not supported.')
           )
         }
       )

R/dm_wrap.R

@@ -40,6 +40,8 @@
 #' dm_nycflights13() %>%
 #'   dm_wrap_tbl(root = airlines)
 dm_wrap_tbl <- function(dm, root, strict = TRUE, progress = NA) {
+  dm_local_error_call()
+
   wrap_plan <- dm_wrap_tbl_plan(dm, {{ root }})
 
   ticker <- new_ticker(
@@ -60,7 +62,10 @@ dm_wrap_tbl <- function(dm, root, strict = TRUE, progress = NA) {
   if (length(wrapped_dm) > 1) {
     if (strict) {
       # FIXME: Detect earlier
-      abort("The `dm` is not cycle free and can't be wrapped into a single tibble.")
+      cli::cli_abort(
+        "The {.cls dm} is not cycle free and can't be wrapped into a single tibble.",
+        call = dm_error_call()
+      )
     }
   }
 

R/financial-db-con.R

@@ -5,13 +5,13 @@
 #' @noRd
 financial_db_con <- function() {
   if (Sys.getenv("DM_OFFLINE") != "") {
-    abort("Offline")
+    cli::cli_abort("Offline")
   }
 
   err_relational <- tryCatch(return(relational_con()), error = identity)
   err_dbedu <- tryCatch(return(dbedu_con()), error = identity)
 
-  abort(paste0(
+  cli::cli_abort(paste0(
     "Can't connect to relational.fel.cvut.cz or databases.pacha.dev:\n",
     conditionMessage(err_relational),
     "\n",

R/gui-check.R

@@ -19,6 +19,6 @@ check_tbl_in_dm <- function(
 
 check_at_least_one_col <- function(cols, call = current_call()) {
   if (length(cols) < 1) {
-    abort("Hey, you should select at least one column!", call = call)
+    cli::cli_abort("At least one column must be selected.", call = call)
   }
 }