Skip to content

Commit

Permalink
exclude %<>% from one_call_pipe_linter (#2331)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Nov 21, 2023
1 parent 209e250 commit 05baf80
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 13 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* `nested_pipe_linter()` for discouraging pipes within pipes, e.g. `df1 %>% inner_join(df2 %>% select(a, b))` (part of #884, @MichaelChirico).
* `nrow_subset_linter()` for discouraging usage like `nrow(subset(x, conditions))` in favor of something like `with(x, sum(conditions))` which doesn't require a full subset of `x` (part of #884, @MichaelChirico).
* `pipe_return_linter()` for discouraging usage of `return()` inside a {magrittr} pipeline (part of #884, @MichaelChirico).
* `one_call_pipe_linter()` for discouraging one-step pipelines like `x |> as.character()` (#2330 and part of #884, @MichaelChirico).

### Lint accuracy fixes: removing false positives

Expand Down
9 changes: 8 additions & 1 deletion R/one_call_pipe_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,20 @@
#' linters = one_call_pipe_linter()
#' )
#'
#' # assignment pipe is exempted
#' lint(
#' text = "DF %<>% mutate(a = 2)",
#' linters = one_call_pipe_linter()
#' )
#'
#' @evalRd rd_tags("one_call_pipe_linter")
#' @seealso
#' - [linters] for a complete list of linters available in lintr.
#' - <https://style.tidyverse.org/pipes.html#short-pipes>
#' @export
one_call_pipe_linter <- function() {
pipes_cond <- xp_text_in_table(magrittr_pipes)
# exception for assignment pipe per #2330
pipes_cond <- xp_text_in_table(setdiff(magrittr_pipes, "%<>%"))

# preceding-sibling::SPECIAL: if there are ever two pipes, don't lint
# OP-LEFT-BRACKET/LBB: accept DT[...] %>% .[...] as a two-call pipe,
Expand Down
6 changes: 6 additions & 0 deletions man/one_call_pipe_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 8 additions & 12 deletions tests/testthat/test-one_call_pipe_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ test_that("one_call_pipe_linter skips allowed usages", {
expect_lint("foo(x) %>% bar()", NULL, linter)
# both calls in second step --> OK
expect_lint("x %>% foo(bar(.))", NULL, linter)

# assignment pipe is exempted
expect_lint("x %<>% as.character()", NULL, linter)
})

test_that("one_call_pipe_linter blocks simple disallowed usages", {
Expand Down Expand Up @@ -41,20 +44,13 @@ test_that("one_call_pipe_linter skips data.table chains", {

test_that("one_call_pipe_linter treats all pipes equally", {
linter <- one_call_pipe_linter()
lint_msg_part <- "Expressions with only a single call shouldn't use pipe "

expect_lint("foo %>% bar() %$% col", NULL, linter)
expect_lint("x %T>% foo()", rex::rex(lint_msg_part, "%T>%."), linter)
expect_lint("x %$%\n foo", rex::rex(lint_msg_part, "%$%."), linter)
expect_lint(
"x %T>% foo()",
rex::rex("Expressions with only a single call shouldn't use pipe %T>%."),
linter
)
expect_lint(
"x %$%\n foo()",
rex::rex("Expressions with only a single call shouldn't use pipe %$%."),
linter
)
expect_lint(
'data %>% filter(type == "console") %$% obscured_gaia_id %>% unique()',
'data %>% filter(type == "console") %$% obscured_id %>% unique()',
NULL,
linter
)
Expand All @@ -64,7 +60,7 @@ test_that("multiple lints are generated correctly", {
expect_lint(
trim_some("{
a %>% b()
c %$% d()
c %$% d
e %T>%
f()
}"),
Expand Down

0 comments on commit 05baf80

Please sign in to comment.