Pipeline support in nrow_subset_linter()

DESCRIPTION

@@ -141,6 +141,7 @@ Collate:
  'nested_ifelse_linter.R'
  'nested_pipe_linter.R'
  'nonportable_path_linter.R'
+ 'shared_constants.R'
  'nrow_subset_linter.R'
  'numeric_leading_zero_linter.R'
  'nzchar_linter.R'
@@ -173,7 +174,6 @@ Collate:
  'seq_linter.R'
  'settings.R'
  'settings_utils.R'
- 'shared_constants.R'
  'sort_linter.R'
  'source_utils.R'
  'spaces_inside_linter.R'

NEWS.md

@@ -59,7 +59,7 @@
 * `consecutive_mutate_linter()` for encouraging consecutive calls to `dplyr::mutate()` to be combined (part of #884, @MichaelChirico).
 * `if_switch_linter()` for encouraging `switch()` over repeated `if`/`else` tests (part of #884, @MichaelChirico).
 * `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).
+* `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` (#2314 and 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).
 * `object_overwrite_linter()` for discouraging re-use of upstream package exports as local variables (#2344, #2346 and part of #884, @MichaelChirico and @AshesITR).

R/nrow_subset_linter.R

@@ -22,19 +22,25 @@
 #'
 #' @evalRd rd_tags("nrow_subset_linter")
 #' @seealso [linters] for a complete list of linters available in lintr.
+#' @include shared_constants.R
 #' @export
 nrow_subset_linter <- make_linter_from_function_xpath(
  function_names = "subset",
- xpath = "
+ xpath = glue("
  parent::expr
  /parent::expr
- /parent::expr[expr/SYMBOL_FUNCTION_CALL[text() = 'nrow']]
- ",
+ /parent::expr[
+ expr/SYMBOL_FUNCTION_CALL[text() = 'nrow']
+ or (self::expr | parent::expr)[
+ (PIPE or SPECIAL[{ xp_text_in_table(setdiff(magrittr_pipes, c('%$%', '%<>%'))) }])
+ and expr/expr/SYMBOL_FUNCTION_CALL[text() = 'nrow']
+ ]
+ ]
+ "),
  lint_message = paste(
  "Use arithmetic to count the number of rows satisfying a condition,",
  "rather than fully subsetting the data.frame and counting the resulting rows.",
- "For example, replace nrow(subset(x, is_treatment))",
- "with sum(x$is_treatment). NB: use na.rm = TRUE if `is_treatment` has",
- "missing values."
+ "For example, replace nrow(subset(x, is_treatment)) with sum(x$is_treatment).",
+ "NB: use na.rm = TRUE if `is_treatment` has missing values."
  )
 )

tests/testthat/test-nrow_subset_linter.R

@@ -29,3 +29,14 @@ test_that("lints vectorize", {
  nrow_subset_linter()
  )
 })
+
+test_that("linter is pipeline-aware", {
+ linter <- nrow_subset_linter()
+ lint_msg <- "Use arithmetic to count the number of rows satisfying a condition"
+
+ expect_lint("x %>% subset(y == z) %>% nrow()", lint_msg, linter)
+ expect_lint("subset(x) %>% nrow()", lint_msg, linter)
+
+ skip_if_not_r_version("4.1.0")
+ expect_lint("x |> subset(y == z) |> nrow()", lint_msg, linter)
+})