From 9dc8feec3b2c48f1adc0fc8522d6606f9ec5700a Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 17 Nov 2023 18:03:52 +0000 Subject: [PATCH 1/6] New nrow_subset_linter --- DESCRIPTION | 1 + NAMESPACE | 1 + NEWS.md | 1 + R/nrow_subset_linter.R | 30 +++++++++++++ inst/lintr/linters.csv | 1 + man/best_practices_linters.Rd | 1 + man/consistency_linters.Rd | 1 + man/efficiency_linters.Rd | 1 + man/linters.Rd | 9 ++-- man/nrow_subset_linter.Rd | 24 ++++++++++ man/readability_linters.Rd | 1 + tests/testthat/test-nrow_subset_linter.R | 56 ++++++++++++++++++++++++ 12 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 R/nrow_subset_linter.R create mode 100644 man/nrow_subset_linter.Rd create mode 100644 tests/testthat/test-nrow_subset_linter.R diff --git a/DESCRIPTION b/DESCRIPTION index 7e0f6250d..3fa6ba621 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -138,6 +138,7 @@ Collate: 'namespace_linter.R' 'nested_ifelse_linter.R' 'nonportable_path_linter.R' + 'nrow_subset_linter.R' 'numeric_leading_zero_linter.R' 'object_length_linter.R' 'object_name_linter.R' diff --git a/NAMESPACE b/NAMESPACE index 8be33824d..6a75d2a45 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -100,6 +100,7 @@ export(namespace_linter) export(nested_ifelse_linter) export(no_tab_linter) export(nonportable_path_linter) +export(nrow_subset_linter) export(numeric_leading_zero_linter) export(object_length_linter) export(object_name_linter) diff --git a/NEWS.md b/NEWS.md index 477b8e61c..ecffcebbb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -15,6 +15,7 @@ * `comparison_negation_linter()` for discouraging negated comparisons when a direct negation is preferable, e.g. `!(x == y)` could be `x != y` (part of #884, @MichaelChirico). * `terminal_close_linter()` for discouraging using `close()` to end functions (part of #884, @MichaelChirico). Such usages are not robust to errors, where `close()` will not be run as intended. Put `close()` in an `on.exit()` hook, or use {withr} to manage connections with proper cleanup. * `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @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). ### Lint accuracy fixes: removing false positives diff --git a/R/nrow_subset_linter.R b/R/nrow_subset_linter.R new file mode 100644 index 000000000..53f550698 --- /dev/null +++ b/R/nrow_subset_linter.R @@ -0,0 +1,30 @@ +#' Block usage of `nrow(subset(x, .))` +#' +#' Using `nrow(subset(x, condition))` to count the instances where `condition` +#' applies inefficiently requires doing a full subset of `x` just to +#' count the number of rows in the resulting subset. +#' There are a number of equivalent expressions that don't require the full +#' subset, e.g. `with(x, sum(condition))` (or, more generically, +#' `with(x, sum(condition, na.rm = TRUE))`). +#' The same can be said of other versions of this like +#' `nrow(DT[(condition)])` for subsetting a `data.table` or +# " `DT %>% filter(condition) %>% nrow()`. +#' +#' @evalRd rd_tags("nrow_subset_linter") +#' @seealso [linters] for a complete list of linters available in lintr. +#' @export +nrow_subset_linter <- make_linter_from_xpath( + xpath = " + //SYMBOL_FUNCTION_CALL[text() = 'subset'] + /parent::expr + /parent::expr + /parent::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 table 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." + ) +) diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index 334f6167d..5e0f55f07 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -57,6 +57,7 @@ namespace_linter,correctness robustness configurable executing nested_ifelse_linter,efficiency readability no_tab_linter,style consistency deprecated nonportable_path_linter,robustness best_practices configurable +nrow_subset_linter,efficiency consistency readability best_practices numeric_leading_zero_linter,style consistency readability object_length_linter,style readability default configurable executing object_name_linter,style consistency default configurable executing diff --git a/man/best_practices_linters.Rd b/man/best_practices_linters.Rd index 4e8cc2ead..819cd1958 100644 --- a/man/best_practices_linters.Rd +++ b/man/best_practices_linters.Rd @@ -45,6 +45,7 @@ The following linters are tagged with 'best_practices': \item{\code{\link{library_call_linter}}} \item{\code{\link{literal_coercion_linter}}} \item{\code{\link{nonportable_path_linter}}} +\item{\code{\link{nrow_subset_linter}}} \item{\code{\link{outer_negation_linter}}} \item{\code{\link{paste_linter}}} \item{\code{\link{print_linter}}} diff --git a/man/consistency_linters.Rd b/man/consistency_linters.Rd index 7d8a609c6..e7f0a0873 100644 --- a/man/consistency_linters.Rd +++ b/man/consistency_linters.Rd @@ -26,6 +26,7 @@ The following linters are tagged with 'consistency': \item{\code{\link{keyword_quote_linter}}} \item{\code{\link{length_levels_linter}}} \item{\code{\link{literal_coercion_linter}}} +\item{\code{\link{nrow_subset_linter}}} \item{\code{\link{numeric_leading_zero_linter}}} \item{\code{\link{object_name_linter}}} \item{\code{\link{paste_linter}}} diff --git a/man/efficiency_linters.Rd b/man/efficiency_linters.Rd index 146c0be54..0f91d8585 100644 --- a/man/efficiency_linters.Rd +++ b/man/efficiency_linters.Rd @@ -23,6 +23,7 @@ The following linters are tagged with 'efficiency': \item{\code{\link{literal_coercion_linter}}} \item{\code{\link{matrix_apply_linter}}} \item{\code{\link{nested_ifelse_linter}}} +\item{\code{\link{nrow_subset_linter}}} \item{\code{\link{outer_negation_linter}}} \item{\code{\link{redundant_equals_linter}}} \item{\code{\link{redundant_ifelse_linter}}} diff --git a/man/linters.Rd b/man/linters.Rd index 91d035bb0..286dae126 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -17,18 +17,18 @@ see also \code{\link[=available_tags]{available_tags()}}. \section{Tags}{ The following tags exist: \itemize{ -\item{\link[=best_practices_linters]{best_practices} (56 linters)} +\item{\link[=best_practices_linters]{best_practices} (57 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (8 linters)} \item{\link[=configurable_linters]{configurable} (34 linters)} -\item{\link[=consistency_linters]{consistency} (24 linters)} +\item{\link[=consistency_linters]{consistency} (25 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} \item{\link[=default_linters]{default} (25 linters)} \item{\link[=deprecated_linters]{deprecated} (8 linters)} -\item{\link[=efficiency_linters]{efficiency} (26 linters)} +\item{\link[=efficiency_linters]{efficiency} (27 linters)} \item{\link[=executing_linters]{executing} (5 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=pkg_testthat_linters]{pkg_testthat} (12 linters)} -\item{\link[=readability_linters]{readability} (57 linters)} +\item{\link[=readability_linters]{readability} (58 linters)} \item{\link[=robustness_linters]{robustness} (16 linters)} \item{\link[=style_linters]{style} (38 linters)} } @@ -91,6 +91,7 @@ The following linters exist: \item{\code{\link{namespace_linter}} (tags: configurable, correctness, executing, robustness)} \item{\code{\link{nested_ifelse_linter}} (tags: efficiency, readability)} \item{\code{\link{nonportable_path_linter}} (tags: best_practices, configurable, robustness)} +\item{\code{\link{nrow_subset_linter}} (tags: best_practices, consistency, efficiency, readability)} \item{\code{\link{numeric_leading_zero_linter}} (tags: consistency, readability, style)} \item{\code{\link{object_length_linter}} (tags: configurable, default, executing, readability, style)} \item{\code{\link{object_name_linter}} (tags: configurable, consistency, default, executing, style)} diff --git a/man/nrow_subset_linter.Rd b/man/nrow_subset_linter.Rd new file mode 100644 index 000000000..3d0f8ab3d --- /dev/null +++ b/man/nrow_subset_linter.Rd @@ -0,0 +1,24 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/nrow_subset_linter.R +\name{nrow_subset_linter} +\alias{nrow_subset_linter} +\title{Block usage of \code{nrow(subset(x, .))}} +\usage{ +nrow_subset_linter() +} +\description{ +Using \code{nrow(subset(x, condition))} to count the instances where \code{condition} +applies inefficiently requires doing a full subset of \code{x} just to +count the number of rows in the resulting subset. +There are a number of equivalent expressions that don't require the full +subset, e.g. \code{with(x, sum(condition))} (or, more generically, +\code{with(x, sum(condition, na.rm = TRUE))}). +The same can be said of other versions of this like +\code{nrow(DT[(condition)])} for subsetting a \code{data.table} or +} +\seealso{ +\link{linters} for a complete list of linters available in lintr. +} +\section{Tags}{ +\link[=best_practices_linters]{best_practices}, \link[=consistency_linters]{consistency}, \link[=efficiency_linters]{efficiency}, \link[=readability_linters]{readability} +} diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index 06deb9233..cf86a179e 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -42,6 +42,7 @@ The following linters are tagged with 'readability': \item{\code{\link{line_length_linter}}} \item{\code{\link{matrix_apply_linter}}} \item{\code{\link{nested_ifelse_linter}}} +\item{\code{\link{nrow_subset_linter}}} \item{\code{\link{numeric_leading_zero_linter}}} \item{\code{\link{object_length_linter}}} \item{\code{\link{object_usage_linter}}} diff --git a/tests/testthat/test-nrow_subset_linter.R b/tests/testthat/test-nrow_subset_linter.R new file mode 100644 index 000000000..cbf92f667 --- /dev/null +++ b/tests/testthat/test-nrow_subset_linter.R @@ -0,0 +1,56 @@ +# TODO(michaelchirico): activate this false positive test when below cases are done. +# test_that("nrow_subset_linter skips allowed usages", { +# # nrow can be avoided here (by chaining the expression and using .N), +# # but the benefit of doing so is not the same as in the other cases. +# lintr::expect_lint( +# "nrow(DT[x == y, 1, by = grp])", +# NULL, +# nrow_subset_linter +# ) +# }) + +test_that("nrow_subset_linter blocks subset() cases", { + expect_lint( + "nrow(subset(x, y == z))", + rex::rex("Use arithmetic to count the number of rows satisfying a condition"), + nrow_subset_linter() + ) + + # TODO(michaelchirico): implement this. + # lintr::expect_lint( + # "x %>% subset(y == z) %>% nrow()", + # "Use arithmetic to count the number of rows satisfying a condition", + # nrow_subset_linter + # ) +}) + +# TODO(michaelchirico): implement these. +# test_that("nrow_subset_linter blocks [ cases", { +# # data.frame subsetting (NB: replacement doesn't use na.rm = TRUE) +# lintr::expect_lint( +# "nrow(x[x$y == x$z, ])", +# "Use arithmetic to count the number of rows satisfying a condition", +# nrow_subset_linter +# ) + +# # data.table subsetting (NB: replacement needs na.rm = TRUE) +# lintr::expect_lint( +# "x[y == z, ]", +# "Use arithmetic to count the number of rows satisfying a condition", +# nrow_subset_linter +# ) +# }) + +# test_that("nrow_subset_linter blocks dplyr::filter() cases", { +# lintr::expect_lint( +# "x %>% filter(y == z) %>% nrow()", +# "Use arithmetic to count the number of rows satisfying a condition", +# nrow_subset_linter +# ) + +# lintr::expect_lint( +# "nrow(dplyr::filter(x, y == z))", +# "Use arithmetic to count the number of rows satisfying a condition", +# nrow_subset_linter +# ) +# }) From 8540655fbd893c02949451e501f8ad1d4f70a703 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Fri, 17 Nov 2023 18:56:53 +0000 Subject: [PATCH 2/6] examples --- R/nrow_subset_linter.R | 15 ++++++++++++++- man/nrow_subset_linter.Rd | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/R/nrow_subset_linter.R b/R/nrow_subset_linter.R index 53f550698..c6e8f9a77 100644 --- a/R/nrow_subset_linter.R +++ b/R/nrow_subset_linter.R @@ -8,7 +8,20 @@ #' `with(x, sum(condition, na.rm = TRUE))`). #' The same can be said of other versions of this like #' `nrow(DT[(condition)])` for subsetting a `data.table` or -# " `DT %>% filter(condition) %>% nrow()`. +#' `DT %>% filter(condition) %>% nrow()`. +#' +#' @examples +#' # will produce lints +#' lint( +#' text = "nrow(subset(x, is_treatment))", +#' linters = nrow_subset_linter() +#' ) +#' +#' # okay +#' lint( +#' text = "with(x, sum(is_treatment, na.rm = TRUE))", +#' linters = nrow_subset_linter() +#' ) #' #' @evalRd rd_tags("nrow_subset_linter") #' @seealso [linters] for a complete list of linters available in lintr. diff --git a/man/nrow_subset_linter.Rd b/man/nrow_subset_linter.Rd index 3d0f8ab3d..c2b1112d7 100644 --- a/man/nrow_subset_linter.Rd +++ b/man/nrow_subset_linter.Rd @@ -15,6 +15,21 @@ subset, e.g. \code{with(x, sum(condition))} (or, more generically, \code{with(x, sum(condition, na.rm = TRUE))}). The same can be said of other versions of this like \code{nrow(DT[(condition)])} for subsetting a \code{data.table} or +\code{DT \%>\% filter(condition) \%>\% nrow()}. +} +\examples{ +# will produce lints +lint( + text = "nrow(subset(x, is_treatment))", + linters = nrow_subset_linter() +) + +# okay +lint( + text = "with(x, sum(is_treatment, na.rm = TRUE))", + linters = nrow_subset_linter() +) + } \seealso{ \link{linters} for a complete list of linters available in lintr. From a44e73712500e6eb5ea77e47bf9a493e5b165626 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 18 Nov 2023 11:28:51 -0800 Subject: [PATCH 3/6] remove commented code --- tests/testthat/test-nrow_subset_linter.R | 49 ------------------------ 1 file changed, 49 deletions(-) diff --git a/tests/testthat/test-nrow_subset_linter.R b/tests/testthat/test-nrow_subset_linter.R index cbf92f667..a67451220 100644 --- a/tests/testthat/test-nrow_subset_linter.R +++ b/tests/testthat/test-nrow_subset_linter.R @@ -1,56 +1,7 @@ -# TODO(michaelchirico): activate this false positive test when below cases are done. -# test_that("nrow_subset_linter skips allowed usages", { -# # nrow can be avoided here (by chaining the expression and using .N), -# # but the benefit of doing so is not the same as in the other cases. -# lintr::expect_lint( -# "nrow(DT[x == y, 1, by = grp])", -# NULL, -# nrow_subset_linter -# ) -# }) - test_that("nrow_subset_linter blocks subset() cases", { expect_lint( "nrow(subset(x, y == z))", rex::rex("Use arithmetic to count the number of rows satisfying a condition"), nrow_subset_linter() ) - - # TODO(michaelchirico): implement this. - # lintr::expect_lint( - # "x %>% subset(y == z) %>% nrow()", - # "Use arithmetic to count the number of rows satisfying a condition", - # nrow_subset_linter - # ) }) - -# TODO(michaelchirico): implement these. -# test_that("nrow_subset_linter blocks [ cases", { -# # data.frame subsetting (NB: replacement doesn't use na.rm = TRUE) -# lintr::expect_lint( -# "nrow(x[x$y == x$z, ])", -# "Use arithmetic to count the number of rows satisfying a condition", -# nrow_subset_linter -# ) - -# # data.table subsetting (NB: replacement needs na.rm = TRUE) -# lintr::expect_lint( -# "x[y == z, ]", -# "Use arithmetic to count the number of rows satisfying a condition", -# nrow_subset_linter -# ) -# }) - -# test_that("nrow_subset_linter blocks dplyr::filter() cases", { -# lintr::expect_lint( -# "x %>% filter(y == z) %>% nrow()", -# "Use arithmetic to count the number of rows satisfying a condition", -# nrow_subset_linter -# ) - -# lintr::expect_lint( -# "nrow(dplyr::filter(x, y == z))", -# "Use arithmetic to count the number of rows satisfying a condition", -# nrow_subset_linter -# ) -# }) From afc20d675eee1251a326c027d69dee93ccc1836a Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 18 Nov 2023 12:04:38 -0800 Subject: [PATCH 4/6] documentation feedback --- R/nrow_subset_linter.R | 5 +---- man/nrow_subset_linter.Rd | 3 --- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/R/nrow_subset_linter.R b/R/nrow_subset_linter.R index c6e8f9a77..eb731e579 100644 --- a/R/nrow_subset_linter.R +++ b/R/nrow_subset_linter.R @@ -6,9 +6,6 @@ #' There are a number of equivalent expressions that don't require the full #' subset, e.g. `with(x, sum(condition))` (or, more generically, #' `with(x, sum(condition, na.rm = TRUE))`). -#' The same can be said of other versions of this like -#' `nrow(DT[(condition)])` for subsetting a `data.table` or -#' `DT %>% filter(condition) %>% nrow()`. #' #' @examples #' # will produce lints @@ -35,7 +32,7 @@ nrow_subset_linter <- make_linter_from_xpath( ", lint_message = paste( "Use arithmetic to count the number of rows satisfying a condition,", - "rather than fully subsetting the table and counting the resulting rows.", + "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." diff --git a/man/nrow_subset_linter.Rd b/man/nrow_subset_linter.Rd index c2b1112d7..4411f777b 100644 --- a/man/nrow_subset_linter.Rd +++ b/man/nrow_subset_linter.Rd @@ -13,9 +13,6 @@ count the number of rows in the resulting subset. There are a number of equivalent expressions that don't require the full subset, e.g. \code{with(x, sum(condition))} (or, more generically, \code{with(x, sum(condition, na.rm = TRUE))}). -The same can be said of other versions of this like -\code{nrow(DT[(condition)])} for subsetting a \code{data.table} or -\code{DT \%>\% filter(condition) \%>\% nrow()}. } \examples{ # will produce lints From a566053d119428a982f9e3b94b62ce231a106637 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 18 Nov 2023 12:06:41 -0800 Subject: [PATCH 5/6] remove 'readability' tag --- inst/lintr/linters.csv | 2 +- man/linters.Rd | 4 ++-- man/nrow_subset_linter.Rd | 2 +- man/readability_linters.Rd | 1 - 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index d03373317..2c40fbd8a 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -57,7 +57,7 @@ namespace_linter,correctness robustness configurable executing nested_ifelse_linter,efficiency readability no_tab_linter,style consistency deprecated nonportable_path_linter,robustness best_practices configurable -nrow_subset_linter,efficiency consistency readability best_practices +nrow_subset_linter,efficiency consistency best_practices numeric_leading_zero_linter,style consistency readability object_length_linter,style readability default configurable executing object_name_linter,style consistency default configurable executing diff --git a/man/linters.Rd b/man/linters.Rd index 8e9042524..ecaa9bc97 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -28,7 +28,7 @@ The following tags exist: \item{\link[=executing_linters]{executing} (5 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=pkg_testthat_linters]{pkg_testthat} (12 linters)} -\item{\link[=readability_linters]{readability} (58 linters)} +\item{\link[=readability_linters]{readability} (57 linters)} \item{\link[=robustness_linters]{robustness} (16 linters)} \item{\link[=style_linters]{style} (38 linters)} } @@ -91,7 +91,7 @@ The following linters exist: \item{\code{\link{namespace_linter}} (tags: configurable, correctness, executing, robustness)} \item{\code{\link{nested_ifelse_linter}} (tags: efficiency, readability)} \item{\code{\link{nonportable_path_linter}} (tags: best_practices, configurable, robustness)} -\item{\code{\link{nrow_subset_linter}} (tags: best_practices, consistency, efficiency, readability)} +\item{\code{\link{nrow_subset_linter}} (tags: best_practices, consistency, efficiency)} \item{\code{\link{numeric_leading_zero_linter}} (tags: consistency, readability, style)} \item{\code{\link{object_length_linter}} (tags: configurable, default, executing, readability, style)} \item{\code{\link{object_name_linter}} (tags: configurable, consistency, default, executing, style)} diff --git a/man/nrow_subset_linter.Rd b/man/nrow_subset_linter.Rd index 4411f777b..3e2edcb2a 100644 --- a/man/nrow_subset_linter.Rd +++ b/man/nrow_subset_linter.Rd @@ -32,5 +32,5 @@ lint( \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=best_practices_linters]{best_practices}, \link[=consistency_linters]{consistency}, \link[=efficiency_linters]{efficiency}, \link[=readability_linters]{readability} +\link[=best_practices_linters]{best_practices}, \link[=consistency_linters]{consistency}, \link[=efficiency_linters]{efficiency} } diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index cf86a179e..06deb9233 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -42,7 +42,6 @@ The following linters are tagged with 'readability': \item{\code{\link{line_length_linter}}} \item{\code{\link{matrix_apply_linter}}} \item{\code{\link{nested_ifelse_linter}}} -\item{\code{\link{nrow_subset_linter}}} \item{\code{\link{numeric_leading_zero_linter}}} \item{\code{\link{object_length_linter}}} \item{\code{\link{object_usage_linter}}} From 0d589beaf9b783ee7226737ef16dae851e7bc8a4 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 19 Nov 2023 11:32:22 -0800 Subject: [PATCH 6/6] more tests --- tests/testthat/test-nrow_subset_linter.R | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/testthat/test-nrow_subset_linter.R b/tests/testthat/test-nrow_subset_linter.R index a67451220..18a3e45d8 100644 --- a/tests/testthat/test-nrow_subset_linter.R +++ b/tests/testthat/test-nrow_subset_linter.R @@ -1,3 +1,10 @@ +test_that("nrow_subset_linter skips allowed usage", { + linter <- nrow_subset_linter() + + expect_lint("nrow(foo(subset(x, y == z)))", NULL, linter) + expect_lint("with(x, sum(y == z))", NULL, linter) +}) + test_that("nrow_subset_linter blocks subset() cases", { expect_lint( "nrow(subset(x, y == z))", @@ -5,3 +12,20 @@ test_that("nrow_subset_linter blocks subset() cases", { nrow_subset_linter() ) }) + +test_that("lints vectorize", { + lint_msg <- rex::rex("Use arithmetic to count the number of rows satisfying a condition") + + expect_lint( + trim_some("{ + nrow(subset(x, y == z)) + subset(x) %>% transform(m = 2) + nrow(subset(a, b == c)) + }"), + list( + list(lint_msg, line_number = 2L), + list(lint_msg, line_number = 4L) + ), + nrow_subset_linter() + ) +})