From f2aeb9d008de5e623aeb5a0690ca2d90a141fc5e Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 23 Feb 2019 15:40:26 +0900
Subject: [PATCH 01/23] Allow empty facetting

---
 R/facet-grid-.r |  9 +++++++++
 R/facet-wrap.r  | 13 ++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index 13d213ca1f..5a52d93b9e 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -223,6 +223,10 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     base_cols <- combine_vars(data, params$plot_env, cols, drop = params$drop)
     base <- df.grid(base_rows, base_cols)
 
+    if (nrow(base) == 0) {
+      return(new_data_frame(list(PANEL = 1L, ROW = 1L, COL = 1L, SCALE_X = 1L, SCALE_Y = 1L)))
+    }
+
     # Add margins
     base <- reshape2::add_margins(base, list(names(rows), names(cols)), params$margins)
     # Work around bug in reshape2
@@ -253,6 +257,11 @@ FacetGrid <- ggproto("FacetGrid", Facet,
     cols <- params$cols
     vars <- c(names(rows), names(cols))
 
+    if (length(vars) == 0) {
+      data$PANEL <- layout$PANEL
+      return(data)
+    }
+
     # Compute faceting values and add margins
     margin_vars <- list(intersect(names(rows), names(data)),
       intersect(names(cols), names(data)))
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index 78c112751e..b381ae9aa3 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -177,8 +177,14 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     if (empty(data)) {
       return(cbind(data, PANEL = integer(0)))
     }
+
     vars <- params$facets
 
+    if (length(vars) == 0) {
+      data$PANEL <- 1L
+      return(data)
+    }
+
     facet_vals <- eval_facets(vars, data, params$plot_env)
     facet_vals[] <- lapply(facet_vals[], as.factor)
 
@@ -229,7 +235,12 @@ FacetWrap <- ggproto("FacetWrap", Facet,
 
     axes <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
 
-    labels_df <- layout[names(params$facets)]
+    if (length(params$facets) == 0) {
+      # add a dummy labels
+      labels_df <- new_data_frame(list("(empty)" = "(empty)"), n = 1)
+    } else {
+      labels_df <- layout[names(params$facets)]
+    }
     attr(labels_df, "facet") <- "wrap"
     strips <- render_strips(
       structure(labels_df, type = "rows"),

From 36829a808415427a6781aa46dd9039bcfeb01cde Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 23 Feb 2019 15:48:14 +0900
Subject: [PATCH 02/23] Compact facet specs

---
 R/facet-.r      | 4 ----
 R/facet-grid-.r | 2 +-
 R/facet-wrap.r  | 1 +
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 6eb990c3a6..53de8ff664 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -311,10 +311,6 @@ as_facets_list <- function(x) {
     x <- lapply(x, as_facets)
   }
 
-  if (sum(vapply(x, length, integer(1))) == 0L) {
-    stop("Must specify at least one variable to facet by", call. = FALSE)
-  }
-
   x
 }
 
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index 5a52d93b9e..c5d2378c6b 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -192,7 +192,7 @@ grid_as_facets_list <- function(rows, cols) {
     cols <- rlang::quos_auto_name(cols)
   }
 
-  list(rows, cols)
+  list(compact(rows), compact(cols))
 }
 
 #' @rdname ggplot2-ggproto
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index b381ae9aa3..ad8d4336ff 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -111,6 +111,7 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
   # Flatten all facets dimensions into a single one
   facets_list <- as_facets_list(facets)
   facets <- rlang::flatten_if(facets_list, rlang::is_list)
+  facets <- compact(facets)
 
   ggproto(NULL, FacetWrap,
     shrink = shrink,

From 81dcbc16853457d10c4ec14db2958e574f65decd Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 24 Feb 2019 22:10:27 +0900
Subject: [PATCH 03/23] Use "(all)" for the dummy strip label

---
 R/facet-wrap.r | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index ad8d4336ff..e2817e1971 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -238,7 +238,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
 
     if (length(params$facets) == 0) {
       # add a dummy labels
-      labels_df <- new_data_frame(list("(empty)" = "(empty)"), n = 1)
+      labels_df <- new_data_frame(list("(all)" = "(all)"), n = 1)
     } else {
       labels_df <- layout[names(params$facets)]
     }

From 0e2d33f5c72bb8de9f6e8953ec2a81e8b0e11f4d Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 20:50:40 +0900
Subject: [PATCH 04/23] Add wrap_as_facets_list()

---
 R/facet-wrap.r | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index e2817e1971..a0e6b16fd8 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -109,9 +109,7 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
   labeller <- check_labeller(labeller)
 
   # Flatten all facets dimensions into a single one
-  facets_list <- as_facets_list(facets)
-  facets <- rlang::flatten_if(facets_list, rlang::is_list)
-  facets <- compact(facets)
+  facets <- wrap_as_facets_list(facets)
 
   ggproto(NULL, FacetWrap,
     shrink = shrink,
@@ -129,6 +127,12 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
   )
 }
 
+wrap_as_facets_list <- function(x) {
+  facets_list <- as_facets_list(x)
+  facets <- rlang::flatten_if(facets_list, rlang::is_list)
+  rlang::as_quosures(compact(facets))
+}
+
 #' @rdname ggplot2-ggproto
 #' @format NULL
 #' @usage NULL

From 8719f91534aab5a184132f729070a52a557d4212 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 20:51:41 +0900
Subject: [PATCH 05/23] Compact in as_facets_list() and simplify
 grid_as_facets_list()

---
 R/facet-.r      | 19 +++++++++----------
 R/facet-grid-.r | 20 ++++++++------------
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 53de8ff664..6d73f2ae5a 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -274,7 +274,7 @@ df.grid <- function(a, b) {
 # creates a facets list with two components, each of which bundles two
 # facetting variables.
 
-as_facets_list <- function(x) {
+expand_facet_specs <- function(x) {
   if (inherits(x, "mapping")) {
     stop("Please use `vars()` to supply facet variables")
   }
@@ -314,6 +314,13 @@ as_facets_list <- function(x) {
   x
 }
 
+# get compacted specs
+as_facets_list <- function(x) {
+  x <- expand_facet_specs(x)
+  x <- lapply(x, compact)
+  x
+}
+
 # Compatibility with plyr::as.quoted()
 as_quoted <- function(x) {
   if (is.character(x)) {
@@ -356,15 +363,7 @@ f_as_facets_list <- function(f) {
   rows <- f_as_facets(lhs(f))
   cols <- f_as_facets(rhs(f))
 
-  if (length(rows) + length(cols) == 0) {
-    stop("Must specify at least one variable to facet by", call. = FALSE)
-  }
-
-  if (length(rows)) {
-    list(rows, cols)
-  } else {
-    list(cols)
-  }
+  list(rows, cols)
 }
 
 as_facets <- function(x) {
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index c5d2378c6b..c0e0ff94a2 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -145,16 +145,8 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
   }
 
   facets_list <- grid_as_facets_list(rows, cols)
-  n <- length(facets_list)
-  if (n > 2L) {
-    stop("A grid facet specification can't have more than two dimensions", call. = FALSE)
-  }
-  if (n == 1L) {
-    rows <- quos()
-    cols <- facets_list[[1]]
-  } else {
-    rows <- facets_list[[1]]
-    cols <- facets_list[[2]]
+  if (length(facets_list) != 2L) {
+    stop("A grid facet specification must have exactly 2 specs; rows and cols", call. = FALSE)
   }
 
   # Check for deprecated labellers
@@ -162,7 +154,7 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
 
   ggproto(NULL, FacetGrid,
     shrink = shrink,
-    params = list(rows = rows, cols = cols, margins = margins,
+    params = list(rows = facets_list[[1]], cols = facets_list[[2]], margins = margins,
       free = free, space_free = space_free, labeller = labeller,
       as.table = as.table, switch = switch, drop = drop)
   )
@@ -173,7 +165,11 @@ grid_as_facets_list <- function(rows, cols) {
     if (!is.null(cols)) {
       stop("`rows` must be `NULL` or a `vars()` list if `cols` is a `vars()` list", call. = FALSE)
     }
-    return(as_facets_list(rows))
+    facets <- as_facets_list(rows)
+    if (length(facets) == 1) { # e.g. facet_grid(list(~ a + b))
+      facets[2] <- quos()
+    }
+    return(facets)
   }
 
   is_cols_vars <- is.null(cols) || rlang::is_quosures(cols)

From 075c3282542935ea4893613c14a744b5883a4d7d Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 20:51:57 +0900
Subject: [PATCH 06/23] Reform some part of tests

---
 tests/testthat/test-facet-.r | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index ed57f2e0b5..be8857c8e4 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -1,8 +1,11 @@
 context("Facetting")
 
 test_that("as_facets_list() coerces formulas", {
-  expect_identical(as_facets_list(~foo), list(quos(foo = foo)))
-  expect_identical(as_facets_list(~foo + bar), list(quos(foo = foo, bar = bar)))
+  expect_identical(as_facets_list(~foo), list(quos(), quos(foo = foo)))
+  expect_identical(wrap_as_facets_list(~foo), quos(foo = foo))
+
+  expect_identical(as_facets_list(~foo + bar), list(quos(), quos(foo = foo, bar = bar)))
+  expect_identical(wrap_as_facets_list(~foo + bar), quos(foo = foo, bar = bar))
 
   expect_identical(as_facets_list(foo ~ bar), list(quos(foo = foo), quos(bar = bar)))
 
@@ -18,8 +21,9 @@ test_that("as_facets_list() coerces strings containing formulas", {
 })
 
 test_that("as_facets_list() coerces character vectors", {
-  expect_identical(as_facets_list("foo"), as_facets_list(local(~foo, globalenv())))
-  expect_identical(as_facets_list(c("foo", "bar")), as_facets_list(local(foo ~ bar, globalenv())))
+  expect_identical(as_facets_list("foo"), list(quos(foo = foo)))
+  expect_identical(as_facets_list(c("foo", "bar")), list(quos(foo = foo), quos(bar = bar)))
+  expect_identical(wrap_as_facets_list(c("foo", "bar")), quos(foo = foo, bar = bar))
 })
 
 test_that("as_facets_list() coerces lists", {
@@ -36,11 +40,16 @@ test_that("as_facets_list() coerces lists", {
   expect_identical(out, exp)
 })
 
-test_that("as_facets_list() errors with empty specs", {
-  expect_error(as_facets_list(list()), "at least one variable to facet by")
-  expect_error(as_facets_list(. ~ .), "at least one variable to facet by")
-  expect_error(as_facets_list(list(. ~ .)), "at least one variable to facet by")
-  expect_error(as_facets_list(list(NULL)), "at least one variable to facet by")
+test_that("as_facets_list() accepts empty specs", {
+  expect_equal(as_facets_list(list()), list())
+  expect_equal(as_facets_list(. ~ .), list(quos(), quos()))
+  expect_equal(as_facets_list(list(. ~ .)), list(quos()))
+  expect_equal(as_facets_list(list(NULL)), list(quos()))
+  
+  expect_equal(wrap_as_facets_list(list()), quos())
+  expect_equal(wrap_as_facets_list(. ~ .), quos())
+  expect_equal(wrap_as_facets_list(list(. ~ .)), quos())
+  expect_equal(wrap_as_facets_list(list(NULL)), quos())
 })
 
 test_that("as_facets_list() coerces quosure lists", {

From e14a37118d19e243dbb5827906ec4fcbe5c71e61 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 22:29:41 +0900
Subject: [PATCH 07/23] Fix check about aes()

---
 R/facet-.r                   | 6 +++---
 tests/testthat/test-facet-.r | 5 +++++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 6d73f2ae5a..d771773ec4 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -275,10 +275,10 @@ df.grid <- function(a, b) {
 # facetting variables.
 
 expand_facet_specs <- function(x) {
-  if (inherits(x, "mapping")) {
-    stop("Please use `vars()` to supply facet variables")
+  if (inherits(x, "uneval")) {
+    stop("Please use `vars()` to supply facet variables", call. = FALSE)
   }
-  if (inherits(x, "quosures")) {
+  if (rlang::is_quosures(x)) {
     x <- rlang::quos_auto_name(x)
     return(list(x))
   }
diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index be8857c8e4..018eca6e2a 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -56,6 +56,11 @@ test_that("as_facets_list() coerces quosure lists", {
   expect_identical(as_facets_list(vars(foo)), list(rlang::quos(foo = foo)))
 })
 
+test_that("facets rejects aes()", {
+  expect_error(as_facets_list(aes(foo)), "Please use `vars()` to supply facet variables")
+  expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables")
+  expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables")
+})
 
 df <- data_frame(x = 1:3, y = 3:1, z = letters[1:3])
 

From 45db9314bc2555c6398eba4fe301eb13ed741301 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 23:46:33 +0900
Subject: [PATCH 08/23] Seperate validation

---
 R/facet-.r      | 7 ++++---
 R/facet-grid-.r | 3 +++
 R/facet-wrap.r  | 2 ++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index d771773ec4..92916b9b2f 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -275,9 +275,6 @@ df.grid <- function(a, b) {
 # facetting variables.
 
 expand_facet_specs <- function(x) {
-  if (inherits(x, "uneval")) {
-    stop("Please use `vars()` to supply facet variables", call. = FALSE)
-  }
   if (rlang::is_quosures(x)) {
     x <- rlang::quos_auto_name(x)
     return(list(x))
@@ -319,6 +316,10 @@ as_facets_list <- function(x) {
   x <- expand_facet_specs(x)
   x <- lapply(x, compact)
   x
+validate_facet_specs <- function(x) {
+  if (inherits(x, "uneval")) {
+    stop("Please use `vars()` to supply facet variables", call. = FALSE)
+  }
 }
 
 # Compatibility with plyr::as.quoted()
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index c0e0ff94a2..f4a77377e6 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -160,6 +160,9 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
   )
 }
 grid_as_facets_list <- function(rows, cols) {
+  validate_facet_specs(rows)
+  validate_facet_specs(cols)
+
   is_rows_vars <- is.null(rows) || rlang::is_quosures(rows)
   if (!is_rows_vars) {
     if (!is.null(cols)) {
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index a0e6b16fd8..8175a13ff1 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -128,6 +128,8 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
 }
 
 wrap_as_facets_list <- function(x) {
+  validate_facet_specs(x)
+
   facets_list <- as_facets_list(x)
   facets <- rlang::flatten_if(facets_list, rlang::is_list)
   rlang::as_quosures(compact(facets))

From fb3ce9a7eb239422793c63aa4cbfc3926eafdaf4 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 23:47:41 +0900
Subject: [PATCH 09/23] Do not compact in as_facets_list()

---
 R/facet-.r | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 92916b9b2f..38bcb35caf 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -274,7 +274,7 @@ df.grid <- function(a, b) {
 # creates a facets list with two components, each of which bundles two
 # facetting variables.
 
-expand_facet_specs <- function(x) {
+as_facets_list <- function(x) {
   if (rlang::is_quosures(x)) {
     x <- rlang::quos_auto_name(x)
     return(list(x))
@@ -311,11 +311,6 @@ expand_facet_specs <- function(x) {
   x
 }
 
-# get compacted specs
-as_facets_list <- function(x) {
-  x <- expand_facet_specs(x)
-  x <- lapply(x, compact)
-  x
 validate_facet_specs <- function(x) {
   if (inherits(x, "uneval")) {
     stop("Please use `vars()` to supply facet variables", call. = FALSE)

From 0d0dd338f094896d9c14a3f3f8fb0f9cd8ed2cd6 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 23:48:14 +0900
Subject: [PATCH 10/23] Use more concrete facet specs

---
 R/facet-grid-.r | 33 ++++++++++++++++-----------------
 R/facet-wrap.r  |  1 +
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index f4a77377e6..487df45020 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -145,7 +145,7 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
   }
 
   facets_list <- grid_as_facets_list(rows, cols)
-  if (length(facets_list) != 2L) {
+  if (!identical(names(facets_list), c("rows", "cols"))) {
     stop("A grid facet specification must have exactly 2 specs; rows and cols", call. = FALSE)
   }
 
@@ -154,11 +154,13 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
 
   ggproto(NULL, FacetGrid,
     shrink = shrink,
-    params = list(rows = facets_list[[1]], cols = facets_list[[2]], margins = margins,
+    params = list(rows = facets_list$rows, cols = facets_list$cols, margins = margins,
       free = free, space_free = space_free, labeller = labeller,
       as.table = as.table, switch = switch, drop = drop)
   )
 }
+
+# returns a list containing exactly two quosures `rows` and `cols`
 grid_as_facets_list <- function(rows, cols) {
   validate_facet_specs(rows)
   validate_facet_specs(cols)
@@ -168,11 +170,14 @@ grid_as_facets_list <- function(rows, cols) {
     if (!is.null(cols)) {
       stop("`rows` must be `NULL` or a `vars()` list if `cols` is a `vars()` list", call. = FALSE)
     }
+    # For backward-compatibility
     facets <- as_facets_list(rows)
-    if (length(facets) == 1) { # e.g. facet_grid(list(~ a + b))
-      facets[2] <- quos()
+    if (length(facets) > 2L) {
+      stop("A grid facet specification can't have more than two dimensions", call. = FALSE)
     }
-    return(facets)
+    rows <- if (length(facets) >= 1) facets[[1]] else rlang::quos()
+    cols <- if (length(facets) >= 2) facets[[2]] else rlang::quos()
+    return(list(rows = rows, cols = cols))
   }
 
   is_cols_vars <- is.null(cols) || rlang::is_quosures(cols)
@@ -180,18 +185,12 @@ grid_as_facets_list <- function(rows, cols) {
     stop("`cols` must be `NULL` or a `vars()` specification", call. = FALSE)
   }
 
-  if (is.null(rows)) {
-    rows <- quos()
-  } else {
-    rows <- rlang::quos_auto_name(rows)
-  }
-  if (is.null(cols)) {
-    cols <- quos()
-  } else {
-    cols <- rlang::quos_auto_name(cols)
-  }
-
-  list(compact(rows), compact(cols))
+  rows <- compact(rows %||% rlang::quos())
+  cols <- compact(cols %||% rlang::quos())
+  list(
+    rows = rlang::quos_auto_name(rows),
+    cols = rlang::quos_auto_name(cols)
+  )
 }
 
 #' @rdname ggplot2-ggproto
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index 8175a13ff1..d4b739b14a 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -127,6 +127,7 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
   )
 }
 
+# returns quosures
 wrap_as_facets_list <- function(x) {
   validate_facet_specs(x)
 

From ba757c6d5dbc2fb4306e84fdccd6472648eee79a Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sat, 2 Mar 2019 23:48:42 +0900
Subject: [PATCH 11/23] Fix tests

---
 tests/testthat/test-facet-.r | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index 018eca6e2a..593962e7af 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -2,11 +2,7 @@ context("Facetting")
 
 test_that("as_facets_list() coerces formulas", {
   expect_identical(as_facets_list(~foo), list(quos(), quos(foo = foo)))
-  expect_identical(wrap_as_facets_list(~foo), quos(foo = foo))
-
   expect_identical(as_facets_list(~foo + bar), list(quos(), quos(foo = foo, bar = bar)))
-  expect_identical(wrap_as_facets_list(~foo + bar), quos(foo = foo, bar = bar))
-
   expect_identical(as_facets_list(foo ~ bar), list(quos(foo = foo), quos(bar = bar)))
 
   exp <- list(quos(foo = foo, bar = bar), quos(baz = baz, bam = bam))
@@ -16,6 +12,12 @@ test_that("as_facets_list() coerces formulas", {
   expect_identical(as_facets_list(foo() + bar() ~ baz() + bam()), exp)
 })
 
+test_that("wrap_as_facets_list() returns quosures", {
+  expect_identical(wrap_as_facets_list(~foo), quos(foo = foo))
+  expect_identical(wrap_as_facets_list(~foo + bar), quos(foo = foo, bar = bar))
+  expect_identical(wrap_as_facets_list(foo ~ bar), quos(foo = foo, bar = bar))
+})
+
 test_that("as_facets_list() coerces strings containing formulas", {
   expect_identical(as_facets_list("foo ~ bar"), as_facets_list(local(foo ~ bar, globalenv())))
 })
@@ -45,11 +47,16 @@ test_that("as_facets_list() accepts empty specs", {
   expect_equal(as_facets_list(. ~ .), list(quos(), quos()))
   expect_equal(as_facets_list(list(. ~ .)), list(quos()))
   expect_equal(as_facets_list(list(NULL)), list(quos()))
-  
+
   expect_equal(wrap_as_facets_list(list()), quos())
   expect_equal(wrap_as_facets_list(. ~ .), quos())
   expect_equal(wrap_as_facets_list(list(. ~ .)), quos())
   expect_equal(wrap_as_facets_list(list(NULL)), quos())
+
+  expect_equal(grid_as_facets_list(list(), NULL), list(rows = quos(), cols = quos()))
+  expect_equal(grid_as_facets_list(. ~ ., NULL), list(rows = quos(), cols = quos()))
+  expect_equal(grid_as_facets_list(list(. ~ .), NULL), list(rows = quos(), cols = quos()))
+  expect_equal(grid_as_facets_list(list(NULL), NULL), list(rows = quos(), cols = quos()))
 })
 
 test_that("as_facets_list() coerces quosure lists", {
@@ -57,9 +64,8 @@ test_that("as_facets_list() coerces quosure lists", {
 })
 
 test_that("facets rejects aes()", {
-  expect_error(as_facets_list(aes(foo)), "Please use `vars()` to supply facet variables")
-  expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables")
-  expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables")
+  expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
+  expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
 })
 
 df <- data_frame(x = 1:3, y = 3:1, z = letters[1:3])

From 16e294db13dc0e5686838159d19a514ec0994d01 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 00:05:15 +0900
Subject: [PATCH 12/23] Fix test about character

---
 tests/testthat/test-facet-.r | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index 593962e7af..328ff72f6b 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -23,9 +23,13 @@ test_that("as_facets_list() coerces strings containing formulas", {
 })
 
 test_that("as_facets_list() coerces character vectors", {
-  expect_identical(as_facets_list("foo"), list(quos(foo = foo)))
-  expect_identical(as_facets_list(c("foo", "bar")), list(quos(foo = foo), quos(bar = bar)))
-  expect_identical(wrap_as_facets_list(c("foo", "bar")), quos(foo = foo, bar = bar))
+  foo <- rlang::new_quosure(quote(foo), globalenv())
+  bar <- rlang::new_quosure(quote(bar), globalenv())
+  foobar <- rlang::as_quosures(list(foo, bar), named = TRUE)
+
+  expect_identical(as_facets_list("foo"), list(foobar[1]))
+  expect_identical(as_facets_list(c("foo", "bar")), list(foobar[1], foobar[2]))
+  expect_identical(wrap_as_facets_list(c("foo", "bar")), foobar)
 })
 
 test_that("as_facets_list() coerces lists", {

From eda75d9b56935a43f1a6fcd5cb5a3ddaf6492a03 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 16:20:35 +0900
Subject: [PATCH 13/23] Compact NULL quosures

---
 R/facet-.r      |  5 +++++
 R/facet-grid-.r |  4 ++--
 R/facet-wrap.r  | 17 ++++++++++++++---
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 38bcb35caf..bc17855d64 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -317,6 +317,11 @@ validate_facet_specs <- function(x) {
   }
 }
 
+compact_quos <- function(x) {
+  null <- vapply(x, rlang::quo_is_null, logical(1))
+  x[!null]
+}
+
 # Compatibility with plyr::as.quoted()
 as_quoted <- function(x) {
   if (is.character(x)) {
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index 487df45020..18827131e5 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -185,8 +185,8 @@ grid_as_facets_list <- function(rows, cols) {
     stop("`cols` must be `NULL` or a `vars()` specification", call. = FALSE)
   }
 
-  rows <- compact(rows %||% rlang::quos())
-  cols <- compact(cols %||% rlang::quos())
+  rows <- compact_quos(rows %||% rlang::quos())
+  cols <- compact_quos(cols %||% rlang::quos())
   list(
     rows = rlang::quos_auto_name(rows),
     cols = rlang::quos_auto_name(cols)
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index d4b739b14a..f3df292192 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -131,9 +131,20 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
 wrap_as_facets_list <- function(x) {
   validate_facet_specs(x)
 
-  facets_list <- as_facets_list(x)
-  facets <- rlang::flatten_if(facets_list, rlang::is_list)
-  rlang::as_quosures(compact(facets))
+  if (is.null(x)) {
+    return(rlang::quos())
+  }
+
+  if (rlang::is_quosures(x)) {
+    facets <- x
+  } else {
+    facets_list <- as_facets_list(x)
+    facets <- rlang::flatten_if(facets_list, rlang::is_list)
+    facets <- rlang::as_quosures(facets)
+  }
+
+  facets <- compact_quos(facets)
+  rlang::quos_auto_name(facets)
 }
 
 #' @rdname ggplot2-ggproto

From d17819f80044a8f2347d1cd84ba16d3c0cfacf1e Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 16:27:14 +0900
Subject: [PATCH 14/23] Add tests about compacting specs

---
 tests/testthat/test-facet-.r | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index 328ff72f6b..b0596c240a 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -46,12 +46,8 @@ test_that("as_facets_list() coerces lists", {
   expect_identical(out, exp)
 })
 
-test_that("as_facets_list() accepts empty specs", {
-  expect_equal(as_facets_list(list()), list())
-  expect_equal(as_facets_list(. ~ .), list(quos(), quos()))
-  expect_equal(as_facets_list(list(. ~ .)), list(quos()))
-  expect_equal(as_facets_list(list(NULL)), list(quos()))
-
+test_that("wrap_as_facets_list() and grid_as_facets_list() accept empty specs", {
+  expect_equal(wrap_as_facets_list(NULL), quos())
   expect_equal(wrap_as_facets_list(list()), quos())
   expect_equal(wrap_as_facets_list(. ~ .), quos())
   expect_equal(wrap_as_facets_list(list(. ~ .)), quos())
@@ -63,6 +59,11 @@ test_that("as_facets_list() accepts empty specs", {
   expect_equal(grid_as_facets_list(list(NULL), NULL), list(rows = quos(), cols = quos()))
 })
 
+test_that("wrap_as_facets_list() and grid_as_facets_list() compact specs", {
+  expect_equal(wrap_as_facets_list(vars(foo, NULL, bar)), quos(foo = foo, bar = bar))
+  expect_equal(grid_as_facets_list(vars(foo, NULL, bar), NULL), list(rows = quos(foo = foo, bar = bar), cols = quos()))
+})
+
 test_that("as_facets_list() coerces quosure lists", {
   expect_identical(as_facets_list(vars(foo)), list(rlang::quos(foo = foo)))
 })

From 2d735e2c374aa5a69a567dc69d6672b687faa3ac Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 22:41:07 +0900
Subject: [PATCH 15/23] Include validation in as_facets_list()

---
 R/facet-.r      | 15 +++++++--------
 R/facet-grid-.r |  9 ++-------
 R/facet-wrap.r  | 18 ++----------------
 3 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index bc17855d64..09eb33c921 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -275,6 +275,9 @@ df.grid <- function(a, b) {
 # facetting variables.
 
 as_facets_list <- function(x) {
+  if (inherits(x, "uneval")) {
+    stop("Please use `vars()` to supply facet variables", call. = FALSE)
+  }
   if (rlang::is_quosures(x)) {
     x <- rlang::quos_auto_name(x)
     return(list(x))
@@ -311,15 +314,11 @@ as_facets_list <- function(x) {
   x
 }
 
-validate_facet_specs <- function(x) {
-  if (inherits(x, "uneval")) {
-    stop("Please use `vars()` to supply facet variables", call. = FALSE)
-  }
-}
-
-compact_quos <- function(x) {
+# flatten a list of quosures to a quosures object, and remove NULL quosure
+compact_facets <- function(x) {
+  x <- rlang::flatten_if(x, rlang::is_list)
   null <- vapply(x, rlang::quo_is_null, logical(1))
-  x[!null]
+  rlang::as_quosures(x[!null])
 }
 
 # Compatibility with plyr::as.quoted()
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index 18827131e5..ea4eb9d851 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -162,9 +162,6 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
 
 # returns a list containing exactly two quosures `rows` and `cols`
 grid_as_facets_list <- function(rows, cols) {
-  validate_facet_specs(rows)
-  validate_facet_specs(cols)
-
   is_rows_vars <- is.null(rows) || rlang::is_quosures(rows)
   if (!is_rows_vars) {
     if (!is.null(cols)) {
@@ -185,11 +182,9 @@ grid_as_facets_list <- function(rows, cols) {
     stop("`cols` must be `NULL` or a `vars()` specification", call. = FALSE)
   }
 
-  rows <- compact_quos(rows %||% rlang::quos())
-  cols <- compact_quos(cols %||% rlang::quos())
   list(
-    rows = rlang::quos_auto_name(rows),
-    cols = rlang::quos_auto_name(cols)
+    rows = compact_facets(as_facets_list(rows)),
+    cols = compact_facets(as_facets_list(cols))
   )
 }
 
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index f3df292192..97d52f2b4f 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -129,22 +129,8 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
 
 # returns quosures
 wrap_as_facets_list <- function(x) {
-  validate_facet_specs(x)
-
-  if (is.null(x)) {
-    return(rlang::quos())
-  }
-
-  if (rlang::is_quosures(x)) {
-    facets <- x
-  } else {
-    facets_list <- as_facets_list(x)
-    facets <- rlang::flatten_if(facets_list, rlang::is_list)
-    facets <- rlang::as_quosures(facets)
-  }
-
-  facets <- compact_quos(facets)
-  rlang::quos_auto_name(facets)
+  facets_list <- as_facets_list(x)
+  compact_facets(facets_list)
 }
 
 #' @rdname ggplot2-ggproto

From 675d1ac39eaadfaebc2927788c956e507f76d5e4 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 23:06:34 +0900
Subject: [PATCH 16/23] Fix a typo

---
 R/facet-wrap.r | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index 97d52f2b4f..a199a9f164 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -241,7 +241,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     axes <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
 
     if (length(params$facets) == 0) {
-      # add a dummy labels
+      # add a dummy label
       labels_df <- new_data_frame(list("(all)" = "(all)"), n = 1)
     } else {
       labels_df <- layout[names(params$facets)]

From 65374821569682c3f7385d5eb1b522ba26949afa Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Sun, 3 Mar 2019 23:56:44 +0900
Subject: [PATCH 17/23] Remove a runtime assersion about grid facet specs

---
 R/facet-grid-.r | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index ea4eb9d851..ae0c4b46d5 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -145,9 +145,6 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
   }
 
   facets_list <- grid_as_facets_list(rows, cols)
-  if (!identical(names(facets_list), c("rows", "cols"))) {
-    stop("A grid facet specification must have exactly 2 specs; rows and cols", call. = FALSE)
-  }
 
   # Check for deprecated labellers
   labeller <- check_labeller(labeller)

From 095a72d1e0d2c7e503f3974e54a610e4c3ae23b8 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Mon, 4 Mar 2019 00:34:39 +0900
Subject: [PATCH 18/23] Simplify the logic

---
 R/facet-grid-.r | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index ae0c4b46d5..13cbf68fe7 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -165,13 +165,15 @@ grid_as_facets_list <- function(rows, cols) {
       stop("`rows` must be `NULL` or a `vars()` list if `cols` is a `vars()` list", call. = FALSE)
     }
     # For backward-compatibility
-    facets <- as_facets_list(rows)
-    if (length(facets) > 2L) {
+    facets_list <- as_facets_list(rows)
+    if (length(facets_list) > 2L) {
       stop("A grid facet specification can't have more than two dimensions", call. = FALSE)
     }
-    rows <- if (length(facets) >= 1) facets[[1]] else rlang::quos()
-    cols <- if (length(facets) >= 2) facets[[2]] else rlang::quos()
-    return(list(rows = rows, cols = cols))
+    # fill with empty quosures
+    facets <- list(rows = rlang::quos(), cols = rlang::quos())
+    facets[seq_along(facets_list)] <- facets_list
+    # Do not compact the legacy specs
+    return(facets)
   }
 
   is_cols_vars <- is.null(cols) || rlang::is_quosures(cols)

From fa856d06094e796a325b83759fe3c5dbbbbff1c6 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Thu, 7 Mar 2019 20:05:20 +0900
Subject: [PATCH 19/23] Use expect_identical() for quosure tests

---
 tests/testthat/test-facet-.r | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index b0596c240a..f8aaa9c0de 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -47,21 +47,21 @@ test_that("as_facets_list() coerces lists", {
 })
 
 test_that("wrap_as_facets_list() and grid_as_facets_list() accept empty specs", {
-  expect_equal(wrap_as_facets_list(NULL), quos())
-  expect_equal(wrap_as_facets_list(list()), quos())
-  expect_equal(wrap_as_facets_list(. ~ .), quos())
-  expect_equal(wrap_as_facets_list(list(. ~ .)), quos())
-  expect_equal(wrap_as_facets_list(list(NULL)), quos())
+  expect_identical(wrap_as_facets_list(NULL), quos())
+  expect_identical(wrap_as_facets_list(list()), quos())
+  expect_identical(wrap_as_facets_list(. ~ .), quos())
+  expect_identical(wrap_as_facets_list(list(. ~ .)), quos())
+  expect_identical(wrap_as_facets_list(list(NULL)), quos())
 
-  expect_equal(grid_as_facets_list(list(), NULL), list(rows = quos(), cols = quos()))
-  expect_equal(grid_as_facets_list(. ~ ., NULL), list(rows = quos(), cols = quos()))
-  expect_equal(grid_as_facets_list(list(. ~ .), NULL), list(rows = quos(), cols = quos()))
-  expect_equal(grid_as_facets_list(list(NULL), NULL), list(rows = quos(), cols = quos()))
+  expect_identical(grid_as_facets_list(list(), NULL), list(rows = quos(), cols = quos()))
+  expect_identical(grid_as_facets_list(. ~ ., NULL), list(rows = quos(), cols = quos()))
+  expect_identical(grid_as_facets_list(list(. ~ .), NULL), list(rows = quos(), cols = quos()))
+  expect_identical(grid_as_facets_list(list(NULL), NULL), list(rows = quos(), cols = quos()))
 })
 
 test_that("wrap_as_facets_list() and grid_as_facets_list() compact specs", {
-  expect_equal(wrap_as_facets_list(vars(foo, NULL, bar)), quos(foo = foo, bar = bar))
-  expect_equal(grid_as_facets_list(vars(foo, NULL, bar), NULL), list(rows = quos(foo = foo, bar = bar), cols = quos()))
+  expect_identical(wrap_as_facets_list(vars(foo, NULL, bar)), quos(foo = foo, bar = bar))
+  expect_identical(grid_as_facets_list(vars(foo, NULL, bar), NULL), list(rows = quos(foo = foo, bar = bar), cols = quos()))
 })
 
 test_that("as_facets_list() coerces quosure lists", {

From a7c017b3a10f8ddf0796f4395ededbb8ff950bce Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Thu, 7 Mar 2019 20:17:10 +0900
Subject: [PATCH 20/23] Improve comments

---
 R/facet-.r                   | 2 +-
 R/facet-grid-.r              | 4 ++--
 R/facet-wrap.r               | 4 ++--
 tests/testthat/test-facet-.r | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/R/facet-.r b/R/facet-.r
index 09eb33c921..4a5adade99 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -314,7 +314,7 @@ as_facets_list <- function(x) {
   x
 }
 
-# flatten a list of quosures to a quosures object, and remove NULL quosure
+# Flatten a list of quosures objects to a quosures object, and compact it
 compact_facets <- function(x) {
   x <- rlang::flatten_if(x, rlang::is_list)
   null <- vapply(x, rlang::quo_is_null, logical(1))
diff --git a/R/facet-grid-.r b/R/facet-grid-.r
index 13cbf68fe7..4b1ba83357 100644
--- a/R/facet-grid-.r
+++ b/R/facet-grid-.r
@@ -157,7 +157,7 @@ facet_grid <- function(rows = NULL, cols = NULL, scales = "fixed",
   )
 }
 
-# returns a list containing exactly two quosures `rows` and `cols`
+# Returns a list of quosures objects. The list has exactly two elements, `rows` and `cols`.
 grid_as_facets_list <- function(rows, cols) {
   is_rows_vars <- is.null(rows) || rlang::is_quosures(rows)
   if (!is_rows_vars) {
@@ -169,7 +169,7 @@ grid_as_facets_list <- function(rows, cols) {
     if (length(facets_list) > 2L) {
       stop("A grid facet specification can't have more than two dimensions", call. = FALSE)
     }
-    # fill with empty quosures
+    # Fill with empty quosures
     facets <- list(rows = rlang::quos(), cols = rlang::quos())
     facets[seq_along(facets_list)] <- facets_list
     # Do not compact the legacy specs
diff --git a/R/facet-wrap.r b/R/facet-wrap.r
index a199a9f164..8c0b112c26 100644
--- a/R/facet-wrap.r
+++ b/R/facet-wrap.r
@@ -127,7 +127,7 @@ facet_wrap <- function(facets, nrow = NULL, ncol = NULL, scales = "fixed",
   )
 }
 
-# returns quosures
+# Returns a quosures object
 wrap_as_facets_list <- function(x) {
   facets_list <- as_facets_list(x)
   compact_facets(facets_list)
@@ -241,7 +241,7 @@ FacetWrap <- ggproto("FacetWrap", Facet,
     axes <- render_axes(ranges, ranges, coord, theme, transpose = TRUE)
 
     if (length(params$facets) == 0) {
-      # add a dummy label
+      # Add a dummy label
       labels_df <- new_data_frame(list("(all)" = "(all)"), n = 1)
     } else {
       labels_df <- layout[names(params$facets)]
diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index f8aaa9c0de..97f3473b5f 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -68,7 +68,7 @@ test_that("as_facets_list() coerces quosure lists", {
   expect_identical(as_facets_list(vars(foo)), list(rlang::quos(foo = foo)))
 })
 
-test_that("facets rejects aes()", {
+test_that("facets reject aes()", {
   expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
   expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
 })

From c0e4c76db4f57eab3cefbd5e623bb9d4101b0b52 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Thu, 7 Mar 2019 21:14:44 +0900
Subject: [PATCH 21/23] Improve test cases

---
 tests/testthat/test-facet-.r | 58 +++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/tests/testthat/test-facet-.r b/tests/testthat/test-facet-.r
index 97f3473b5f..133a26bbc2 100644
--- a/tests/testthat/test-facet-.r
+++ b/tests/testthat/test-facet-.r
@@ -12,12 +12,6 @@ test_that("as_facets_list() coerces formulas", {
   expect_identical(as_facets_list(foo() + bar() ~ baz() + bam()), exp)
 })
 
-test_that("wrap_as_facets_list() returns quosures", {
-  expect_identical(wrap_as_facets_list(~foo), quos(foo = foo))
-  expect_identical(wrap_as_facets_list(~foo + bar), quos(foo = foo, bar = bar))
-  expect_identical(wrap_as_facets_list(foo ~ bar), quos(foo = foo, bar = bar))
-})
-
 test_that("as_facets_list() coerces strings containing formulas", {
   expect_identical(as_facets_list("foo ~ bar"), as_facets_list(local(foo ~ bar, globalenv())))
 })
@@ -46,6 +40,27 @@ test_that("as_facets_list() coerces lists", {
   expect_identical(out, exp)
 })
 
+test_that("as_facets_list() coerces quosures objectss", {
+  expect_identical(as_facets_list(vars(foo)), list(quos(foo = foo)))
+})
+
+test_that("facets reject aes()", {
+  expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
+  expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
+})
+
+test_that("wrap_as_facets_list() returns a quosures object with compacted", {
+  expect_identical(wrap_as_facets_list(vars(foo)), quos(foo = foo))
+  expect_identical(wrap_as_facets_list(~foo + bar), quos(foo = foo, bar = bar))
+  expect_identical(wrap_as_facets_list(vars(foo, NULL, bar)), quos(foo = foo, bar = bar))
+})
+
+test_that("grid_as_facets_list() returns a list of quosures objects with compacted", {
+  expect_identical(grid_as_facets_list(vars(foo), NULL), list(rows = quos(foo = foo), cols = quos()))
+  expect_identical(grid_as_facets_list(~foo, NULL), list(rows = quos(), cols = quos(foo = foo)))
+  expect_identical(grid_as_facets_list(vars(foo, NULL, bar), NULL), list(rows = quos(foo = foo, bar = bar), cols = quos()))
+})
+
 test_that("wrap_as_facets_list() and grid_as_facets_list() accept empty specs", {
   expect_identical(wrap_as_facets_list(NULL), quos())
   expect_identical(wrap_as_facets_list(list()), quos())
@@ -59,20 +74,6 @@ test_that("wrap_as_facets_list() and grid_as_facets_list() accept empty specs",
   expect_identical(grid_as_facets_list(list(NULL), NULL), list(rows = quos(), cols = quos()))
 })
 
-test_that("wrap_as_facets_list() and grid_as_facets_list() compact specs", {
-  expect_identical(wrap_as_facets_list(vars(foo, NULL, bar)), quos(foo = foo, bar = bar))
-  expect_identical(grid_as_facets_list(vars(foo, NULL, bar), NULL), list(rows = quos(foo = foo, bar = bar), cols = quos()))
-})
-
-test_that("as_facets_list() coerces quosure lists", {
-  expect_identical(as_facets_list(vars(foo)), list(rlang::quos(foo = foo)))
-})
-
-test_that("facets reject aes()", {
-  expect_error(facet_wrap(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
-  expect_error(facet_grid(aes(foo)), "Please use `vars()` to supply facet variables", fixed = TRUE)
-})
-
 df <- data_frame(x = 1:3, y = 3:1, z = letters[1:3])
 
 test_that("facets split up the data", {
@@ -135,6 +136,23 @@ test_that("vars() accepts optional names", {
   expect_named(wrap$params$facets, c("A", "b"))
 })
 
+test_that("facets_wrap() compacts the facet spec and accept empty spec", {
+  p <- ggplot(df, aes(x, y)) + geom_point() + facet_wrap(vars(NULL))
+  d <- layer_data(p)
+
+  expect_equal(d$PANEL, c(1L, 1L, 1L))
+  expect_equal(d$group, c(-1L, -1L, -1L))
+})
+
+test_that("facets_grid() compacts the facet spec and accept empty spec", {
+  p <- ggplot(df, aes(x, y)) + geom_point() + facet_grid(vars(NULL))
+  d <- layer_data(p)
+
+  expect_equal(d$PANEL, c(1L, 1L, 1L))
+  expect_equal(d$group, c(-1L, -1L, -1L))
+})
+
+
 test_that("facets with free scales scale independently", {
   l1 <- ggplot(df, aes(x, y)) + geom_point() +
     facet_wrap(~z, scales = "free")

From 7db3894cbd581a782c0783dadfd9586ffa23683b Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Thu, 7 Mar 2019 21:20:40 +0900
Subject: [PATCH 22/23] Add a NEWS bullet

---
 NEWS.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index ae3c268e9c..64c06c1348 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -70,6 +70,9 @@
 * Re-generate economics data. This leads to some changes in the values of all
   columns (especially in `psavert`), but more importantly, strips the grouping
   attributes from `economics_long`.
+  
+* `facet_wrap()` and `facet_grid()` now automatically remove NULL from facet
+  specs, and accept empty specs (@yutannihilation, #3070, #2986).
 
 # ggplot2 3.1.0
 

From d6f58a646e847a3d52c25433967f729d871b7e85 Mon Sep 17 00:00:00 2001
From: Hiroaki Yutani <yutani.ini@gmail.com>
Date: Tue, 19 Mar 2019 12:28:28 +0900
Subject: [PATCH 23/23] Use new_quosures() instead of as_quosures()

---
 R/facet-.r | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/R/facet-.r b/R/facet-.r
index 4a5adade99..4ccdcb3cd8 100644
--- a/R/facet-.r
+++ b/R/facet-.r
@@ -318,7 +318,7 @@ as_facets_list <- function(x) {
 compact_facets <- function(x) {
   x <- rlang::flatten_if(x, rlang::is_list)
   null <- vapply(x, rlang::quo_is_null, logical(1))
-  rlang::as_quosures(x[!null])
+  rlang::new_quosures(x[!null])
 }
 
 # Compatibility with plyr::as.quoted()