Merge pull request #536 from lorenzwalthert/feature-curly-for-mutlili…

…ne-fun-dec

- Function declarations should gain braces for body if strict = TRUE (#536).

NEWS.md

@@ -2,28 +2,35 @@
 
 ## Breaking changes
 
-* `style_file()` now correctly styles multiple files from different 
-  directories. We no longer display the file name of the styled file, but
-  the absolute path. This is also reflected in the invisible return value of the 
-  function (#522).
-* `style_file()` and friends do not write content back to a file when 
-  styling does not cause any changes in the file. This means the modification 
-  date of files styled is only changed when the content is changed (#532).
+* `style_file()` now correctly styles multiple files from different directories.
+  We no longer display the file name of the styled file, but the absolute path.
+  This is also reflected in the invisible return value of the function (#522).
+
+* `style_file()` and friends do not write content back to a file when styling
+  does not cause any changes in the file. This means the modification date of
+  files styled is only changed when the content is changed (#532).
 
 ## New features
 
-* curlyl-curly (`{{`) syntactic sugar introduced with rlang 0.4.0 is now 
-  explicitly handled, as opposed previously where it was just treated as two 
+* curlyl-curly (`{{`) syntactic sugar introduced with rlang 0.4.0 is now
+  explicitly handled, as opposed previously where it was just treated as two
   consequtive curly braces (#528).
-* `style_pkg()`, `style_dir()` and the Addins can now style `.Rprofile`, and 
+
+* `style_pkg()`, `style_dir()` and the Addins can now style `.Rprofile`, and
   hidden files are now also styled (#530).
 
 ## Minor improvements and fixes
 
 * escape characters in roxygen code examples are now correctly escaped (#512).
-* style selection Addin now preserves line break when the last line selected is 
+
+* style selection Addin now preserves line break when the last line selected is
   an entire line (#520).
+
 * style file Addin can now properly handle cancelling (#511).
+
+* The body of a multi-line function declaration is now indented correctly for
+  `strict = FALSE` and also wrapped in curly braces for `strict = TRUE` (#536).
+
 * advice for contributors in `CONTRIBUTING.md` was updated (#508).
 
 ## Adaption
@@ -41,8 +48,8 @@ This is primarily a maintenance release upon the request of the CRAN team
 - Users can now control style configurations for styler Addins (#463, #500),
   using the `Set style` Addin. See `?styler::styler_addins` for details.
 
-- `return()` is now always put in braces and put on a new line when used in 
-  a conditional statement (#492).
+- `return()` is now always put in braces and put on a new line when used in a
+  conditional statement (#492).
 
 - `%>%` almost always causes a line break now for `strict = TRUE` (#503).
 
@@ -55,20 +62,20 @@ This is primarily a maintenance release upon the request of the CRAN team
 - indention in roxygen code example styling (#455) and EOF spacing (#469) was
   fixed.
 
-- indention for for loop edge case (#457) and comments in pipe chain (#482)
-  were fixed.
+- indention for for loop edge case (#457) and comments in pipe chain (#482) were
+  fixed.
 
 - line-break styling around comma is improved (#479).
 
--  bug that can cause an error when the variable `text` in any name space
-  before styler on the search path was defined and did not have length 1 is
-  fixed (#484).
+- bug that can cause an error when the variable `text` in any name space before
+  styler on the search path was defined and did not have length 1 is fixed
+  (#484).
 
 - slightly confusing warning about empty strings caused with roxygen code
   examples and Rmd was removed.
 
-- right apostrophe to let package pass R CMD Check in strict Latin-1
-  locale was removed (#490, reason for release).
+- right apostrophe to let package pass R CMD Check in strict Latin-1 locale was
+  removed (#490, reason for release).
 
 ## Adaption of styler
 

R/indent.R

@@ -87,13 +87,19 @@ indent_without_paren <- function(pd, indent_by = 2) {
 #'   definitions without parenthesis.
 #' @keywords internal
 indent_without_paren_for_while_fun <- function(pd, indent_by) {
+
+  tokens <- c("FOR", "WHILE", "FUNCTION")
   nrow <- nrow(pd)
-  if (!(pd$token[1] %in% c("FOR", "WHILE", "FUNCTION"))) {
+  if (!(pd$token[1] %in% tokens)) {
     return(pd)
   }
   if (is_curly_expr(pd$child[[nrow]])) {
     return(pd)
   }
+
+  if (pd$newlines[length(pd$newlines) - 1] == 0 ) {
+    return(pd)
+  }
   pd$indent[nrow] <- indent_by
   pd
 }
@@ -103,19 +109,33 @@ indent_without_paren_for_while_fun <- function(pd, indent_by) {
 #' @keywords internal
 indent_without_paren_if_else <- function(pd, indent_by) {
   expr_after_if <- next_non_comment(pd, which(pd$token == "')'")[1])
+  is_if <- pd$token[1] %in% "IF"
   has_if_without_curly <-
-    pd$token[1] %in% "IF" && pd$child[[expr_after_if]]$token[1] != "'{'"
-  if (has_if_without_curly) {
+    is_if && pd$child[[expr_after_if]]$token[1] != "'{'"
+  if (!is_if) {
+    return(pd)
+  }
+  needs_indention_now <- pd$lag_newlines[next_non_comment(pd, which(pd$token == "')'"))] > 0
+
+  if (needs_indention_now) {
     pd$indent[expr_after_if] <- indent_by
   }
 
+
+
   else_idx <- which(pd$token == "ELSE")
+  if (length(else_idx) == 0) {
+    return(pd)
+  }
   expr_after_else_idx <- next_non_comment(pd, else_idx)
   has_else_without_curly_or_else_chid <-
     any(pd$token == "ELSE") &&
       pd$child[[expr_after_else_idx]]$token[1] != "'{'" &&
       pd$child[[expr_after_else_idx]]$token[1] != "IF"
-  if (has_else_without_curly_or_else_chid) {
+
+  needs_indention_now <- pd$lag_newlines[next_non_comment(pd, which(pd$token == "ELSE"))] > 0
+
+  if (has_else_without_curly_or_else_chid && needs_indention_now) {
     pd$indent[seq(else_idx + 1, nrow(pd))] <- indent_by
   }
   pd
@@ -212,7 +232,10 @@ needs_indention <- function(pd,
 #' @importFrom rlang seq2
 #' @keywords internal
 #' @examples
-#' style_text("call(named = c, \nnamed = b)", strict = FALSE)
+#' style_text(c(
+#'   "call(named = c,",
+#'   "named = b)"
+#' ), strict = FALSE)
 needs_indention_one <- function(pd,
                                 potential_trigger_pos,
                                 other_trigger_tokens) {

R/rules-other.R

@@ -33,12 +33,13 @@ add_brackets_in_pipe_one <- function(pd, pos) {
 #'   braces. Used for unindention.
 #' @keywords internal
 #' @importFrom purrr when
-wrap_if_else_while_for_multi_line_in_curly <- function(pd, indent_by = 2) {
+wrap_if_else_while_for_fun_multi_line_in_curly <- function(pd, indent_by = 2) {
   key_token <- when(
     pd,
     is_cond_expr(.) ~ "')'",
     is_while_expr(.) ~ "')'",
-    is_for_expr(.) ~ "forcond"
+    is_for_expr(.) ~ "forcond",
+    is_function_dec(.) ~ "')'"
   )
   if (length(key_token) > 0) {
     pd <- pd %>%
@@ -56,7 +57,7 @@ wrap_if_else_while_for_multi_line_in_curly <- function(pd, indent_by = 2) {
 
 #' Wrap a multi-line statement in curly braces
 #'
-#' @inheritParams wrap_if_else_while_for_multi_line_in_curly
+#' @inheritParams wrap_if_else_while_for_fun_multi_line_in_curly
 #' @inheritParams wrap_subexpr_in_curly
 #' @param key_token The token that comes right before the token that contains
 #'   the expression to be wrapped (ignoring comments). For if and while loops,
@@ -125,7 +126,7 @@ wrap_subexpr_in_curly <- function(pd,
     stretch_out = c(!to_be_wrapped_starts_with_comment, TRUE),
     space_after = space_after
   )
-  new_expr$indent <- pd$indent[last(ind_to_be_wrapped)] - indent_by
+  new_expr$indent <- max(pd$indent[last(ind_to_be_wrapped)] - indent_by, 0)
   new_expr_in_expr <- new_expr %>%
     wrap_expr_in_expr() %>%
     remove_attributes(c("token_before", "token_after"))

R/style-guides.R

@@ -155,8 +155,8 @@ tidyverse_style <- function(scope = "tokens",
       resolve_semicolon,
       add_brackets_in_pipe,
       remove_terminal_token_before_and_after,
-      wrap_if_else_while_for_multi_line_in_curly =
-        if (strict) wrap_if_else_while_for_multi_line_in_curly
+      wrap_if_else_while_for_fun_multi_line_in_curly =
+        if (strict) wrap_if_else_while_for_fun_multi_line_in_curly
     )
   }
 

tests/testthat/indention_multiple/fun_for_new_line-out.R

@@ -1,5 +1,6 @@
-function()
+function() {
   NULL
+}
 
 for (i in 1:3)
 {

tests/testthat/indention_operators/function-multiline-no-braces-non-strict-in.R

@@ -0,0 +1,26 @@
+g <- function(k)
+  NULL
+
+
+g <- function(k) h(
+  NULL
+)
+
+
+g <- function(k) h( # y
+  NULL # x
+)
+
+g <- function(k) h( # y
+  NULL
+)
+
+
+g <- function(k) h(
+  NULL # 3jkö
+)
+
+g <- function(k) h(
+  if (TRUE)
+    x
+)