Review error messages (#5557)

* Add full stops to error messages.

* Remove self-ref

* Replace `expect_snapshot_warning` and `expect_snapshot_error()` with `expect_snapshot()`.

* Review and accept snapshots

* error msg

* style

* typo

* Revert "Replace `expect_snapshot_warning` and `expect_snapshot_error()` with `expect_snapshot()`."

This reverts commit 3b7dcc7.

* change

* Update snapshots

* Adjust failing tests

* Apply suggestions from code review

Co-authored-by: Teun van den Brand <[email protected]>

* Apply suggestions from teunbrand

---------

Co-authored-by: Teun van den Brand <[email protected]>

Author: olivroy

R/aes.R

@@ -425,7 +425,7 @@ alternative_aes_extract_usage <- function(x) {
   } else if (is_call(x, "$")) {
     as.character(x[[3]])
   } else {
-    cli::cli_abort("Don't know how to get alternative usage for {.var {x}}")
+    cli::cli_abort("Don't know how to get alternative usage for {.var {x}}.")
   }
 }
 

R/annotation-custom.R

@@ -71,7 +71,7 @@ GeomCustomAnn <- ggproto("GeomCustomAnn", Geom,
   draw_panel = function(data, panel_params, coord, grob, xmin, xmax,
                         ymin, ymax) {
     if (!inherits(coord, "CoordCartesian")) {
-      cli::cli_abort("{.fn annotation_custom} only works with {.fn coord_cartesian}")
+      cli::cli_abort("{.fn annotation_custom} only works with {.fn coord_cartesian}.")
     }
     corners <- data_frame0(
       x = c(xmin, xmax),

R/annotation-map.R

@@ -63,7 +63,7 @@ annotation_map <- function(map, ...) {
   if (!is.null(map$long)) map$x <- map$long
   if (!is.null(map$region)) map$id <- map$region
   if (!all(c("x", "y", "id") %in% names(map))) {
-    cli::cli_abort("{.arg map} must have the columns {.col x}, {.col y}, and {.col id}")
+    cli::cli_abort("{.arg map} must have the columns {.col x}, {.col y}, and {.col id}.")
   }
 
   layer(

R/annotation-raster.R

@@ -74,7 +74,7 @@ GeomRasterAnn <- ggproto("GeomRasterAnn", Geom,
   draw_panel = function(data, panel_params, coord, raster, xmin, xmax,
                         ymin, ymax, interpolate = FALSE) {
     if (!inherits(coord, "CoordCartesian")) {
-      cli::cli_abort("{.fn annotation_raster} only works with {.fn coord_cartesian}")
+      cli::cli_abort("{.fn annotation_raster} only works with {.fn coord_cartesian}.")
     }
     corners <- data_frame0(
       x = c(xmin, xmax),

R/annotation.R

@@ -13,7 +13,7 @@
 #'
 #' @section Unsupported geoms:
 #' Due to their special nature, reference line geoms [geom_abline()],
-#' [geom_hline()], and [geom_vline()] can't be used with [annotate()].
+#' [geom_hline()], and [geom_vline()] can't be used with `annotate()`.
 #' You can use these geoms directly for annotations.
 #' @param geom name of geom to use for annotation
 #' @param x,y,xmin,ymin,xmax,ymax,xend,yend positioning aesthetics -

R/autolayer.R

@@ -15,5 +15,5 @@ autolayer <- function(object, ...) {
 
 #' @export
 autolayer.default <- function(object, ...) {
-  cli::cli_abort("No autolayer method available for {.cls {class(object)[1]}} objects")
+  cli::cli_abort("No autolayer method available for {.cls {class(object)[1]}} objects.")
 }

R/autoplot.R

@@ -17,7 +17,7 @@ autoplot <- function(object, ...) {
 autoplot.default <- function(object, ...) {
   cli::cli_abort(c(
     "Objects of class {.cls {class(object)[[1]]}} are not supported by autoplot.",
-    "i" = "have you loaded the required package?"
+    "i" = "Have you loaded the required package?"
   ))
 }
 

R/axis-secondary.R

@@ -123,7 +123,7 @@ set_sec_axis <- function(sec.axis, scale) {
   if (!is.waive(sec.axis)) {
     if (is.formula(sec.axis)) sec.axis <- sec_axis(sec.axis)
     if (!is.sec_axis(sec.axis)) {
-      cli::cli_abort("Secondary axes must be specified using {.fn sec_axis}")
+      cli::cli_abort("Secondary axes must be specified using {.fn sec_axis}.")
     }
     scale$secondary.axis <- sec.axis
   }
@@ -165,7 +165,7 @@ AxisSecondary <- ggproto("AxisSecondary", NULL,
       return()
     }
     if (!is.function(self$trans)) {
-      cli::cli_abort("Transformation for secondary axes must be a function")
+      cli::cli_abort("Transformation for secondary axes must be a function.")
     }
     if (is.derived(self$name) && !is.waive(scale$name)) self$name <- scale$name
     if (is.derived(self$breaks)) self$breaks <- scale$breaks
@@ -194,7 +194,7 @@ AxisSecondary <- ggproto("AxisSecondary", NULL,
 
     # Test for monotonicity
     if (!is_unique(sign(diff(full_range))))
-      cli::cli_abort("Transformation for secondary axes must be monotonic")
+      cli::cli_abort("Transformation for secondary axes must be monotonic.")
   },
 
   break_info = function(self, range, scale) {

R/bin.R

@@ -51,13 +51,11 @@ bin_breaks <- function(breaks, closed = c("right", "left")) {
 bin_breaks_width <- function(x_range, width = NULL, center = NULL,
                              boundary = NULL, closed = c("right", "left")) {
   if (length(x_range) != 2) {
-    cli::cli_abort("{.arg x_range} must have two elements")
+    cli::cli_abort("{.arg x_range} must have two elements.")
   }
 
-  check_number_decimal(width)
-  if (width <= 0) {
-    cli::cli_abort("{.arg binwidth} must be positive")
-  }
+  # binwidth seems to be the argument name supplied to width. (stat-bin and stat-bindot)
+  check_number_decimal(width, min = 0, allow_infinite = FALSE, arg = "binwidth")
 
   if (!is.null(boundary) && !is.null(center)) {
     cli::cli_abort("Only one of {.arg boundary} and {.arg center} may be specified.")
@@ -105,7 +103,7 @@ bin_breaks_width <- function(x_range, width = NULL, center = NULL,
 bin_breaks_bins <- function(x_range, bins = 30, center = NULL,
                             boundary = NULL, closed = c("right", "left")) {
   if (length(x_range) != 2) {
-    cli::cli_abort("{.arg x_range} must have two elements")
+    cli::cli_abort("{.arg x_range} must have two elements.")
   }
 
   check_number_whole(bins, min = 1)

R/compat-plyr.R

@@ -19,7 +19,7 @@ unrowname <- function(x) {
   } else if (is.matrix(x)) {
     dimnames(x)[1] <- list(NULL)
   } else {
-    cli::cli_abort("Can only remove rownames from {.cls data.frame} and {.cls matrix} objects")
+    cli::cli_abort("Can only remove rownames from {.cls data.frame} and {.cls matrix} objects.")
   }
   x
 }
@@ -239,7 +239,7 @@ as.quoted <- function(x, env = parent.frame()) {
   } else if (is.call(x)) {
     as.list(x)[-1]
   } else {
-    cli::cli_abort("Must be a character vector, call, or formula")
+    cli::cli_abort("Must be a character vector, call, or formula.")
   }
   attributes(x) <- list(env = env, class = 'quoted')
   x

R/coord-.R

@@ -66,27 +66,27 @@ Coord <- ggproto("Coord",
   render_fg = function(panel_params, theme) element_render(theme, "panel.border"),
 
   render_bg = function(self, panel_params, theme) {
-    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_bg} method")
+    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_bg} method.")
   },
 
   render_axis_h = function(self, panel_params, theme) {
-    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_axis_h} method")
+    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_axis_h} method.")
   },
 
   render_axis_v = function(self, panel_params, theme) {
-    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_axis_v} method")
+    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn render_axis_v} method.")
   },
 
   # transform range given in transformed coordinates
   # back into range in given in (possibly scale-transformed)
   # data coordinates
   backtransform_range = function(self, panel_params) {
-    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn backtransform_range} method")
+    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn backtransform_range} method.")
   },
 
   # return range stored in panel_params
   range = function(self, panel_params) {
-    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn range} method")
+    cli::cli_abort("{.fn {snake_class(self)}} has not implemented a {.fn range} method.")
   },
 
   setup_panel_params = function(scale_x, scale_y, params = list()) {

R/coord-sf.R

@@ -127,7 +127,7 @@ CoordSf <- ggproto("CoordSf", CoordCartesian,
     }
 
     if (length(x_labels) != length(x_breaks)) {
-      cli::cli_abort("Breaks and labels along x direction are different lengths")
+      cli::cli_abort("{.arg breaks} and {.arg labels} along {.code x} direction have different lengths.")
     }
     graticule$degree_label[graticule$type == "E"] <- x_labels
 
@@ -152,7 +152,7 @@ CoordSf <- ggproto("CoordSf", CoordCartesian,
     }
 
     if (length(y_labels) != length(y_breaks)) {
-      cli::cli_abort("Breaks and labels along y direction are different lengths")
+      cli::cli_abort("{.arg breaks} and {.arg labels} along {.code y} direction have different lengths.")
     }
     graticule$degree_label[graticule$type == "N"] <- y_labels
 
@@ -203,7 +203,7 @@ CoordSf <- ggproto("CoordSf", CoordCartesian,
       if (self$lims_method != "geometry_bbox") {
         cli::cli_warn(c(
                 "Projection of {.field x} or {.field y} limits failed in {.fn coord_sf}.",
-          "i" = "Consider setting {.code lims_method = \"geometry_bbox\"} or {.code default_crs = NULL}."
+          "i" = "Consider setting {.code lims_method = {.val geometry_bbox}} or {.code default_crs = NULL}."
         ))
       }
       coord_bbox <- self$params$bbox
@@ -409,7 +409,7 @@ sf_rescale01 <- function(x, x_range, y_range) {
 calc_limits_bbox <- function(method, xlim, ylim, crs, default_crs) {
   if (any(!is.finite(c(xlim, ylim))) && method != "geometry_bbox") {
     cli::cli_abort(c(
-            "Scale limits cannot be mapped onto spatial coordinates in {.fn coord_sf}",
+            "Scale limits cannot be mapped onto spatial coordinates in {.fn coord_sf}.",
       "i" = "Consider setting {.code lims_method = \"geometry_bbox\"} or {.code default_crs = NULL}."
     ))
   }
@@ -542,14 +542,12 @@ coord_sf <- function(xlim = NULL, ylim = NULL, expand = TRUE,
     label_axes <- parse_axes_labeling(label_axes)
   } else if (!is.list(label_axes)) {
     cli::cli_abort("Panel labeling format not recognized.")
-    label_axes <- list(left = "N", bottom = "E")
   }
 
   if (is.character(label_graticule)) {
     label_graticule <- unlist(strsplit(label_graticule, ""))
   } else {
     cli::cli_abort("Graticule labeling format not recognized.")
-    label_graticule <- ""
   }
 
   # switch limit method to "orthogonal" if not specified and default_crs indicates projected coords

R/facet-.R

@@ -83,10 +83,10 @@ Facet <- ggproto("Facet", NULL,
   params = list(),
 
   compute_layout = function(data, params) {
-    cli::cli_abort("Not implemented")
+    cli::cli_abort("Not implemented.")
   },
   map_data = function(data, layout, params) {
-    cli::cli_abort("Not implemented")
+    cli::cli_abort("Not implemented.")
   },
   init_scales = function(layout, x_scale = NULL, y_scale = NULL, params) {
     scales <- list()
@@ -132,7 +132,7 @@ Facet <- ggproto("Facet", NULL,
     rep(list(zeroGrob()), vec_unique_count(layout$PANEL))
   },
   draw_panels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
-    cli::cli_abort("Not implemented")
+    cli::cli_abort("Not implemented.")
   },
   draw_labels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, labels, params) {
     panel_dim <-  find_panel(panels)
@@ -323,13 +323,13 @@ as_facets_list <- function(x) {
 
 validate_facets <- function(x) {
   if (inherits(x, "uneval")) {
-    cli::cli_abort("Please use {.fn vars} to supply facet variables")
+    cli::cli_abort("Please use {.fn vars} to supply facet variables.")
   }
   # Native pipe have higher precedence than + so any type of gg object can be
   # expected here, not just ggplot
   if (inherits(x, "gg")) {
     cli::cli_abort(c(
-      "Please use {.fn vars} to supply facet variables",
+      "Please use {.fn vars} to supply facet variables.",
       "i" = "Did you use {.code %>%} or {.code |>} instead of {.code +}?"
     ))
   }
@@ -500,7 +500,7 @@ check_layout <- function(x) {
     return()
   }
 
-  cli::cli_abort("Facet layout has a bad format. It must contain columns {.col PANEL}, {.col SCALE_X}, and {.col SCALE_Y}")
+  cli::cli_abort("Facet layout has a bad format. It must contain columns {.col PANEL}, {.col SCALE_X}, and {.col SCALE_Y}.")
 }
 
 check_facet_vars <- function(..., name) {
@@ -509,8 +509,8 @@ check_facet_vars <- function(..., name) {
   problems <- intersect(vars_names, reserved_names)
   if (length(problems) != 0) {
     cli::cli_abort(c(
-      "{.val {problems}} {?is/are} not {?an/} allowed name{?/s} for faceting variables",
-      "i" = "Change the name of your data columns to not be {.or {.str {reserved_names}}}"
+      "{.val {problems}} {?is/are} not {?an/} allowed name{?/s} for faceting variables.",
+      "i" = "Change the name of your data columns to not be {.or {.str {reserved_names}}}."
     ), call = call2(name))
   }
 }
@@ -631,7 +631,7 @@ combine_vars <- function(data, env = emptyenv(), vars = NULL, drop = TRUE) {
   }
 
   if (empty(base)) {
-    cli::cli_abort("Faceting variables must have at least one value")
+    cli::cli_abort("Faceting variables must have at least one value.")
   }
 
   base

R/facet-grid-.R

@@ -137,8 +137,8 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
     y = any(space %in% c("free_y", "free"))
   )
 
-  if (!is.null(switch) && !switch %in% c("both", "x", "y")) {
-    cli::cli_abort("{.arg switch} must be either {.val both}, {.val x}, or {.val y}")
+  if (!is.null(switch)) {
+    arg_match0(switch, c("both", "x", "y"))
   }
 
   facets_list <- grid_as_facets_list(rows, cols)
@@ -159,7 +159,7 @@ grid_as_facets_list <- function(rows, cols) {
   is_rows_vars <- is.null(rows) || is_quosures(rows)
   if (!is_rows_vars) {
     if (!is.null(cols)) {
-      msg <- "{.arg rows} must be {.val NULL} or a {.fn vars} list if {.arg cols} is a {.fn vars} list"
+      msg <- "{.arg rows} must be {.code NULL} or a {.fn vars} list if {.arg cols} is a {.fn vars} list."
       # Native pipe have higher precedence than + so any type of gg object can be
       # expected here, not just ggplot
       if (inherits(rows, "gg")) {
@@ -173,7 +173,7 @@ grid_as_facets_list <- function(rows, cols) {
     # For backward-compatibility
     facets_list <- as_facets_list(rows)
     if (length(facets_list) > 2L) {
-      cli::cli_abort("A grid facet specification can't have more than two dimensions")
+      cli::cli_abort("A grid facet specification can't have more than two dimensions.")
     }
     # Fill with empty quosures
     facets <- list(rows = quos(), cols = quos())
@@ -206,7 +206,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     dups <- intersect(names(rows), names(cols))
     if (length(dups) > 0) {
       cli::cli_abort(c(
-              "Faceting variables can only appear in {.arg rows} or {.arg cols}, not both.\n",
+              "Faceting variables can only appear in {.arg rows} or {.arg cols}, not both.",
         "i" = "Duplicated variables: {.val {dups}}"
       ), call = call2(snake_class(self)))
     }
@@ -303,7 +303,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
   },
   draw_panels = function(panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales")
+      cli::cli_abort("{.fn {snake_class(coord)}} doesn't support free scales.")
     }
 
     cols <- which(layout$ROW == 1)
@@ -321,7 +321,7 @@ FacetGrid <- ggproto("FacetGrid", Facet,
 
     aspect_ratio <- theme$aspect.ratio
     if (!is.null(aspect_ratio) && (params$space_free$x || params$space_free$y)) {
-      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio")
+      cli::cli_abort("Free scales cannot be mixed with a fixed aspect ratio.")
     }
     if (is.null(aspect_ratio) && !params$free$x && !params$free$y) {
       aspect_ratio <- coord$aspect(ranges[[1]])

R/facet-wrap.R

@@ -217,7 +217,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
   },
   draw_panels = function(self, panels, layout, x_scales, y_scales, ranges, coord, data, theme, params) {
     if ((params$free$x || params$free$y) && !coord$is_free()) {
-      cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}")
+      cli::cli_abort("{.fn {snake_class(self)}} can't use free scales with {.fn {snake_class(coord)}}.")
     }
 
     if (inherits(coord, "CoordFlip")) {
@@ -470,8 +470,8 @@ wrap_dims <- function(n, nrow = NULL, ncol = NULL) {
   }
   if (nrow * ncol < n) {
     cli::cli_abort(c(
-      "Need {n} panels, but together {.arg nrow} and {.arg ncol} only provide {nrow * ncol}",
-      i = "Please increase {.arg ncol} and/or {.arg nrow}"
+      "Need {n} panel{?s}, but together {.arg nrow} and {.arg ncol} only provide {nrow * ncol}.",
+      i = "Please increase {.arg ncol} and/or {.arg nrow}."
     ))
   }
 

R/fortify.R

@@ -78,8 +78,8 @@ validate_as_data_frame <- function(data) {
 fortify.default <- function(model, data, ...) {
   msg0 <- paste0(
     "{{.arg data}} must be a {{.cls data.frame}}, ",
-    "or an object coercible by {{.code fortify()}}, or a valid ",
-    "{{.cls data.frame}}-like object coercible by {{.code as.data.frame()}}"
+    "or an object coercible by {{.fn fortify}}, or a valid ",
+    "{{.cls data.frame}}-like object coercible by {{.fn as.data.frame}}"
   )
   if (inherits(model, "uneval")) {
     msg <- c(

R/geom-.R

@@ -239,8 +239,8 @@ check_aesthetics <- function(x, n) {
   }
 
   cli::cli_abort(c(
-    "Aesthetics must be either length 1 or the same as the data ({n})",
-    "x" = "Fix the following mappings: {.col {names(which(!good))}}"
+    "Aesthetics must be either length 1 or the same as the data ({n}).",
+    "x" = "Fix the following mappings: {.col {names(which(!good))}}."
   ))
 }
 

R/geom-boxplot.R

@@ -230,7 +230,7 @@ GeomBoxplot <- ggproto("GeomBoxplot", Geom,
     # this may occur when using geom_boxplot(stat = "identity")
     if (nrow(data) != 1) {
       cli::cli_abort(c(
-        "Can only draw one boxplot per group",
+        "Can only draw one boxplot per group.",
         "i"= "Did you forget {.code aes(group = ...)}?"
       ))
     }