diff --git a/NEWS.md b/NEWS.md index ce17cd42b0..88ac3aff11 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # dplyr (development version) +* `join_by()` now allows its helper functions to be namespaced with `dplyr::`, + like `join_by(dplyr::between(x, lower, upper))` (#6838). + * `dplyr_reconstruct()`'s default method has been rewritten to avoid materializing duckplyr queries too early (#6947). diff --git a/R/join-by.R b/R/join-by.R index d4a1c395ee..d02ff7035f 100644 --- a/R/join-by.R +++ b/R/join-by.R @@ -431,7 +431,22 @@ parse_join_by_expr <- function(expr, i, error_call) { abort(message, call = error_call) } - op <- as_string(expr[[1]]) + if (is_call(expr, ns = "dplyr")) { + # Normalize by removing the `dplyr::` + expr[[1]] <- sym(call_name(expr)) + } + + op <- expr[[1]] + + if (!is_symbol(op)) { + if (is_call(op, name = "::")) { + stop_invalid_namespaced_expression(expr, i, error_call) + } else { + stop_invalid_top_expression(expr, i, error_call) + } + } + + op <- as_string(op) switch( op, @@ -476,6 +491,15 @@ stop_invalid_top_expression <- function(expr, i, call) { abort(message, call = call) } +stop_invalid_namespaced_expression <- function(expr, i, call) { + message <- c( + glue("Expressions can only be namespace prefixed with `dplyr::`."), + i = glue("Expression {i} is {err_expr(expr)}.") + ) + + abort(message, call = call) +} + parse_join_by_name <- function(expr, i, default_side, diff --git a/tests/testthat/_snaps/join-by.md b/tests/testthat/_snaps/join-by.md index 5e8e5a5fed..0ae8873aad 100644 --- a/tests/testthat/_snaps/join-by.md +++ b/tests/testthat/_snaps/join-by.md @@ -16,6 +16,38 @@ ! Expressions using `==` can't contain missing arguments. x Argument `y` is missing. +# allows for namespaced helpers (#6838) + + Code + join_by(dplyr::between(x, left, right)) + Output + Join By: + - dplyr::between(x, left, right) + +--- + + Code + join_by(dplyr::within(xl, xu, yl, yu)) + Output + Join By: + - dplyr::within(xl, xu, yl, yu) + +--- + + Code + join_by(dplyr::overlaps(xl, xu, yl, yu)) + Output + Join By: + - dplyr::overlaps(xl, xu, yl, yu) + +--- + + Code + join_by(dplyr::closest(x < y)) + Output + Join By: + - dplyr::closest(x < y) + # has an informative print method Code @@ -125,6 +157,24 @@ ! Each element of `...` must be a single column name or a join by expression. x Element 1 is not a name and not an expression. +--- + + Code + join_by(1()) + Condition + Error in `join_by()`: + ! Expressions must use one of: `==`, `>=`, `>`, `<=`, `<`, `closest()`, `between()`, `overlaps()`, or `within()`. + i Expression 1 is `1()`. + +--- + + Code + join_by(dplyrr::between(x, left, right)) + Condition + Error in `join_by()`: + ! Expressions can only be namespace prefixed with `dplyr::`. + i Expression 1 is `dplyrr::between(x, left, right)`. + --- Code diff --git a/tests/testthat/test-join-by.R b/tests/testthat/test-join-by.R index afbcfacf8f..8ddb875661 100644 --- a/tests/testthat/test-join-by.R +++ b/tests/testthat/test-join-by.R @@ -236,6 +236,23 @@ test_that("nicely catches required missing arguments when wrapped", { expect_snapshot(error = TRUE, fn(a)) }) +test_that("allows for namespaced helpers (#6838)", { + # Captures namespaced expression for printing + expect_snapshot(join_by(dplyr::between(x, left, right))) + expect_snapshot(join_by(dplyr::within(xl, xu, yl, yu))) + expect_snapshot(join_by(dplyr::overlaps(xl, xu, yl, yu))) + expect_snapshot(join_by(dplyr::closest(x < y))) + + # Underlying values are otherwise the same as non-namespaced version + by <- join_by(dplyr::between(x, left, right)) + reference <- join_by(between(x, left, right)) + + expect_identical(by$condition, reference$condition) + expect_identical(by$filter, reference$filter) + expect_identical(by$x, reference$x) + expect_identical(by$y, reference$y) +}) + test_that("has an informative print method", { expect_snapshot(join_by(a, b)) expect_snapshot(join_by("a", "b")) @@ -266,6 +283,13 @@ test_that("has informative error messages", { # Garbage input expect_snapshot(error = TRUE, join_by(1)) + # Call with non-symbol first element + expect_snapshot(error = TRUE, join_by(1())) + + # Namespace prefixed helper with non-dplyr namespace + # (typo or re-export, which currently isn't allowed) + expect_snapshot(error = TRUE, join_by(dplyrr::between(x, left, right))) + # Top level usage of `$` expect_snapshot(error = TRUE, join_by(x$a))