Skip to content

Commit 35ab376

Browse files
defer implementing parallel warning (etc) branches for now
1 parent 01fb1e3 commit 35ab376

File tree

2 files changed

+5
-28
lines changed

2 files changed

+5
-28
lines changed

R/unnecessary_nesting_linter.R

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -62,32 +62,20 @@
6262
#' @export
6363
unnecessary_nesting_linter <- function(allow_assignment = TRUE) {
6464
exit_calls <- c("stop", "return", "abort", "quit", "q")
65-
# These calls can be called in the sibling branch and not trigger a lint,
66-
# allowing for cleanly parallel code, where breaking it would often harm readability:
67-
# > if (A) {
68-
# > stop()
69-
# > } else {
70-
# > warning()
71-
# > }
72-
# NB: print() is intentionally excluded since its usage is usually a mistake (?print_linter)
73-
signal_calls <- c(
74-
exit_calls,
75-
"warning", "warn", "message", "cat", "LOG", "stopifnot"
76-
)
7765
exit_call_expr <- glue("
78-
expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]]
66+
expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]]
7967
")
8068
# block IF here for cases where a nested if/else is entirely within
8169
# one of the branches.
8270
# TODO(michaelchirico): we could try and make the parallel exits requirement
8371
# more recursive, but it's a pain to do so.
84-
no_signal_call_expr <- glue("
72+
no_exit_call_expr <- glue("
8573
expr[
8674
OP-LEFT-BRACE
8775
and expr[
8876
position() = last()
8977
and not(IF)
90-
and not(expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(signal_calls)}]])
78+
and not(expr[SYMBOL_FUNCTION_CALL[{xp_text_in_table(exit_calls)}]])
9179
]
9280
]
9381
")
@@ -107,8 +95,8 @@ unnecessary_nesting_linter <- function(allow_assignment = TRUE) {
10795
OP-LEFT-BRACE
10896
and expr[position() = last() and {exit_call_expr}]
10997
and (
110-
following-sibling::{no_signal_call_expr}
111-
or preceding-sibling::{no_signal_call_expr}
98+
following-sibling::{no_exit_call_expr}
99+
or preceding-sibling::{no_exit_call_expr}
112100
)
113101
]
114102
]

tests/testthat/test-unnecessary_nesting_linter.R

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,6 @@ test_that("unnecessary_nesting_linter skips allowed usages", {
2121
expect_lint(double_return_lines, NULL, linter)
2222
})
2323

24-
test_that("parallel stop()/warning() branches are OK", {
25-
stop_warning_lines <- c(
26-
"if (i == force.iter) {",
27-
" stop(msg, call. = FALSE)",
28-
"} else {",
29-
" warning(attempt, call. = FALSE)",
30-
"}"
31-
)
32-
expect_lint(stop_warning_lines, NULL, unnecessary_nesting_linter())
33-
})
34-
3524
# TODO(michaelchirico): consider if there's a nice easy pattern to enforce for
3625
# multiple if/else cases. This test in particular would be easy to un-nest,
3726
# but it's not true in general.

0 commit comments

Comments
 (0)