Fix behaviour of combine_vars() with more than one facet variable

[paleolimbot]

This PR fixes #2810, which describes the unusual failure of facet_wrap() when used with layers containing different numbers of facet variables.

library(ggplot2)

df_all_vars <- expand.grid(letter = c("a", "b"), number = c(1, 2), x = 1, y = 1)
df_some_vars <- data.frame(letter = c("a", "b"), x = 2, y = 2)

ggplot() +
  geom_point(aes(x, y), data = df_all_vars) +
  geom_point(aes(x, y), data = df_some_vars) +
  facet_wrap(vars(letter, number))
#> Error in gList(structure(list(x = structure(0.5, class = "unit", valid.unit = 0L, unit = "npc"), : only 'grobs' allowed in "gList"

This took a very long time to track down. In the end, FacetWrap$compute_layout() was generating a layout with 12 rows, when there are only four panels computed by FacetWrap$map_data(). This resulted in some NULL grobs due to index recycling, which are not valid but don't generate errors until render time (r-lib/gtable#80). This layout is computed by combine_vars(), which has incorrect handling when there are different numbers of facet variables in each layer's data.

library(ggplot2)
df <- expand.grid(letter = c("a", "b"), number = c(1, 2))
combine_vars(list(df, df["letter"]), vars = vars(letter = letter, number = number))
#>    letter number
#> 1       a      1
#> 2       b      1
#> 3       a      2
#> 4       b      2
#> 5       a      1
#> 6       a      1
#> 7       a      2
#> 8       a      2
#> 9       b      1
#> 10      b      1
#> 11      b      2
#> 12      b      2

This PR fixes this behaviour (by wrapping the line that generates too many combinations with unique()) and adds tests to verify the existing behaviour of combine_vars(). There some odd behaviour when using the drop = FALSE argument, notably that NAs are dropped (unless they are factor values), and that missing combinations of multiple facet variables are included (in addition to missing factor levels).

This function could probably be rewritten to be clearer, but I didn't do this because I didn't want to break the existing behaviour.

[hadley]

I admittedly don't remember how this is supposed to work, but the fix seems reasonable and the tests are comprehensive.

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/