Use cli error chaining + add clickable link to bad config

R/cache.R

@@ -53,10 +53,10 @@ load_cache <- function(file, path = NULL) {
         invokeRestart("muffleWarning")
       },
       error = function(e) {
-        cli_warn(c(
-          x = "Could not load cache file {.file {file}}:",
-          i = conditionMessage(e)
-        ))
+        cli_warn(
+          "Could not load cache file {.file {file}}:",
+          parent = e
+        )
       }
     )
   } # else nothing to do for source file that has no cache

R/lint.R

@@ -88,10 +88,10 @@ lint <- function(filename, linters = NULL, ..., cache = FALSE, parse_settings =
         lints[[length(lints) + 1L]] <- withCallingHandlers(
           get_lints(expr, linter, linters[[linter]], lint_cache, source_expressions$lines),
           error = function(cond) {
-            cli_abort(c(
+            cli_abort(
               "Linter {.fn linter} failed in {.file {filename}}:",
-              conditionMessage(cond)
-            ))
+              parent = cond
+            )
           }
         )
       }
@@ -410,16 +410,13 @@ Lint <- function(filename, line_number = 1L, column_number = 1L, # nolint: objec
   }
   max_col <- max(nchar(line) + 1L, 1L, na.rm = TRUE)
   if (!is_number(column_number) || column_number < 0L || column_number > max_col) {
-    cli_abort(c(
-      i = "{.arg column_number} must be an integer between {.val {0}} and {.code nchar(line) + 1} ({.val {max_col}})",
-      x = "Instead, it was {.val {column_number}}."
-    ))
+    cli_abort("
+      {.arg column_number} must be an integer between {.val {0}} and {.val {max_col}} ({.code nchar(line) + 1}),
+      not {.obj_type_friendly {column_number}}.
+    ")
   }
   if (!is_number(line_number) || line_number < 1L) {
-    cli_abort(c(
-      i = "{.arg line_number} must be a positive integer.",
-      x = "Instead, it was {.val {line_number}}."
-    ))
+    cli_abort("{.arg line_number} must be a positive integer, not {.obj_type_friendly {line_number}}.")
   }
   check_ranges(ranges, max_col)
 
@@ -449,25 +446,28 @@ is_valid_range <- function(range, max_col) {
     range[[2L]] <= max_col
 }
 
-check_ranges <- function(ranges, max_col) {
+check_ranges <- function(ranges, max_col, call = parent.frame()) {
   if (is.null(ranges)) {
     return()
   }
   if (!is.list(ranges)) {
-    cli_abort(c(
-      i = "{.arg ranges} must be {.code NULL} or a {.cls list}.",
-      x = "Instead, it was {.cls {class(ranges)}}."
-    ))
+    cli_abort(
+      "{.arg ranges} must be {.code NULL} or a list, not {.obj_type_friendly {ranges}}.",
+      call = call
+    )
   }
 
   for (range in ranges) {
     if (!is_number(range, 2L)) {
-      cli_abort("{.arg ranges} must only contain {.cls integer} vectors of length 2 without {.code NA}s.")
+      cli_abort(
+        "{.arg ranges} must only contain integer vectors of length 2 without {.code NA}s.",
+        call = call
+      )
     } else if (!is_valid_range(range, max_col)) {
-      cli_abort(c(
-        x = "Invalid range specified.",
-        i = "Argument {.arg ranges} must satisfy 0 <= range[1L] <= range[2L] <= nchar(line) + 1 ({.val {max_col}})."
-      ))
+      cli_abort(
+        "{.arg ranges} must satisfy {.val {0}} <= range[1L] <= range[2L] <= {.val {max_col}} (nchar(line) + 1).",
+        call = call
+      )
     }
   }
 }

R/settings.R

@@ -61,7 +61,8 @@
 #'  ```
 #'
 #' @param filename Source file to be linted.
-read_settings <- function(filename) {
+#' @param call Passed to malformed to ensure linear trace.
+read_settings <- function(filename, call = parent.frame()) {
   reset_settings()
 
   config_file <- find_config(filename)
@@ -71,7 +72,7 @@ read_settings <- function(filename) {
     default_settings[["encoding"]] <- default_encoding
   }
 
-  config <- read_config_file(config_file)
+  config <- read_config_file(config_file, call = call)
   validate_config_file(config, config_file, default_settings)
 
   for (setting in names(default_settings)) {
@@ -89,59 +90,68 @@ read_settings <- function(filename) {
   }
 }
 
-read_config_file <- function(config_file) {
+#' @param call Passed to malformed to ensure linear trace.
+#' @noRd
+read_config_file <- function(config_file, call = parent.frame()) {
   if (is.null(config_file)) {
     return(NULL)
   }
 
+  # clickable link for eventual error messages.
+  malformed_file <- cli::style_hyperlink( # nolint: object_usage_linter. TODO(#2252).
+    cli::col_blue(basename(config_file)),
+    paste0("file://", config_file)
+  )
   config <- new.env()
   if (endsWith(config_file, ".R")) {
     load_config <- function(file) sys.source(file, config, keep.source = FALSE, keep.parse.data = FALSE)
     malformed <- function(e) {
-      cli_abort(c(
-        "Malformed config file, ensure it is valid R syntax.",
-        conditionMessage(e)
-      ))
+      cli_abort(
+        "Malformed config file ({malformed_file}), ensure it is valid R syntax.",
+        parent = e,
+        call = call
+      )
     }
   } else {
     load_config <- function(file) {
       dcf_values <- read.dcf(file, all = TRUE)
       for (setting in names(dcf_values)) {
-        parsed_setting <- tryCatch(
+        parsed_setting <- withCallingHandlers(
           str2lang(dcf_values[[setting]]),
           error = function(e) {
-            cli_abort(c(
+            cli_abort(
               "Malformed config setting {.field {setting}}:",
-              conditionMessage(e)
-            ))
+              parent = e
+            )
           }
         )
         setting_value <- withCallingHandlers(
           tryCatch(
             eval(parsed_setting),
             error = function(e) {
-              cli_abort(c(
-                "Error from config setting {.code {setting}} in {.code {format(conditionCall(e))}}:",
-                conditionMessage(e)
-              ))
+              cli_abort(
+                "Error from config setting {.code {setting}}.",
+                parent = e
+              )
             }
           ),
           warning = function(w) {
-            cli_warn(c(
-              "Warning from config setting {.code {setting}} in {.code {format(conditionCall(w))}}:",
-              conditionMessage(w)
-            ))
+            cli_warn(
+              "Warning from config setting {.code {setting}}.",
+              parent = w
+            )
             invokeRestart("muffleWarning")
           }
         )
         assign(setting, setting_value, envir = config)
       }
     }
     malformed <- function(e) {
-      cli_abort(c(
-        x = "Malformed config file:",
-        i = conditionMessage(e)
-      ))
+      cli_abort(
+        "Malformed config file ({malformed_file}):",
+        parent = e,
+        call = call
+      )
     }
   }
   withCallingHandlers(
@@ -150,10 +160,10 @@ read_config_file <- function(config_file) {
       error = malformed
     ),
     warning = function(w) {
-      cli::cli_warn(c(
-        x = "Warning encountered while loading config:",
-        i = conditionMessage(w)
-      ))
+      cli::cli_warn(
+        "Warning encountered while loading config:",
+        parent = w
+      )
       invokeRestart("muffleWarning")
     }
   )
@@ -164,7 +174,11 @@ validate_config_file <- function(config, config_file, defaults) {
   matched <- names(config) %in% names(defaults)
   if (!all(matched)) {
     unused_settings <- names(config)[!matched] # nolint: object_usage_linter. TODO(#2252).
-    cli_warn("Found unused settings in config {.str config_file}: {.field unused_settings}")
+    config_link <- cli::style_hyperlink( # nolint: object_usage_linter. TODO(#2252).
+      cli::col_blue(basename(config_file)),
+      url = paste0("file://", config_file)
+    )
+    cli_warn("Found unused settings in config file ({config_link}): {.field unused_settings}")
   }
 
   validate_regex(config,

R/with.R

@@ -31,8 +31,11 @@
 #' names(my_undesirable_functions)
 #' @export
 modify_defaults <- function(defaults, ...) {
-  if (missing(defaults) || !is.list(defaults) || !all(nzchar(names2(defaults)))) {
-    cli_abort("{.arg defaults} must be a named list.")
+  if (missing(defaults)) {
+    cli_abort("{.arg defaults} is a required argument, but is missing.")
+  }
+  if (!is.list(defaults) || !all(nzchar(names2(defaults)))) {
+    cli_abort("{.arg defaults} must be a named list, not {.obj_type_friendly {defaults}}.")
   }
   vals <- list(...)
   nms <- names2(vals)
@@ -90,7 +93,7 @@ modify_defaults <- function(defaults, ...) {
 #' @export
 linters_with_tags <- function(tags, ..., packages = "lintr", exclude_tags = "deprecated") {
   if (!is.character(tags) && !is.null(tags)) {
-    cli_abort("{.arg tags} must be a {.cls character} vector, or {.code NULL}.")
+    cli_abort("{.arg tags} must be a character vector, or {.code NULL}, not {.obj_type_friendly {tags}}.")
   }
   tagged_linters <- list()
 
@@ -102,8 +105,8 @@ linters_with_tags <- function(tags, ..., packages = "lintr", exclude_tags = "dep
       if (!all(available$linter %in% ns_exports)) {
         missing_linters <- setdiff(available$linter, ns_exports) # nolint: object_usage_linter. TODO(#2252).
         cli_abort(c(
-          i = "Linters {.fn {missing_linters}} are advertised by {.fn available_linters}.",
-          x = "But these linters are not exported by package {.pkg {package}}."
+          x = "Can't find linters {.fn {missing_linters}}.",
+          i = "These are advertised by {.fn available_linters}, but are not exported by package {.pkg {package}}."
         ))
       }
       linter_factories <- mget(available$linter, envir = pkg_ns)
@@ -178,9 +181,12 @@ linters_with_defaults <- function(..., defaults = default_linters) {
   dots <- list(...)
   if (missing(defaults) && "default" %in% names(dots)) {
     cli_warn(c(
-      "Did you mean {.arg defaults}?",
-      x = "{.arg default} is not an argument to {.fun linters_with_defaults}.",
-      i = "This warning will be removed when {.fun with_defaults} is fully deprecated."
+      x = "
+        {.arg default} is not an argument to {.help [{.fn linters_with_defaults}](lintr::linters_with_defaults)}.
+      ",
+      i = "Did you mean {.arg defaults}?",
+      # make message more subtle
+      cli::col_silver("This warning will be removed when {.fun with_defaults} is fully deprecated.")
     ))
     defaults <- dots$default
     nms <- names2(dots)
@@ -207,10 +213,10 @@ call_linter_factory <- function(linter_factory, linter_name, package) {
   linter <- tryCatch(
     linter_factory(),
     error = function(e) {
-      cli_abort(c(
+      cli_abort(
         "Could not create linter with {.fun {package}::{linter_name}}.",
-        conditionMessage(e)
-      ))
+        parent = e
+      )
     }
   )
   # Otherwise, all linters would be called "linter_factory".

R/xml_nodes_to_lints.R

@@ -55,7 +55,7 @@ xml_nodes_to_lints <- function(xml, source_expression, lint_message,
   } else if (!is_node(xml)) {
     cli_abort(c(
       x = "Expected an {.cls xml_nodeset}, a {.cls list} of xml_nodes, or an {.cls xml_node}.",
-      i = "Instead got an object of class(es): {.cls {class(xml)}}"
+      i = "Instead got {.obj_type_friendly {xml}}."
     ))
   }
   type <- match.arg(type, c("style", "warning", "error"))

tests/testthat/test-Lint-builder.R

@@ -2,7 +2,7 @@ test_that("Lint() errors on invalid input", {
   dummy_line <- "abc"
   expect_error(
     Lint("dummy.R", line = dummy_line, column_number = NA_integer_),
-    "`column_number` must be an integer between 0 and `nchar(line) + 1` (4)",
+    "`column_number` must be an integer between 0 and 4 (`nchar(line) + 1`)",
     fixed = TRUE
   )
   expect_error(
@@ -12,22 +12,22 @@ test_that("Lint() errors on invalid input", {
   )
   expect_error(
     Lint("dummy.R", ranges = c(1L, 3L)),
-    "`ranges` must be `NULL` or a <list>",
+    "`ranges` must be `NULL` or a list",
     fixed = TRUE
   )
   expect_error(
     Lint("dummy.R", ranges = list(1L)),
-    "`ranges` must only contain <integer> vectors of length 2 without `NA`s.",
+    "`ranges` must only contain integer vectors of length 2 without `NA`s.",
     fixed = TRUE
   )
   expect_error(
     Lint("dummy.R", ranges = list(c(1L, NA_integer_))),
-    "`ranges` must only contain <integer> vectors of length 2 without `NA`s.",
+    "`ranges` must only contain integer vectors of length 2 without `NA`s.",
     fixed = TRUE
   )
   expect_error(
     Lint("dummy.R", line = dummy_line, ranges = list(c(1L, 2L), c(1L, 5L))),
-    "Argument `ranges` must satisfy 0 <= range[1L] <= range[2L] <= nchar(line) + 1 (4).",
+    "`ranges` must satisfy 0 <= range[1L] <= range[2L] <= 4 (nchar(line) + 1).",
     fixed = TRUE
   )
 })

tests/testthat/test-lint_package.R

@@ -211,15 +211,15 @@ test_that("package using .lintr.R config lints correctly", {
   # config has bad R syntax
   expect_error(
     lint_package(test_path("dummy_packages", "RConfigInvalid")),
-    "Malformed config file, ensure it is valid R syntax",
+    "Malformed config file (lintr_test_config.R), ensure it is valid R syntax",
     fixed = TRUE
   )
 
   # config produces unused variables
   withr::local_options(lintr.linter_file = "lintr_test_config_extraneous")
   expect_warning(
     expect_length(lint_package(r_config_pkg), 2L),
-    "Found unused settings in config",
+    "Found unused settings in config file",
     fixed = TRUE
   )
 

tests/testthat/test-with.R

@@ -1,11 +1,18 @@
 test_that("modify_defaults produces error with missing or incorrect defaults", {
-  lint_msg <- "`defaults` must be a named list."
-  expect_error(modify_defaults(), lint_msg, fixed = TRUE)
-  expect_error(modify_defaults("assignment_linter"), lint_msg, fixed = TRUE)
+  expect_error(
+    modify_defaults(),
+    "`defaults` is a required argument, but is missing",
+    fixed = TRUE
+  )
+  expect_error(
+    modify_defaults("assignment_linter"),
+    "`defaults` must be a named list",
+    fixed = TRUE
+  )
 })
 
 test_that("linters_with_tags produces error with incorrect tags", {
-  expect_error(linters_with_tags(1L:4L), "`tags` must be a <character> vector, or `NULL`.", fixed = TRUE)
+  expect_error(linters_with_tags(1L:4L), "`tags` must be a character vector, or `NULL`", fixed = TRUE)
 })
 
 test_that("linters_with_defaults works as expected with unnamed args", {
@@ -34,7 +41,7 @@ test_that("linters_with_tags() verifies the output of available_linters()", {
   )
   expect_error(
     linters_with_tags(NULL),
-    "Linters `fake_linter()` and `very_fake_linter()` are advertised by `available_linters()`",
+    "Can't find linters `fake_linter()` and `very_fake_linter()`",
     fixed = TRUE
   )
 })