Warn about simultaneous use of mapped and static aesthetics (#6743)

* mapped aesthetics as variable

* all aesthetics as variable

* add warning

* add test

Author: teunbrand

R/layer.R

@@ -59,6 +59,7 @@ layer <- function(geom = NULL, stat = NULL,
   if (!is.null(mapping)) {
     mapping <- validate_mapping(mapping, call_env)
   }
+  mapped_aes <- mapped_aesthetics(mapping)
 
   data <- fortify(data)
 
@@ -70,13 +71,14 @@ layer <- function(geom = NULL, stat = NULL,
   params$na.rm <- params$na.rm %||% FALSE
 
   # Split up params between aesthetics, geom, and stat
+  all_aes <- unique(c(geom$aesthetics(), position$aesthetics(), stat$aesthetics()))
   params <- rename_aes(params)
-  aes_params  <- params[intersect(names(params), union(geom$aesthetics(), position$aesthetics()))]
+  aes_params  <- params[intersect(names(params), all_aes)]
   geom_params <- params[intersect(names(params), geom$parameters(TRUE))]
   stat_params <- params[intersect(names(params), stat$parameters(TRUE))]
 
   ignore <- c("key_glyph", "name", "layout")
-  all <- c(geom$parameters(TRUE), stat$parameters(TRUE), geom$aesthetics(), position$aesthetics(), ignore)
+  all <- c(geom$parameters(TRUE), stat$parameters(TRUE), all_aes, ignore)
 
   # Take care of plain patterns provided as aesthetic
   pattern <- vapply(aes_params, is_pattern, logical(1))
@@ -87,21 +89,31 @@ layer <- function(geom = NULL, stat = NULL,
   # Warn about extra params and aesthetics
   extra_param <- setdiff(names(params), all)
   # Take care of size->linewidth renaming in layer params
-  if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aesthetics(mapping)) {
+  if (geom$rename_size && "size" %in% extra_param && !"linewidth" %in% mapped_aes) {
     aes_params <- c(aes_params, params["size"])
     extra_param <- setdiff(extra_param, "size")
     deprecate("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"), user_env = user_env)
   }
-  if (check.param && length(extra_param) > 0) {
-    cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env)
+  if (check.param) {
+    if (length(extra_param) > 0) {
+      cli::cli_warn("Ignoring unknown parameters: {.arg {extra_param}}", call = call_env)
+    }
+    double_defined <- intersect(mapped_aes, names(aes_params))
+    if (length(double_defined) > 0) {
+      cli::cli_warn(
+        c(
+          "The {.and {.field {double_defined}}} aesthetic{?s} {?is/are} \\
+          defined twice: once in {.arg mapping} and once as a static aesthetic.",
+          "i" = "The static aesthetic overrules the mapped aesthetic."
+        ),
+        call = call_env
+      )
+    }
   }
 
-  extra_aes <- setdiff(
-    mapped_aesthetics(mapping),
-    c(geom$aesthetics(), stat$aesthetics(), position$aesthetics())
-  )
+  extra_aes <- setdiff(mapped_aes, all_aes)
   # Take care of size->linewidth aes renaming
-  if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aesthetics(mapping)) {
+  if (geom$rename_size && "size" %in% extra_aes && !"linewidth" %in% mapped_aes) {
     extra_aes <- setdiff(extra_aes, "size")
     deprecate("3.4.0", I("Using `size` aesthetic for lines"), I("`linewidth`"), user_env = user_env)
   }

tests/testthat/_snaps/layer.md

@@ -47,6 +47,11 @@
 
     Ignoring empty aesthetics: `fill` and `shape`.
 
+# aesthetics defined twice create warning
+
+    The size aesthetic is defined twice: once in `mapping` and once as a static aesthetic.
+    i The static aesthetic overrules the mapped aesthetic.
+
 # invalid aesthetics throws errors
 
     Problem while computing aesthetics.

tests/testthat/test-layer.R

@@ -33,6 +33,10 @@ test_that("empty aesthetics create warning", {
   expect_snapshot_warning(ggplot_build(p))
 })
 
+test_that("aesthetics defined twice create warning", {
+  expect_snapshot_warning(geom_point(aes(size = foo), size = 12))
+})
+
 test_that("invalid aesthetics throws errors", {
   # We want to test error and ignore the scale search message
   suppressMessages({