diff --git a/NEWS.md b/NEWS.md index 07073e65b..660e7b66a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -20,6 +20,7 @@ * `object_name_linter()` no longer attempts to lint strings in function calls on the LHS of assignments (#1466, @MichaelChirico). * `infix_spaces_linter()` allows finer control for linting `=` in different scenarios using parse tags `EQ_ASSIGN`, `EQ_SUB`, and `EQ_FORMALS` (#1977, @MichaelChirico). * `equals_na_linter()` checks for `x %in% NA`, which is a more convoluted form of `is.na(x)` (#2088, @MichaelChirico). +* `brace_linter()`'s new `function_braces` param allows to specify 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 over multiple lines), and `"never"` (#1807, #2240, @salim-b). ## New and improved features diff --git a/R/brace_linter.R b/R/brace_linter.R index 787cd5324..1d2023c5c 100644 --- a/R/brace_linter.R +++ b/R/brace_linter.R @@ -8,9 +8,12 @@ #' - 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_braces Character scalar specifying whether to require function bodies to be wrapped in curly braces. +#' `"multi_line"` means to only require curly braces when a function body is not on the same line as its header or it +#' spans over multiple lines. #' #' @examples #' # will produce lints @@ -50,7 +53,10 @@ #' - #' - #' @export -brace_linter <- function(allow_single_line = FALSE) { +brace_linter <- function(allow_single_line = FALSE, + function_braces = c("multi_line", "always", "never")) { + function_braces <- match.arg(function_braces) + xp_cond_open <- xp_and(c( # matching } is on same line if (isTRUE(allow_single_line)) { @@ -124,7 +130,9 @@ brace_linter <- function(allow_single_line = FALSE) { # TODO (AshesITR): 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])]" + xp_function_brace_always <- "(//FUNCTION | //OP-LAMBDA)/parent::expr[not(expr[OP-LEFT-BRACE])]" + xp_function_brace_multi_line <- + "(//FUNCTION | //OP-LAMBDA)/parent::expr[@line1 != @line2 and not(expr[OP-LEFT-BRACE])]" # if (x) { ... } else if (y) { ... } else { ... } is OK; fully exact pairing # of if/else would require this to be @@ -192,14 +200,25 @@ brace_linter <- function(allow_single_line = FALSE) { ) ) - lints <- c( - lints, - xml_nodes_to_lints( - xml_find_all(xml, xp_function_brace), - source_expression = source_expression, - lint_message = "Any function spanning multiple lines should use curly braces." + if (function_braces == "always") { + lints <- c( + lints, + xml_nodes_to_lints( + xml_find_all(xml, xp_function_brace_always), + source_expression = source_expression, + lint_message = "Any function body should be wrapped in curly braces." + ) ) - ) + } else if (function_braces == "multi_line") { + lints <- c( + lints, + xml_nodes_to_lints( + xml_find_all(xml, xp_function_brace_multi_line), + source_expression = source_expression, + lint_message = "Any function body spanning multiple lines should be wrapped in curly braces." + ) + ) + } lints <- c( lints, diff --git a/man/brace_linter.Rd b/man/brace_linter.Rd index 3fb01e06c..e28a9e411 100644 --- a/man/brace_linter.Rd +++ b/man/brace_linter.Rd @@ -4,10 +4,17 @@ \alias{brace_linter} \title{Brace linter} \usage{ -brace_linter(allow_single_line = FALSE) +brace_linter( + allow_single_line = FALSE, + function_braces = c("multi_line", "always", "never") +) } \arguments{ -\item{allow_single_line}{if \code{TRUE}, allow an open and closed curly pair on the same line.} +\item{allow_single_line}{If \code{TRUE}, allow an open and closed curly pair on the same line.} + +\item{function_braces}{Character scalar specifying whether to require function bodies to be wrapped in curly braces. +\code{"multi_line"} means to only require curly braces when a function body is not on the same line as its header or it +spans over multiple lines.} } \description{ Perform various style checks related to placement and spacing of curly braces: @@ -20,7 +27,7 @@ Perform various style checks related to placement and spacing of curly braces: \item Closing curly braces in \code{if} conditions are on the same line as the corresponding \verb{else}. \item Either both or neither branch in \code{if}/\verb{else} use curly braces, i.e., either both branches use \code{{...}} or neither does. -\item Functions spanning multiple lines use curly braces. +\item Function bodies are wrapped in curly braces. } } \examples{ diff --git a/tests/testthat/test-brace_linter.R b/tests/testthat/test-brace_linter.R index e3adc1a59..0a75d6f67 100644 --- a/tests/testthat/test-brace_linter.R +++ b/tests/testthat/test-brace_linter.R @@ -299,24 +299,57 @@ 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) + expect_lint( + "function(x) x + 4", + NULL, + brace_linter(function_braces = "multi_line") + ) lines <- trim_some(" function(x) { x + 4 } ") - expect_lint(lines, NULL, linter) + expect_lint( + lines, + NULL, + brace_linter(function_braces = "multi_line") + ) + msg_always <- rex::rex("Any function body should be wrapped in curly braces.") + msg_multi_line <- rex::rex("Any function body spanning multiple lines should be wrapped in curly braces.") + expect_lint( + "function(x) x + 4", + msg_always, + brace_linter(function_braces = "always") + ) lines <- trim_some(" function(x) - x+4 + x + 4 ") expect_lint( lines, - rex::rex("Any function spanning multiple lines should use curly braces."), - linter + msg_always, + brace_linter(function_braces = "always") + ) + expect_lint( + lines, + msg_multi_line, + brace_linter(function_braces = "multi_line") + ) + expect_lint( + lines, + NULL, + brace_linter(function_braces = "never") + ) + lines <- trim_some(" + function(x) x + + 4 + ") + expect_lint( + lines, + msg_multi_line, + brace_linter(function_braces = "multi_line") ) })