Warn on the use of ..var.. and stat(var) (#4950)

Author: yutannihilation

DESCRIPTION

@@ -36,7 +36,7 @@ Imports:
     grid,
     gtable (>= 0.1.1),
     isoband,
-    lifecycle,
+    lifecycle (> 1.0.1),
     MASS,
     mgcv,
     rlang (>= 1.0.0),
@@ -71,6 +71,8 @@ Suggests:
     testthat (>= 3.1.2),
     vdiffr (>= 1.0.0),
     xml2
+Remotes:
+    r-lib/lifecycle
 Enhances: 
     sp
 VignetteBuilder: 

NEWS.md

@@ -1,5 +1,8 @@
 # ggplot2 (development version)
 
+* The dot-dot notation (`..var..`) and `stat()`, which have been superseded by
+  `after_stat()`, are now formally deprecated (@yutannihilation, #3693).
+
 * `geom_col()` and `geom_bar()` gain a new `just` argument. This is set to `0.5`
   by default; use `just = 0`/`just = 1` to place columns on the left/right
   of the axis breaks.

R/aes-evaluation.r

@@ -93,31 +93,44 @@ is_dotted_var <- function(x) {
 }
 
 # Determine if aesthetic is calculated
-is_calculated_aes <- function(aesthetics) {
-  vapply(aesthetics, is_calculated, logical(1), USE.NAMES = FALSE)
+is_calculated_aes <- function(aesthetics, warn = FALSE) {
+  vapply(aesthetics, is_calculated, warn = warn, logical(1), USE.NAMES = FALSE)
 }
 is_scaled_aes <- function(aesthetics) {
   vapply(aesthetics, is_scaled, logical(1), USE.NAMES = FALSE)
 }
 is_staged_aes <- function(aesthetics) {
   vapply(aesthetics, is_staged, logical(1), USE.NAMES = FALSE)
 }
-is_calculated <- function(x) {
+is_calculated <- function(x, warn = FALSE) {
   if (is_call(get_expr(x), "after_stat")) {
     return(TRUE)
   }
   # Support of old recursive behaviour
   if (is.atomic(x)) {
     FALSE
   } else if (is.symbol(x)) {
-    is_dotted_var(as.character(x))
+    res <- is_dotted_var(as.character(x))
+    if (res && warn) {
+      what <- I(glue("The dot-dot notation (`{x}`)"))
+      var <- gsub(match_calculated_aes, "\\1", as.character(x))
+      with <- I(glue("`after_stat({var})`"))
+      lifecycle::deprecate_warn("3.4.0", what, with, id = "ggplot-warn-aes-dot-dot")
+    }
+    res
   } else if (is_quosure(x)) {
-    is_calculated(quo_get_expr(x))
+    is_calculated(quo_get_expr(x), warn = warn)
   } else if (is.call(x)) {
     if (identical(x[[1]], quote(stat))) {
+      if (warn) {
+        what <- I(glue("`{expr_deparse(x)}`"))
+        x[[1]] <- quote(after_stat)
+        with <- I(glue("`{expr_deparse(x)}`"))
+        lifecycle::deprecate_warn("3.4.0", what, with, id = "ggplot-warn-aes-stat")
+      }
       TRUE
     } else {
-      any(vapply(x, is_calculated, logical(1)))
+      any(vapply(x, is_calculated, warn = warn, logical(1)))
     }
   } else if (is.pairlist(x)) {
     FALSE

R/layer.r

@@ -129,11 +129,7 @@ layer <- function(geom = NULL, stat = NULL,
   if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aesthetics(mapping)) {
     aes_params <- c(aes_params, params["size"])
     extra_param <- setdiff(extra_param, "size")
-    # TODO: move to cli_warn()
-    cli::cli_inform(c(
-      "{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
-      "i" = "Please use {.field linewidth} aesthetic instead"
-    ), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
+    lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
   }
   if (check.param && length(extra_param) > 0) {
     cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env)
@@ -146,11 +142,7 @@ layer <- function(geom = NULL, stat = NULL,
   # Take care of size->linewidth aes renaming
   if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aesthetics(mapping)) {
     extra_aes <- setdiff(extra_aes, "size")
-    # TODO: move to cli_warn()
-    cli::cli_inform(c(
-      "{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
-      "i" = "Please use {.field linewidth} aesthetic instead"
-    ), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
+    lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
   }
   if (check.aes && length(extra_aes) > 0) {
     cli::cli_warn("Ignoring unknown aesthetics: {.field {extra_aes}}", call = call_env)
@@ -247,11 +239,7 @@ Layer <- ggproto("Layer", NULL,
           !"linewidth" %in% names(self$computed_mapping) &&
           "linewidth" %in% self$geom$aesthetics()) {
         self$computed_mapping$size <- plot$mapping$size
-        # TODO: move to cli_warn()
-        cli::cli_inform(c(
-          "{.field size} aesthetic has been deprecated for use with lines as of ggplot2 3.4.0",
-          "i" = "Please use {.field linewidth} aesthetic instead"
-        ), .frequency = "regularly", .frequency_id = "ggplot-size-linewidth")
+        lifecycle::deprecate_warn("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"))
       }
       # defaults() strips class, but it needs to be preserved for now
       class(self$computed_mapping) <- "uneval"
@@ -267,7 +255,7 @@ Layer <- ggproto("Layer", NULL,
 
     # Drop aesthetics that are set or calculated
     set <- names(aesthetics) %in% names(self$aes_params)
-    calculated <- is_calculated_aes(aesthetics)
+    calculated <- is_calculated_aes(aesthetics, warn = TRUE)
     modifiers <- is_scaled_aes(aesthetics)
 
     aesthetics <- aesthetics[!set & !calculated & !modifiers]

tests/testthat/_snaps/aes-calculated.md

@@ -6,3 +6,13 @@
 
     Duplicated aesthetics after name standardisation: colour
 
+# A deprecated warning is issued when stat(var) or ..var.. is used
+
+    `stat(foo)` was deprecated in ggplot2 3.4.0.
+    Please use `after_stat(foo)` instead.
+
+---
+
+    The dot-dot notation (`..bar..`) was deprecated in ggplot2 3.4.0.
+    Please use `after_stat(bar)` instead.
+

tests/testthat/_snaps/layer.md

@@ -47,7 +47,7 @@
     x `fill = after_stat(data)`
     i Did you map your stat in the wrong layer?
 
-# function aesthetics are wrapped with stat()
+# function aesthetics are wrapped with after_stat()
 
     Problem while computing aesthetics.
     i Error occurred in the 1st layer.

tests/testthat/test-aes-calculated.r

@@ -69,3 +69,11 @@ test_that("staged aesthetics warn appropriately for duplicated names", {
   # One warning in building due to `stage()`/`after_scale()`
   expect_snapshot_warning(ggplot_build(p))
 })
+
+test_that("A deprecated warning is issued when stat(var) or ..var.. is used", {
+  p1 <- ggplot(NULL, aes(stat(foo)))
+  expect_snapshot_warning(b1 <- ggplot_build(p1))
+
+  p2 <- ggplot(NULL, aes(..bar..))
+  expect_snapshot_warning(b2 <- ggplot_build(p2))
+})

tests/testthat/test-geom-sf.R

@@ -99,18 +99,18 @@ test_that("geom_sf() removes rows containing missing aes", {
     colour = c("red", NA)
   )
 
-  p <- ggplot(pts) + geom_sf()
+  p <- ggplot(pts)
   expect_warning(
-    expect_identical(grob_xy_length(p + aes(size = size)), c(1L, 1L)),
+    expect_identical(grob_xy_length(p + geom_sf(aes(size = size))), c(1L, 1L)),
     "Removed 1 row containing missing values"
   )
   expect_warning(
-    expect_identical(grob_xy_length(p + aes(shape = shape)), c(1L, 1L)),
+    expect_identical(grob_xy_length(p + geom_sf(aes(shape = shape))), c(1L, 1L)),
     "Removed 1 row containing missing values"
   )
   # default colour scale maps a colour even to a NA, so identity scale is needed to see if NA is removed
   expect_warning(
-    expect_identical(grob_xy_length(p + aes(colour = colour) + scale_colour_identity()),
+    expect_identical(grob_xy_length(p + geom_sf(aes(colour = colour)) + scale_colour_identity()),
                      c(1L, 1L)),
     "Removed 1 row containing missing values"
   )

tests/testthat/test-layer.r

@@ -55,7 +55,7 @@ test_that("missing aesthetics trigger informative error", {
   )
 })
 
-test_that("function aesthetics are wrapped with stat()", {
+test_that("function aesthetics are wrapped with after_stat()", {
   df <- data_frame(x = 1:10)
   expect_snapshot_error(
     ggplot_build(ggplot(df, aes(colour = density, fill = density)) + geom_point())
@@ -65,7 +65,7 @@ test_that("function aesthetics are wrapped with stat()", {
 test_that("computed stats are in appropriate layer", {
   df <- data_frame(x = 1:10)
   expect_snapshot_error(
-    ggplot_build(ggplot(df, aes(colour = stat(density), fill = stat(density))) + geom_point())
+    ggplot_build(ggplot(df, aes(colour = after_stat(density), fill = after_stat(density))) + geom_point())
   )
 })
 

tests/testthat/test-stat-contour.R

@@ -65,7 +65,7 @@ test_that("geom_contour() and stat_contour() result in identical layer data", {
 
 test_that("basic stat_contour() plot builds", {
   p <- ggplot(faithfuld, aes(waiting, eruptions)) +
-    geom_contour(aes(z = density, col = factor(stat(level))))
+    geom_contour(aes(z = density, col = factor(after_stat(level))))
 
   # stat_contour() visual tests are unstable due to the
   # implementation in isoband

tests/testthat/test-stat-sf-coordinates.R

@@ -28,7 +28,7 @@ test_that("stat_sf_coordinates() retrieves coordinates from sf objects", {
   # computed variables (x and y)
   df_point <- sf::st_sf(geometry = sf::st_sfc(sf::st_point(c(1, 2))))
   expect_identical(
-    comp_sf_coord(df_point, aes(x = stat(x) + 10, y = stat(y) * 10))[, c("x", "y")],
+    comp_sf_coord(df_point, aes(x = after_stat(x) + 10, y = after_stat(y) * 10))[, c("x", "y")],
     data_frame(x = 11, y = 20)
   )
 })