Skip to content

Commit

Permalink
desc() inside arrange() checks the number of arguments. (tidyverse#5929)
Browse files Browse the repository at this point in the history
  • Loading branch information
romainfrancois committed Jun 25, 2021
1 parent 91c66a3 commit a5fef45
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 5 deletions.
12 changes: 7 additions & 5 deletions R/arrange.R
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,13 @@ arrange_rows <- function(.data, dots) {
})

quosures <- map(dots, function(quosure) {
if (quo_is_call(quosure, "desc")) {
quosure <- new_quosure(
node_cadr(quo_get_expr(quosure)),
quo_get_env(quosure)
)
if (quo_is_call(quosure, "desc", ns = c("", "dplyr"))) {
expr <- quo_get_expr(quosure)
if (!has_length(expr, 2L)) {
abort("`desc()` expects exactly one argument.")
}

quosure <- new_quosure(node_cadr(expr), quo_get_env(quosure))
}
quosure
})
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/_snaps/arrange.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@
i `..1 = rep(x, 2)`.
i `..1` must be size 1, not 2.

# desc() inside arrange() checks the number of arguments (#5921)

Code
df <- data.frame(x = 1, y = 2)
arrange(df, desc(x, y))
Error <rlang_error>
`desc()` expects exactly one argument.

9 changes: 9 additions & 0 deletions tests/testthat/test-arrange.r
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,12 @@ test_that("arrange() preserves the call stack on error (#5308)", {

expect_true(some(stack, is_call, "foobar"))
})

test_that("desc() inside arrange() checks the number of arguments (#5921)", {
expect_snapshot(error = TRUE, {
df <- data.frame(x = 1, y = 2)

arrange(df, desc(x, y))
})
})

0 comments on commit a5fef45

Please sign in to comment.