Add param function_braces to brace_linter()

NEWS.md

@@ -55,6 +55,7 @@
 * `todo_comment_linter()` has a new argument `except_regex` for setting _valid_ TODO comments, e.g. for forcing TODO comments to be linked to GitHub issues like `TODO(#154)` (#2047, @MichaelChirico).
 * `vector_logic_linter()` is extended to recognize incorrect usage of scalar operators `&&` and `||` inside subsetting expressions like `dplyr::filter(x, A && B)` (#2166, @MichaelChirico).
 * `any_is_na_linter()` is extended to catch the unusual usage `NA %in% x` (#2113, @MichaelChirico).
+* `brace_linter()`' has a new argument `function_bodies` (default `"multi_line"`) which controls whether to require function bodies to be wrapped in curly braces, with the options `"always"`, `"multi_line"` (only require curly braces when a function body spans multiple lines), `"not_inline"` (only require curly braces when a function body starts on a new line) and `"never"` (#1807, #2240, @salim-b).
 * `make_linter_from_xpath()` errors up front when `lint_message` is missing (instead of delaying this error until the linter is used, #2541, @MichaelChirico).
 * `paste_linter()` is extended to recommend using `paste()` instead of `paste0()` for simply aggregating a character vector with `collapse=`, i.e., when `sep=` is irrelevant (#1108, @MichaelChirico).
 * `expect_no_lint()` was added as new function to cover the typical use case of expecting no lint message, akin to the recent {testthat} functions like `expect_no_warning()` (#2580, @F-Noelle).

R/brace_linter.R

@@ -8,9 +8,14 @@
 #' - Closing curly braces in `if` conditions are on the same line as the corresponding `else`.
 #' - Either both or neither branch in `if`/`else` use curly braces, i.e., either both branches use `{...}` or neither
 #' does.
-#' - Functions spanning multiple lines use curly braces.
+#' - Function bodies are wrapped in curly braces.
 #'
-#' @param allow_single_line if `TRUE`, allow an open and closed curly pair on the same line.
+#' @param allow_single_line If `TRUE`, allow an open and closed curly pair on the same line.
+#' @param function_bodies Whether to require function bodies to be wrapped in curly braces. One of
+#' - `"always"` to require braces for all function definitions, including inline functions,
+#' - `"not_inline"` to require braces when a function body does not start on the same line as its signature,
+#' - `"multi_line"` (the default) to require braces when a function definition spans multiple lines,
+#' - `"never"` to never require braces in function bodies.
 #'
 #' @examples
 #' # will produce lints
@@ -50,7 +55,10 @@
 #' - <https://style.tidyverse.org/syntax.html#indenting>
 #' - <https://style.tidyverse.org/syntax.html#if-statements>
 #' @export
-brace_linter <- function(allow_single_line = FALSE) {
+brace_linter <- function(allow_single_line = FALSE,
+ function_bodies = c("multi_line", "always", "not_inline", "never")) {
+ function_bodies <- match.arg(function_bodies)
+
  xp_cond_open <- xp_and(c(
  # matching } is on same line
  if (isTRUE(allow_single_line)) {
@@ -124,7 +132,25 @@ brace_linter <- function(allow_single_line = FALSE) {
  # TODO(#1103): if c_style_braces is TRUE, this needs to be @line2 + 1
  xp_else_same_line <- glue("//ELSE[{xp_else_closed_curly} and @line1 != {xp_else_closed_curly}/@line2]")
 
- xp_function_brace <- "(//FUNCTION | //OP-LAMBDA)/parent::expr[@line1 != @line2 and not(expr[OP-LEFT-BRACE])]"
+ if (function_bodies != "never") {
+ xp_cond_function_brace <- switch(
+ function_bodies,
+ always = "true",
+ multi_line = "@line1 != @line2",
+ not_inline = "@line1 != expr/@line1"
+ )
+
+ xp_function_brace <- glue(
+ "(//FUNCTION | //OP-LAMBDA)/parent::expr[{xp_cond_function_brace} and not(expr/OP-LEFT-BRACE)]"
+ )
+
+ msg_function_brace <- switch(
+ function_bodies,
+ always = "Wrap function bodies in curly braces.",
+ multi_line = "Wrap multi-line function bodies in curly braces.",
+ not_inline = "Wrap function bodies starting on a new line in curly braces."
+ )
+ }
 
  # if (x) { ... } else if (y) { ... } else { ... } is OK; fully exact pairing
  # of if/else would require this to be
@@ -188,15 +214,16 @@ brace_linter <- function(allow_single_line = FALSE) {
  lint_message = "`else` should come on the same line as the previous `}`."
  )
  )
-
- lints <- c(
- lints,
- xml_nodes_to_lints(
- xml_find_all(xml, xp_function_brace),
- source_expression = source_expression,
- lint_message = "Use curly braces for any function spanning multiple lines."
+ if (function_bodies != "never") {
+ lints <- c(
+ lints,
+ xml_nodes_to_lints(
+ xml_find_all(xml, xp_function_brace),
+ source_expression = source_expression,
+ lint_message = msg_function_brace
+ )
  )
- )
+ }
 
  lints <- c(
  lints,

tests/testthat/test-brace_linter.R

@@ -299,25 +299,62 @@ test_that("brace_linter lints else correctly", {
 })
 
 test_that("brace_linter lints function expressions correctly", {
- linter <- brace_linter()
- expect_lint("function(x) 4", NULL, linter)
+ msg_always <- rex::rex("Wrap function bodies in curly braces.")
+ msg_multi_line <- rex::rex("Wrap multi-line function bodies in curly braces.")
+ msg_not_inline <- rex::rex("Wrap function bodies starting on a new line in curly braces.")
+
+ linter_always <- brace_linter(function_bodies = "always")
+ linter_multi_line <- brace_linter(function_bodies = "multi_line")
+ linter_not_inline <- brace_linter(function_bodies = "not_inline")
+ linter_never <- brace_linter(function_bodies = "never")
 
  lines <- trim_some("
  function(x) {
  x + 4
  }
  ")
- expect_lint(lines, NULL, linter)
+ expect_lint(lines, NULL, linter_always)
+ expect_lint(lines, NULL, linter_multi_line)
+ expect_lint(lines, NULL, linter_not_inline)
+ expect_lint(lines, NULL, linter_never)
+
+ lints_single_line <- list(
+ rex::rex("Opening curly braces should never go on their own line and should always be followed by a new line."),
+ rex::rex("Closing curly-braces should always be on their own line, unless they are followed by an else.")
+ )
+ expect_lint("function(x) { x + 4 }", lints_single_line, linter_always)
+ expect_lint("function(x) { x + 4 }", lints_single_line, linter_multi_line)
+ expect_lint("function(x) { x + 4 }", lints_single_line, linter_not_inline)
+ expect_lint("function(x) { x + 4 }", lints_single_line, linter_never)
+ # using function_bodies = "always" without allow_single_line = TRUE prohibits inline function definitions:
+ expect_lint(
+ "function(x) { x + 4 }",
+ NULL,
+ brace_linter(allow_single_line = TRUE, function_bodies = "always")
+ )
+
+ expect_lint("function(x) x + 4", msg_always, linter_always)
+ expect_lint("function(x) x + 4", NULL, linter_multi_line)
+ expect_lint("function(x) x + 4", NULL, linter_not_inline)
+ expect_lint("function(x) x + 4", NULL, linter_never)
+
+ lines <- trim_some("
+ function(x) x +
+ 4
+ ")
+ expect_lint(lines, msg_always, linter_always)
+ expect_lint(lines, msg_multi_line, linter_multi_line)
+ expect_lint(lines, NULL, linter_not_inline)
+ expect_lint(lines, NULL, linter_never)
 
  lines <- trim_some("
  function(x)
- x+4
+ x + 4
  ")
- expect_lint(
- lines,
- rex::rex("Use curly braces for any function spanning multiple lines."),
- linter
- )
+ expect_lint(lines, msg_always, linter_always)
+ expect_lint(lines, msg_multi_line, linter_multi_line)
+ expect_lint(lines, msg_not_inline, linter_not_inline)
+ expect_lint(lines, NULL, linter_never)
 })
 
 test_that("brace_linter lints if/else matching braces correctly", {