Also lint nrow(filter(.)) in nrow_subset_linter()

NEWS.md

@@ -64,7 +64,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` (#2314 and 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` (#2313, #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

@@ -14,6 +14,16 @@
 #' linters = nrow_subset_linter()
 #' )
 #'
+#' lint(
+#' text = "nrow(filter(x, is_treatment))",
+#' linters = nrow_subset_linter()
+#' )
+#'
+#' lint(
+#' text = "x %>% filter(x, is_treatment) %>% nrow()",
+#' linters = nrow_subset_linter()
+#' )
+#'
 #' # okay
 #' lint(
 #' text = "with(x, sum(is_treatment, na.rm = TRUE))",
@@ -25,7 +35,7 @@
 #' @include shared_constants.R
 #' @export
 nrow_subset_linter <- make_linter_from_function_xpath(
- function_names = "subset",
+ function_names = c("subset", "filter"),
  xpath = glue("
  parent::expr
  /parent::expr

tests/testthat/test-nrow_subset_linter.R

@@ -13,6 +13,14 @@ test_that("nrow_subset_linter blocks subset() cases", {
  )
 })
 
+test_that("nrow_subset_linter blocks filter() cases", {
+ expect_lint(
+ "nrow(filter(x, y == z))",
+ rex::rex("Use arithmetic to count the number of rows satisfying a condition"),
+ nrow_subset_linter()
+ )
+})
+
 test_that("lints vectorize", {
  lint_msg <- rex::rex("Use arithmetic to count the number of rows satisfying a condition")
 
@@ -21,10 +29,12 @@ test_that("lints vectorize", {
  nrow(subset(x, y == z))
  subset(x) %>% transform(m = 2)
  nrow(subset(a, b == c))
+ x %>% filter(y == z) %>% nrow()
  }"),
  list(
  list(lint_msg, line_number = 2L),
- list(lint_msg, line_number = 4L)
+ list(lint_msg, line_number = 4L),
+ list(lint_msg, line_number = 5L)
  ),
  nrow_subset_linter()
  )
@@ -35,7 +45,7 @@ test_that("linter is pipeline-aware", {
  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)
+ expect_lint("filter(x, y == z) %>% nrow()", lint_msg, linter)
 
  skip_if_not_r_version("4.1.0")
  expect_lint("x |> subset(y == z) |> nrow()", lint_msg, linter)