Clearer error messages for invalid aesthetics (#3091)

* Clearer error messages for invalid aesthetics

Fixes #3060

* closes #3060

* Modified error messages, fixed spacing, right line endings

* Replace use of deparse with rlang::as_label

* Fix NEWS mispelling and punctuation. layer.R format suggestions

* Left in merge >>>, removed

* Remove punctuation in test-layer.R that was failing

* Add support for multiple problematic aesthetics, fix NEWS.md addition

* Change error to display whole problematic phrase, like fill = density

* Update tests to be more specific

* Fix failing tests, ideally

* Another go at test passing

* expect_error uses regex, so ( caused failing. Added fixed = TRUE

Author: clairemcwhite

NEWS.md

@@ -91,6 +91,8 @@ core developer team.
 
 * ggplot2 now works in Turkish locale (@yutannihilation, #3011).
 
+* Clearer error messages for inappropriate aesthetics (@clairemcwhite, #3060).
+
 * ggplot2 no longer attaches any external packages when using functions that 
   depend on packages that are suggested but not imported by ggplot2. The 
   affected functions include `geom_hex()`, `stat_binhex()`, 

R/layer.r

@@ -242,6 +242,16 @@ Layer <- ggproto("Layer", NULL,
     evaled <- lapply(aesthetics, eval_tidy, data = data)
     evaled <- compact(evaled)
 
+    nondata_cols <- check_nondata_cols(evaled)
+    if (length(nondata_cols) > 0) {
+      msg <- paste0(
+        "Aesthetics must be valid data columns. Problematic aesthetic(s): ",
+        paste0(vapply(nondata_cols, function(x) {paste0(x, " = ", as_label(aesthetics[[x]]))}, character(1)), collapse = ", "),
+        ". \nDid you mistype the name of a data column or forget to add stat()?"
+      )
+      stop(msg, call. = FALSE)
+    }
+
     n <- nrow(data)
     if (n == 0) {
       # No data, so look at longest evaluated aesthetic
@@ -293,6 +303,18 @@ Layer <- ggproto("Layer", NULL,
     env$stat <- stat
 
     stat_data <- new_data_frame(lapply(new, eval_tidy, data, env))
+
+    # Check that all columns in aesthetic stats are valid data
+    nondata_stat_cols <- check_nondata_cols(stat_data)
+    if (length(nondata_stat_cols) > 0) {
+      msg <- paste0(
+        "Aesthetics must be valid computed stats. Problematic aesthetic(s): ",
+        paste0(vapply(nondata_stat_cols, function(x) {paste0(x, " = ", as_label(aesthetics[[x]]))}, character(1)), collapse = ", "),
+        ". \nDid you map your stat in the wrong layer?"
+      )
+      stop(msg, call. = FALSE)
+    }
+
     names(stat_data) <- names(new)
 
     # Add any new scales, if needed

R/utilities.r

@@ -350,6 +350,14 @@ is.discrete <- function(x) {
   is.factor(x) || is.character(x) || is.logical(x)
 }
 
+# This function checks that all columns of a dataframe `x` are data and
+# returns the names of any columns that are not.
+# We define "data" as atomic types or lists, not functions or otherwise
+check_nondata_cols <- function(x) {
+  idx <- (vapply(x, function(x) rlang::is_vector(x), logical(1)))
+  names(x)[which(!idx)]
+}
+
 compact <- function(x) {
   null <- vapply(x, is.null, logical(1))
   x[!null]

tests/testthat/test-layer.r

@@ -38,6 +38,24 @@ test_that("missing aesthetics trigger informative error", {
   )
 })
 
+test_that("function aesthetics are wrapped with stat()", {
+  df <- data_frame(x = 1:10)
+  expect_error(
+    ggplot_build(ggplot(df, aes(colour = density, fill = density)) + geom_point()),
+    "Aesthetics must be valid data columns. Problematic aesthetic(s): colour = density, fill = density",
+    fixed = TRUE
+  )
+})
+
+test_that("computed stats are in appropriate layer", {
+  df <- data_frame(x = 1:10)
+  expect_error(
+    ggplot_build(ggplot(df, aes(colour = stat(density), fill = stat(density))) + geom_point()),
+    "Aesthetics must be valid computed stats. Problematic aesthetic(s): colour = stat(density), fill = stat(density)",
+    fixed = TRUE
+  )
+})
+
 test_that("if an aes is mapped to a function that returns NULL, it is removed", {
   df <- data_frame(x = 1:10)
   null <- function(...) NULL