From beee6eb08bce829e5e4c08328e3df773907bb2f0 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 26 Feb 2025 20:50:25 +0000 Subject: [PATCH 01/13] change default --- R/pipe_consistency_linter.R | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/R/pipe_consistency_linter.R b/R/pipe_consistency_linter.R index 89ee40952..08fabcef4 100644 --- a/R/pipe_consistency_linter.R +++ b/R/pipe_consistency_linter.R @@ -1,11 +1,11 @@ #' Pipe consistency linter #' -#' Check that pipe operators are used consistently by file, or optionally -#' specify one valid pipe operator. +#' Check that the recommended pipe operator is used, or more conservatively that +#' pipes are consistent by file. #' -#' @param pipe Which pipe operator is valid (either `"%>%"` or `"|>"`). By default -#' (`"auto"`), the linter has no preference but will check that each file uses -#' only one type of pipe operator. +#' @param pipe Which pipe operator is valid (either `"%>%"` or `"|>"`). The default +#' is to use the Guide-recommended native pipe (`|>`). Value `"auto"` will instead +#' only enforce consistency, i.e., that in any given file there is only one pipe. #' #' @examples #' # will produce lints @@ -21,18 +21,18 @@ #' #' # okay #' lint( -#' text = "1:3 %>% mean() %>% as.character()", +#' text = "1:3 |> mean() |> as.character()", #' linters = pipe_consistency_linter() #' ) #' #' lint( -#' text = "1:3 |> mean() |> as.character()", -#' linters = pipe_consistency_linter() +#' text = "1:3 %>% mean() %>% as.character()", +#' linters = pipe_consistency_linter("%>%") #' ) #' @evalRd rd_tags("pipe_consistency_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -pipe_consistency_linter <- function(pipe = c("auto", "%>%", "|>")) { +pipe_consistency_linter <- function(pipe = c("|>", "auto", "%>%")) { pipe <- match.arg(pipe) xpath_magrittr <- glue("//SPECIAL[{ xp_text_in_table(magrittr_pipes) }]") From 0e5424fa75da54baad735ae0cd73f7e0fe3838a8 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 26 Feb 2025 20:51:17 +0000 Subject: [PATCH 02/13] expect_no_lint --- tests/testthat/test-pipe_consistency_linter.R | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 4a236b156..d7be6c04e 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -2,18 +2,17 @@ test_that("pipe_consistency skips allowed usage", { skip_if_not_r_version("4.1.0") linter <- pipe_consistency_linter() - expect_lint("1:3 %>% mean() %>% as.character()", NULL, linter) - expect_lint("1:3 |> mean() |> as.character()", NULL, linter) + expect_no_lint("1:3 %>% mean() %>% as.character()", linter) + expect_no_lint("1:3 |> mean() |> as.character()", linter) # With no pipes - expect_lint("x <- 1:5", NULL, linter) + expect_no_lint("x <- 1:5", linter) # Across multiple lines - expect_lint( + expect_no_lint( trim_some(" 1:3 %>% mean() %>% as.character() "), - NULL, linter ) }) @@ -96,9 +95,8 @@ test_that("pipe_consistency_linter works with |> argument", { linter ) - expect_lint( + expect_no_lint( "1:3 |> mean() |> as.character()", - NULL, linter ) @@ -134,9 +132,8 @@ test_that("pipe_consistency_linter works with %>% argument", { linter ) - expect_lint( + expect_no_lint( "1:3 %>% mean() %>% as.character()", - NULL, linter ) @@ -156,7 +153,7 @@ test_that("pipe_consistency_linter works with other magrittr pipes", { linter <- pipe_consistency_linter() expected_message <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 1 instances of |>.") - expect_lint("1:3 %>% mean() %T% print()", NULL, linter) + expect_no_lint("1:3 %>% mean() %T% print()", linter) expect_lint( "1:3 |> mean() %T>% print()", list( From 37f461ec235c22c49d90f556c3a84fe1b1e9a9b1 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 26 Feb 2025 21:10:48 +0000 Subject: [PATCH 03/13] Use the native pipe by default --- NEWS.md | 5 + R/pipe_consistency_linter.R | 4 +- R/zzz.R | 1 + inst/lintr/linters.csv | 2 +- man/default_linters.Rd | 3 +- man/linters.Rd | 4 +- man/pipe_consistency_linter.Rd | 25 ++-- tests/testthat/default_linter_testcode.R | 1 + tests/testthat/test-pipe_consistency_linter.R | 107 ++++++++++++------ 9 files changed, 102 insertions(+), 50 deletions(-) diff --git a/NEWS.md b/NEWS.md index 218223120..c8f016318 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,11 +10,16 @@ + `linter=` argument of `Lint()`. + `with_defaults()`. + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`. +* The default for `pipe_consistency_linter()` is changed from `"auto"` (just be consistent) to `"|>"` (R native pipe required) to coincide with the same change in the Tidyverse Style Guide (#2707, @MichaelChirico). ## Bug fixes * `Lint()`, and thus all linters, ensures that the returned object's `message` attribute is consistently a simple character string (and not, for example, an object of class `"glue"`; #2740, @MichaelChirico). +## Changes to default linters + +* `pipe_consistency_linter()`, with its new default to enforce the native pipe `|>`, is now a default linter, since it corresponds directly to a rule in the Tidyverse Style Guide (#2707, @MichaelChirico). + ## New and improved features * `brace_linter()`' has a new argument `function_bodies` (default `"multi_line"`) which controls when to require function bodies to be wrapped in curly braces, with the options `"always"`, `"multi_line"` (only require curly braces when a function body spans multiple lines), `"not_inline"` (only require curly braces when a function body starts on a new line) and `"never"` (#1807, #2240, @salim-b). diff --git a/R/pipe_consistency_linter.R b/R/pipe_consistency_linter.R index 08fabcef4..c0bf83f44 100644 --- a/R/pipe_consistency_linter.R +++ b/R/pipe_consistency_linter.R @@ -30,7 +30,9 @@ #' linters = pipe_consistency_linter("%>%") #' ) #' @evalRd rd_tags("pipe_consistency_linter") -#' @seealso [linters] for a complete list of linters available in lintr. +#' @seealso +#' - [linters] for a complete list of linters available in lintr. +#' - #' @export pipe_consistency_linter <- function(pipe = c("|>", "auto", "%>%")) { pipe <- match.arg(pipe) diff --git a/R/zzz.R b/R/zzz.R index eb916cf4f..ea2903e19 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -26,6 +26,7 @@ default_linters <- modify_defaults( object_name_linter(), object_usage_linter(), paren_body_linter(), + pipe_consistency_linter(), pipe_continuation_linter(), quotes_linter(), return_linter(), diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index b8e0cf979..116a5bcdb 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -74,7 +74,7 @@ package_hooks_linter,style correctness package_development paren_body_linter,style readability default paste_linter,best_practices consistency configurable pipe_call_linter,style readability -pipe_consistency_linter,style readability configurable +pipe_consistency_linter,default style readability configurable pipe_continuation_linter,style readability default pipe_return_linter,best_practices common_mistakes print_linter,best_practices consistency diff --git a/man/default_linters.Rd b/man/default_linters.Rd index cdaa763c7..e226a1332 100644 --- a/man/default_linters.Rd +++ b/man/default_linters.Rd @@ -5,7 +5,7 @@ \alias{default_linters} \title{Default linters} \format{ -An object of class \code{list} of length 25. +An object of class \code{list} of length 26. } \usage{ default_linters @@ -38,6 +38,7 @@ The following linters are tagged with 'default': \item{\code{\link{object_name_linter}}} \item{\code{\link{object_usage_linter}}} \item{\code{\link{paren_body_linter}}} +\item{\code{\link{pipe_consistency_linter}}} \item{\code{\link{pipe_continuation_linter}}} \item{\code{\link{quotes_linter}}} \item{\code{\link{return_linter}}} diff --git a/man/linters.Rd b/man/linters.Rd index 4ec1cba87..7b110be1f 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -22,7 +22,7 @@ The following tags exist: \item{\link[=configurable_linters]{configurable} (44 linters)} \item{\link[=consistency_linters]{consistency} (32 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} -\item{\link[=default_linters]{default} (25 linters)} +\item{\link[=default_linters]{default} (26 linters)} \item{\link[=efficiency_linters]{efficiency} (29 linters)} \item{\link[=executing_linters]{executing} (6 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} @@ -109,7 +109,7 @@ The following linters exist: \item{\code{\link{paren_body_linter}} (tags: default, readability, style)} \item{\code{\link{paste_linter}} (tags: best_practices, configurable, consistency)} \item{\code{\link{pipe_call_linter}} (tags: readability, style)} -\item{\code{\link{pipe_consistency_linter}} (tags: configurable, readability, style)} +\item{\code{\link{pipe_consistency_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{pipe_continuation_linter}} (tags: default, readability, style)} \item{\code{\link{pipe_return_linter}} (tags: best_practices, common_mistakes)} \item{\code{\link{print_linter}} (tags: best_practices, consistency)} diff --git a/man/pipe_consistency_linter.Rd b/man/pipe_consistency_linter.Rd index ff53277a9..247a8dcb8 100644 --- a/man/pipe_consistency_linter.Rd +++ b/man/pipe_consistency_linter.Rd @@ -4,16 +4,16 @@ \alias{pipe_consistency_linter} \title{Pipe consistency linter} \usage{ -pipe_consistency_linter(pipe = c("auto", "\%>\%", "|>")) +pipe_consistency_linter(pipe = c("|>", "auto", "\%>\%")) } \arguments{ -\item{pipe}{Which pipe operator is valid (either \code{"\%>\%"} or \code{"|>"}). By default -(\code{"auto"}), the linter has no preference but will check that each file uses -only one type of pipe operator.} +\item{pipe}{Which pipe operator is valid (either \code{"\%>\%"} or \code{"|>"}). The default +is to use the Guide-recommended native pipe (\verb{|>}). Value \code{"auto"} will instead +only enforce consistency, i.e., that in any given file there is only one pipe.} } \description{ -Check that pipe operators are used consistently by file, or optionally -specify one valid pipe operator. +Check that the recommended pipe operator is used, or more conservatively that +pipes are consistent by file. } \examples{ # will produce lints @@ -29,18 +29,21 @@ lint( # okay lint( - text = "1:3 \%>\% mean() \%>\% as.character()", + text = "1:3 |> mean() |> as.character()", linters = pipe_consistency_linter() ) lint( - text = "1:3 |> mean() |> as.character()", - linters = pipe_consistency_linter() + text = "1:3 \%>\% mean() \%>\% as.character()", + linters = pipe_consistency_linter("\%>\%") ) } \seealso{ -\link{linters} for a complete list of linters available in lintr. +\itemize{ +\item \link{linters} for a complete list of linters available in lintr. +\item \url{https://style.tidyverse.org/pipes.html#magrittr} +} } \section{Tags}{ -\link[=configurable_linters]{configurable}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +\link[=configurable_linters]{configurable}, \link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/tests/testthat/default_linter_testcode.R b/tests/testthat/default_linter_testcode.R index 1cadc18cc..23c17c281 100644 --- a/tests/testthat/default_linter_testcode.R +++ b/tests/testthat/default_linter_testcode.R @@ -39,6 +39,7 @@ my_metric <- function(x) sum(x) + prod(x) # no_tab +# pipe_consistency # pipe_continuation # seq_linter # spaces_inside diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index d7be6c04e..40d26423e 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -2,41 +2,37 @@ test_that("pipe_consistency skips allowed usage", { skip_if_not_r_version("4.1.0") linter <- pipe_consistency_linter() - expect_no_lint("1:3 %>% mean() %>% as.character()", linter) expect_no_lint("1:3 |> mean() |> as.character()", linter) # With no pipes expect_no_lint("x <- 1:5", linter) # Across multiple lines expect_no_lint( trim_some(" - 1:3 %>% - mean() %>% + 1:3 |> + mean() |> as.character() "), linter ) }) -test_that("pipe_consistency lints inconsistent usage", { +test_that("pipe_consistency lints blocked usage", { skip_if_not_r_version("4.1.0") linter <- pipe_consistency_linter() - expected_msg <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 1 instances of |>.") + lint_message <- rex::rex("Use the |> pipe operator instead of the %>% pipe operator.") + + + expect_lint("1:3 %>% mean() %>% as.character()", list(lint_message, lint_message), linter) expect_lint( "1:3 |> mean() %>% as.character()", - list( - list(message = expected_msg, line_number = 1L, column_number = 5L), - list(message = expected_msg, line_number = 1L, column_number = 15L) - ), + list(lint_message, line_number = 1L, column_number = 15L), linter ) expect_lint( "1:3 %>% mean() |> as.character()", - list( - list(message = expected_msg, line_number = 1L, column_number = 5L), - list(message = expected_msg, line_number = 1L, column_number = 16L) - ), + list(lint_message, line_number = 1L, column_number = 5L), linter ) @@ -46,21 +42,13 @@ test_that("pipe_consistency lints inconsistent usage", { mean() |> as.character() "), - list( - list(message = expected_msg, line_number = 1L, column_number = 5L), - list(message = expected_msg, line_number = 2L, column_number = 10L) - ), + list(lint_message, line_number = 1L, column_number = 5L), linter ) - expected_msg_multi <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 2 instances of |>.") expect_lint( "1:3 |> sort() |> mean() %>% as.character()", - list( - list(message = expected_msg_multi, line_number = 1L, column_number = 5L), - list(message = expected_msg_multi, line_number = 1L, column_number = 15L), - list(message = expected_msg_multi, line_number = 1L, column_number = 25L) - ), + list(lint_message, line_number = 1L, column_number = 25L), linter ) }) @@ -70,7 +58,7 @@ test_that("pipe_consistency_linter works with |> argument", { skip_if_not_r_version("4.1.0") linter <- pipe_consistency_linter(pipe = "|>") - expected_message <- rex::rex("Use the |> pipe operator instead of the %>% pipe operator.") + lint_message <- rex::rex("Use the |> pipe operator instead of the %>% pipe operator.") expect_lint( trim_some(" @@ -79,8 +67,8 @@ test_that("pipe_consistency_linter works with |> argument", { as.character() "), list( - list(message = expected_message, line_number = 1L, column_number = 5L), - list(message = expected_message, line_number = 2L, column_number = 10L) + list(lint_message, line_number = 1L, column_number = 5L), + list(lint_message, line_number = 2L, column_number = 10L) ), linter ) @@ -91,7 +79,7 @@ test_that("pipe_consistency_linter works with |> argument", { mean() %>% as.character() "), - list(message = expected_message, line_number = 2L, column_number = 10L), + list(lint_message, line_number = 2L, column_number = 10L), linter ) @@ -106,7 +94,15 @@ test_that("pipe_consistency_linter works with |> argument", { mean() %>% as.character() "), - list(message = expected_message, line_number = 2L, column_number = 10L), + list(lint_message, line_number = 2L, column_number = 10L), + linter + ) + + expect_lint("1:3 %>% mean() %T% print()", lint_message, linter) + + expect_lint( + "1:3 |> mean() %T>% print()", + list(lint_message, line_number = 1L, column_number = 15L), linter ) }) @@ -148,17 +144,60 @@ test_that("pipe_consistency_linter works with %>% argument", { ) }) -test_that("pipe_consistency_linter works with other magrittr pipes", { - skip_if_not_r_version("4.1.0") - linter <- pipe_consistency_linter() - expected_message <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 1 instances of |>.") +test_that("simply enforcing a consistent style is supported", { + linter <- pipe_consistency_linter("auto") + lint_message <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 1 instances of |>.") + + expect_no_lint("1:3 %>% mean() %>% as.character()", linter) + + expect_lint( + "1:3 |> mean() %>% as.character()", + list( + list(lint_message, line_number = 1L, column_number = 5L), + list(lint_message, line_number = 1L, column_number = 15L) + ), + linter + ) + + expect_lint( + "1:3 %>% mean() |> as.character()", + list( + list(lint_message, line_number = 1L, column_number = 5L), + list(lint_message, line_number = 1L, column_number = 16L) + ), + linter + ) + + expect_lint( + trim_some(" + 1:3 %>% + mean() |> + as.character() + "), + list( + list(lint_message, line_number = 1L, column_number = 5L), + list(lint_message, line_number = 2L, column_number = 10L) + ), + linter + ) + + lint_message_multi <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 2 instances of |>.") + expect_lint( + "1:3 |> sort() |> mean() %>% as.character()", + list( + list(lint_message_multi, line_number = 1L, column_number = 5L), + list(lint_message_multi, line_number = 1L, column_number = 15L), + list(lint_message_multi, line_number = 1L, column_number = 25L) + ), + linter + ) expect_no_lint("1:3 %>% mean() %T% print()", linter) expect_lint( "1:3 |> mean() %T>% print()", list( - list(message = expected_message, line_number = 1L, column_number = 5L), - list(message = expected_message, line_number = 1L, column_number = 15L) + list(lint_message, line_number = 1L, column_number = 5L), + list(lint_message, line_number = 1L, column_number = 15L) ), linter ) From 0192120c0ff422bb9a0ed5b2b3a16cca17fda3b1 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Wed, 26 Feb 2025 21:19:58 +0000 Subject: [PATCH 04/13] skip on old R --- tests/testthat/test-pipe_consistency_linter.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 40d26423e..619f15e1a 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -145,6 +145,8 @@ test_that("pipe_consistency_linter works with %>% argument", { }) test_that("simply enforcing a consistent style is supported", { + skip_if_not_r_version("4.1.0") + linter <- pipe_consistency_linter("auto") lint_message <- rex::rex("Stick to one pipe operator; found 1 instances of %>% and 1 instances of |>.") From b3b6b5db02435d5dbd573d61e5f373fd5379afb0 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 08:47:01 -0800 Subject: [PATCH 05/13] improvements from review Co-authored-by: AshesITR --- tests/testthat/test-pipe_consistency_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 619f15e1a..721252da3 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -98,7 +98,7 @@ test_that("pipe_consistency_linter works with |> argument", { linter ) - expect_lint("1:3 %>% mean() %T% print()", lint_message, linter) + expect_lint("1:3 %>% mean() %T>% print()", lint_message, linter) expect_lint( "1:3 |> mean() %T>% print()", From 4caf143b8fa3b017e43891be7e21790792d57b75 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 08:48:22 -0800 Subject: [PATCH 06/13] clicked the wrong button Co-authored-by: AshesITR --- R/pipe_consistency_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/pipe_consistency_linter.R b/R/pipe_consistency_linter.R index c0bf83f44..b80c29b40 100644 --- a/R/pipe_consistency_linter.R +++ b/R/pipe_consistency_linter.R @@ -4,7 +4,7 @@ #' pipes are consistent by file. #' #' @param pipe Which pipe operator is valid (either `"%>%"` or `"|>"`). The default -#' is to use the Guide-recommended native pipe (`|>`). Value `"auto"` will instead +#' is the native pipe (`|>`). `"auto"` will instead #' only enforce consistency, i.e., that in any given file there is only one pipe. #' #' @examples From c3aa0d11df2f20f98f5a7f27967381df03ff12ee Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 16:48:50 +0000 Subject: [PATCH 07/13] document --- man/pipe_consistency_linter.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/pipe_consistency_linter.Rd b/man/pipe_consistency_linter.Rd index 247a8dcb8..6628977aa 100644 --- a/man/pipe_consistency_linter.Rd +++ b/man/pipe_consistency_linter.Rd @@ -8,7 +8,7 @@ pipe_consistency_linter(pipe = c("|>", "auto", "\%>\%")) } \arguments{ \item{pipe}{Which pipe operator is valid (either \code{"\%>\%"} or \code{"|>"}). The default -is to use the Guide-recommended native pipe (\verb{|>}). Value \code{"auto"} will instead +is the native pipe (\verb{|>}). \code{"auto"} will instead only enforce consistency, i.e., that in any given file there is only one pipe.} } \description{ From d272066760a512e7495e4666ff0751ef2da740fb Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 17:56:17 +0000 Subject: [PATCH 08/13] there are two lints now --- tests/testthat/test-pipe_consistency_linter.R | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 721252da3..19c216c2a 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -98,7 +98,11 @@ test_that("pipe_consistency_linter works with |> argument", { linter ) - expect_lint("1:3 %>% mean() %T>% print()", lint_message, linter) + expect_lint( + "1:3 %>% mean() %T>% print()", + list("%>%, "%T>%"), + linter + ) expect_lint( "1:3 |> mean() %T>% print()", From 1e02bb4d586f01663e1de15a19a71e8c990a50e3 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 10:03:32 -0800 Subject: [PATCH 09/13] missing " --- tests/testthat/test-pipe_consistency_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 19c216c2a..14e1faa73 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -100,7 +100,7 @@ test_that("pipe_consistency_linter works with |> argument", { expect_lint( "1:3 %>% mean() %T>% print()", - list("%>%, "%T>%"), + list("%>%", "%T>%"), linter ) From a10eed8995dd1446047ed8897425b744483ec221 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Thu, 27 Feb 2025 18:37:02 +0000 Subject: [PATCH 10/13] customize message to the pipe used --- R/pipe_consistency_linter.R | 3 ++- tests/testthat/test-pipe_consistency_linter.R | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/R/pipe_consistency_linter.R b/R/pipe_consistency_linter.R index b80c29b40..2e5c7bfb3 100644 --- a/R/pipe_consistency_linter.R +++ b/R/pipe_consistency_linter.R @@ -66,10 +66,11 @@ pipe_consistency_linter <- function(pipe = c("|>", "auto", "%>%")) { type = "style" ) } else if (pipe == "|>" && n_magrittr > 0L) { + lint_message <- sprintf("Use the |> pipe operator instead of the %s pipe operator.", xml_text(match_magrittr)) xml_nodes_to_lints( xml = match_magrittr, source_expression = source_expression, - lint_message = "Use the |> pipe operator instead of the %>% pipe operator.", + lint_message = lint_message, type = "style" ) } else { diff --git a/tests/testthat/test-pipe_consistency_linter.R b/tests/testthat/test-pipe_consistency_linter.R index 14e1faa73..a7c5a24d3 100644 --- a/tests/testthat/test-pipe_consistency_linter.R +++ b/tests/testthat/test-pipe_consistency_linter.R @@ -59,6 +59,7 @@ test_that("pipe_consistency_linter works with |> argument", { linter <- pipe_consistency_linter(pipe = "|>") lint_message <- rex::rex("Use the |> pipe operator instead of the %>% pipe operator.") + lint_message_tee <- rex::rex("Use the |> pipe operator instead of the %T>% pipe operator.") expect_lint( trim_some(" @@ -100,13 +101,13 @@ test_that("pipe_consistency_linter works with |> argument", { expect_lint( "1:3 %>% mean() %T>% print()", - list("%>%", "%T>%"), + list(lint_message, lint_message_tee), linter ) expect_lint( "1:3 |> mean() %T>% print()", - list(lint_message, line_number = 1L, column_number = 15L), + list(lint_message_tee, line_number = 1L, column_number = 15L), linter ) }) From b2108e88fff0eedfa66228fd824b15c79d353f03 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 11:09:11 -0800 Subject: [PATCH 11/13] clearer description of "auto" in NEWS Co-authored-by: AshesITR --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c8f016318..7dcee26d4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,7 +10,7 @@ + `linter=` argument of `Lint()`. + `with_defaults()`. + Linters `closed_curly_linter()`, `open_curly_linter()`, `paren_brace_linter()`, and `semicolon_terminator_linter()`. -* The default for `pipe_consistency_linter()` is changed from `"auto"` (just be consistent) to `"|>"` (R native pipe required) to coincide with the same change in the Tidyverse Style Guide (#2707, @MichaelChirico). +* The default for `pipe_consistency_linter()` is changed from `"auto"` (require one pipe style, either magrittr or native) to `"|>"` (R native pipe required) to coincide with the same change in the Tidyverse Style Guide (#2707, @MichaelChirico). ## Bug fixes From a728df7e8d950f1806fc6803631b419d4a7a65d8 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 11:09:29 -0800 Subject: [PATCH 12/13] reorder signature (to be re-documented next) Co-authored-by: AshesITR --- R/pipe_consistency_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/pipe_consistency_linter.R b/R/pipe_consistency_linter.R index 2e5c7bfb3..81df66bb9 100644 --- a/R/pipe_consistency_linter.R +++ b/R/pipe_consistency_linter.R @@ -34,7 +34,7 @@ #' - [linters] for a complete list of linters available in lintr. #' - #' @export -pipe_consistency_linter <- function(pipe = c("|>", "auto", "%>%")) { +pipe_consistency_linter <- function(pipe = c("|>", "%>%", "auto")) { pipe <- match.arg(pipe) xpath_magrittr <- glue("//SPECIAL[{ xp_text_in_table(magrittr_pipes) }]") From 1ba785eb144afbc55acb97e7617622106c0bea02 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 3 Mar 2025 19:10:23 +0000 Subject: [PATCH 13/13] document --- man/pipe_consistency_linter.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/pipe_consistency_linter.Rd b/man/pipe_consistency_linter.Rd index 6628977aa..9cb188a79 100644 --- a/man/pipe_consistency_linter.Rd +++ b/man/pipe_consistency_linter.Rd @@ -4,7 +4,7 @@ \alias{pipe_consistency_linter} \title{Pipe consistency linter} \usage{ -pipe_consistency_linter(pipe = c("|>", "auto", "\%>\%")) +pipe_consistency_linter(pipe = c("|>", "\%>\%", "auto")) } \arguments{ \item{pipe}{Which pipe operator is valid (either \code{"\%>\%"} or \code{"|>"}). The default