Merge f999174 into 15a9578

R/lengths_linter.R

@@ -34,7 +34,8 @@ lengths_linter <- function() {
   loop_funs <- c("sapply", "vapply", "map_int", "map_dbl")
   xpath <- glue::glue("
   //SYMBOL_FUNCTION_CALL[ {xp_text_in_table(loop_funs)} ]
-    /parent::expr/parent::expr[expr[position() = 3 and SYMBOL[text() = 'length']]]
+    /parent::expr
+    /parent::expr[expr/SYMBOL[text() = 'length']]
   ")
 
   Linter(function(source_expression) {

tests/testthat/test-lengths_linter.R

@@ -1,32 +1,45 @@
 test_that("lengths_linter skips allowed usages", {
+  linter <- lengths_linter()
+
+  expect_lint("length(x)", NULL, linter)
+  expect_lint("function(x) length(x) + 1L", NULL, linter)
+  expect_lint("vapply(x, fun, integer(length(y)))", NULL, linter)
+  expect_lint("sapply(x, sqrt, simplify = length(x))", NULL, linter)
+
   # TODO(#1570): also throw a lint here, and for map(x, length)
-  expect_lint("lapply(x, length)", NULL, lengths_linter())
+  expect_lint("lapply(x, length)", NULL, linter)
 })
 
 test_that("lengths_linter blocks simple disallowed base usages", {
-  expect_lint(
-    "sapply(x, length)",
-    rex::rex("Use lengths() to find the length of each element in a list."),
-    lengths_linter()
-  )
-
-  expect_lint(
-    "vapply(x, length, integer(1L))",
-    rex::rex("Use lengths() to find the length of each element in a list."),
-    lengths_linter()
-  )
+  linter <- lengths_linter()
+  lint_msg <- rex::rex("Use lengths() to find the length of each element in a list.")
+
+  expect_lint("sapply(x, length)", lint_msg, linter)
+  expect_lint("sapply(x, FUN = length)", lint_msg, linter)
+  expect_lint("sapply(FUN = length, x)", lint_msg, linter)
+
+  expect_lint("vapply(x, length, integer(1L))", lint_msg, linter)
 })
 
 test_that("lengths_linter blocks simple disallowed purrr usages", {
-  expect_lint(
-    "purrr::map_dbl(x, length)",
-    rex::rex("Use lengths() to find the length of each element in a list."),
-    lengths_linter()
-  )
-
-  expect_lint(
-    "map_int(x, length)",
-    rex::rex("Use lengths() to find the length of each element in a list."),
-    lengths_linter()
-  )
+  linter <- lengths_linter()
+  lint_msg <- rex::rex("Use lengths() to find the length of each element in a list.")
+
+  expect_lint("purrr::map_dbl(x, length)", lint_msg, linter)
+  expect_lint("map_dbl(x, .f = length)", lint_msg, linter)
+  expect_lint("map_dbl(.f = length, x)", lint_msg, linter)
+  expect_lint("map_int(x, length)", lint_msg, linter)
+})
+
+test_that("lengths_linter blocks simple disallowed usages with pipes", {
+  skip_if_not_r_version("4.1.0")
+
+  linter <- lengths_linter()
+  lint_msg <- rex::rex("Use lengths() to find the length of each element in a list.")
+
+  expect_lint("x |> sapply(length)", lint_msg, linter)
+  expect_lint("x %>% sapply(length)", lint_msg, linter)
+
+  expect_lint("x |> map_int(length)", lint_msg, linter)
+  expect_lint("x %>% map_int(length)", lint_msg, linter)
 })