Skip to content

Commit

Permalink
Merge branch 'main' into ul-rhs-in
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Nov 20, 2023
2 parents 8519951 + 8414018 commit 550f937
Show file tree
Hide file tree
Showing 11 changed files with 357 additions and 3 deletions.
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ Collate:
'namespace.R'
'namespace_linter.R'
'nested_ifelse_linter.R'
'nested_pipe_linter.R'
'nonportable_path_linter.R'
'nrow_subset_linter.R'
'numeric_leading_zero_linter.R'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export(missing_package_linter)
export(modify_defaults)
export(namespace_linter)
export(nested_ifelse_linter)
export(nested_pipe_linter)
export(no_tab_linter)
export(nonportable_path_linter)
export(nrow_subset_linter)
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
* `which_grepl_linter()` for discouraging `which(grepl(ptn, x))` in favor of directly using `grep(ptn, x)` (part of #884, @MichaelChirico).
* `list_comparison_linter()` for discouraging comparisons on the output of `lapply()`, e.g. `lapply(x, sum) > 10` (part of #884, @MichaelChirico).
* `print_linter()` for discouraging usage of `print()` on string literals like `print("Reached here")` or `print(paste("Found", nrow(DF), "rows."))` (#1894, @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).
* `pipe_return_linter()` for discouraging usage of `return()` inside a {magrittr} pipeline (part of #884, @MichaelChirico).

Expand Down
86 changes: 86 additions & 0 deletions R/nested_pipe_linter.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#' Block usage of pipes nested inside other calls
#'
#' Nesting pipes harms readability; extract sub-steps to separate variables,
#' append further pipeline steps, or otherwise refactor such usage away.
#'
#' @param allow_inline Logical, default `TRUE`, in which case only "inner"
#' pipelines which span more than one line are linted. If `FALSE`, even
#' "inner" pipelines that fit in one line are linted.
#' @param allow_outer_calls Character vector dictating which "outer"
#' calls to exempt from the requirement to unnest (see examples). Defaults
#' to [try()], [tryCatch()], and [withCallingHandlers()].
#'
#' @examples
#' # will produce lints
#' code <- "df1 %>%\n inner_join(df2 %>%\n select(a, b)\n )"
#' writeLines(code)
#' lint(
#' text = code,
#' linters = nested_pipe_linter()
#' )
#'
#' lint(
#' text = "df1 %>% inner_join(df2 %>% select(a, b))",
#' linters = nested_pipe_linter(allow_inline = FALSE)
#' )
#'
#' lint(
#' text = "tryCatch(x %>% filter(grp == 'a'), error = identity)",
#' linters = nested_pipe_linter(allow_outer_calls = character())
#' )
#'
#' # okay
#' lint(
#' text = "df1 %>% inner_join(df2 %>% select(a, b))",
#' linters = nested_pipe_linter()
#' )
#'
#' code <- "df1 %>%\n inner_join(df2 %>%\n select(a, b)\n )"
#' writeLines(code)
#' lint(
#' text = code,
#' linters = nested_pipe_linter(allow_outer_calls = "inner_join")
#' )
#'
#' lint(
#' text = "tryCatch(x %>% filter(grp == 'a'), error = identity)",
#' linters = nested_pipe_linter()
#' )
#'
#' @evalRd rd_tags("nested_pipe_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
nested_pipe_linter <- function(
allow_inline = TRUE,
allow_outer_calls = c("try", "tryCatch", "withCallingHandlers")) {
multiline_and <- if (allow_inline) "@line1 != @line2 and" else ""
xpath <- glue("
(//PIPE | //SPECIAL[{ xp_text_in_table(magrittr_pipes) }])
/parent::expr[{multiline_and} preceding-sibling::expr/SYMBOL_FUNCTION_CALL[
not({ xp_text_in_table(allow_outer_calls) })
and (
text() != 'switch'
or parent::expr
/following-sibling::expr[1]
/*[self::PIPE or self::SPECIAL[{ xp_text_in_table(magrittr_pipes) }]]
)
]]
")

Linter(function(source_expression) {
if (!is_lint_level(source_expression, "expression")) {
return(list())
}

xml <- source_expression$xml_parsed_content

bad_expr <- xml_find_all(xml, xpath)

xml_nodes_to_lints(
bad_expr,
source_expression = source_expression,
lint_message = "Don't nest pipes inside other calls.",
type = "warning"
)
})
}
1 change: 1 addition & 0 deletions inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ missing_argument_linter,correctness common_mistakes configurable
missing_package_linter,robustness common_mistakes
namespace_linter,correctness robustness configurable executing
nested_ifelse_linter,efficiency readability
nested_pipe_linter,readability consistency configurable
no_tab_linter,style consistency deprecated
nonportable_path_linter,robustness best_practices configurable
nrow_subset_linter,efficiency consistency best_practices
Expand Down
1 change: 1 addition & 0 deletions man/configurable_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/consistency_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

68 changes: 68 additions & 0 deletions man/nested_pipe_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/readability_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 550f937

Please sign in to comment.