From 57f74d8edb2a9d678a8f115dfb7b49d97d81a3b4 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Tue, 26 Jul 2022 15:21:20 +0100 Subject: [PATCH 1/2] detect alignemnt for function declarations --- NEWS.md | 2 ++ R/detect-alignment-utils.R | 36 +++++++++++++++++++++-- R/detect-alignment.R | 6 ++-- R/rules-spaces.R | 11 ++++--- man/alignment_drop_last_expr.Rd | 27 ++++++++++++++++++ tests/testthat/alignment/fun-decs-in.R | 38 +++++++++++++++++++++++++ tests/testthat/alignment/fun-decs-out.R | 38 +++++++++++++++++++++++++ 7 files changed, 150 insertions(+), 8 deletions(-) create mode 100644 man/alignment_drop_last_expr.Rd create mode 100644 tests/testthat/alignment/fun-decs-in.R create mode 100644 tests/testthat/alignment/fun-decs-out.R diff --git a/NEWS.md b/NEWS.md index 1c2d81066..ce072c002 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,8 @@ editor_options: **Features** - `filetype` `.qmd` is now supported, but not turned on by default (#931). +- alignment is now detected for function declaration in a similar way as for + function calls (#968). - new R option `styler.ignore_alignment` controls if alignment should be detected (and preserved) or not (#932). diff --git a/R/detect-alignment-utils.R b/R/detect-alignment-utils.R index e868be4e8..95ad1702d 100644 --- a/R/detect-alignment-utils.R +++ b/R/detect-alignment-utils.R @@ -41,6 +41,38 @@ alignment_drop_comments <- function(pd_by_line) { compact() } + +#' Remove last expression +#' +#' In a *nest*, if the last token is an `expr`, the *nest* represents either +#' an if, while or for statement or a function call. We don't call about that +#' part, in fact it's important to remove it for alignment. See 'Examples'. +#' +#' @examples +#' if (FALSE) { +#' call( +#' x = 12, +#' y = 3, +#' ) +#' +#' function(a = 33, +#' qq = 4) { +#' # we don't care about this part for alignment detection +#' } +#' } +#' @keywords internal +alignment_drop_last_expr <- function(pds_by_line) { + # TODO could be skipped if we know it's not a function dec + pd_last_line <- pds_by_line[[length(pds_by_line)]] + last_two_lines <- pd_last_line$token[c(nrow(pd_last_line) - 1, nrow(pd_last_line))] + if (identical(last_two_lines, c("')'", "expr"))) { + pd_last_line <- pd_last_line[-nrow(pd_last_line), ] + } + pds_by_line[[length(pds_by_line)]] <- pd_last_line + pds_by_line +} + + #' Ensure last pd has a trailing comma #' #' Must be after [alignment_ensure_no_closing_brace()] because if it comes after @@ -83,9 +115,9 @@ alignment_col1_all_named <- function(relevant_pd_by_line) { return(FALSE) } x$token[3] == "expr" && - x$token[1] %in% c("SYMBOL_SUB", "STR_CONST") && + x$token[1] %in% c("SYMBOL_SUB", "STR_CONST", "SYMBOL_FORMALS") && x$token[2] %in% c( - "EQ_SUB", "SPECIAL-IN", "LT", "GT", "EQ", "NE" + "EQ_SUB", "EQ_FORMALS", "SPECIAL-IN", "LT", "GT", "EQ", "NE" ) }) %>% all() diff --git a/R/detect-alignment.R b/R/detect-alignment.R index 834067935..9d6cef428 100644 --- a/R/detect-alignment.R +++ b/R/detect-alignment.R @@ -92,11 +92,13 @@ token_is_on_aligned_line <- function(pd_flat) { return(FALSE) } - pd_by_line <- alignment_drop_comments(pd_by_line) %>% - alignment_ensure_no_closing_brace(last_line_is_closing_brace_only) + pd_by_line <- alignment_drop_comments(pd_by_line) if (length(pd_by_line) < 1) { return(TRUE) } + pd_by_line <- alignment_drop_last_expr(pd_by_line) %>% + alignment_ensure_no_closing_brace(last_line_is_closing_brace_only) + pd_by_line <- pd_by_line %>% alignment_ensure_trailing_comma() # now, pd only contains arguments separated by values, ideal for iterating diff --git a/R/rules-spaces.R b/R/rules-spaces.R index 56532f0e9..bc3940cf4 100644 --- a/R/rules-spaces.R +++ b/R/rules-spaces.R @@ -16,10 +16,13 @@ set_space_around_op <- function(pd_flat, strict) { if (!any(op_after)) { return(pd_flat) } - if (sum(pd_flat$lag_newlines) > 2 && - is_function_call(pd_flat) && - any(pd_flat$token %in% c("EQ_SUB", "','")) && - !getOption("styler.ignore_alignment", FALSE) + if ( + !getOption("styler.ignore_alignment", FALSE) && + ( + (is_function_call(pd_flat) && sum(pd_flat$lag_newlines) > 2) || + (is_function_dec(pd_flat) && sum(pd_flat$lag_newlines) > 1) + ) && + any(pd_flat$token %in% c("EQ_SUB", "','", "EQ_FORMALS")) ) { is_on_aligned_line <- token_is_on_aligned_line(pd_flat) } else { diff --git a/man/alignment_drop_last_expr.Rd b/man/alignment_drop_last_expr.Rd new file mode 100644 index 000000000..dc86683c1 --- /dev/null +++ b/man/alignment_drop_last_expr.Rd @@ -0,0 +1,27 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/detect-alignment-utils.R +\name{alignment_drop_last_expr} +\alias{alignment_drop_last_expr} +\title{Remove last expression} +\usage{ +alignment_drop_last_expr(pds_by_line) +} +\description{ +In a \emph{nest}, if the last token is an \code{expr}, the \emph{nest} represents either +an if, while or for statement or a function call. We don't call about that +part, in fact it's important to remove it for alignment. See 'Examples'. +} +\examples{ +if (FALSE) { + call( + x = 12, + y = 3, + ) + + function(a = 33, + qq = 4) { + # we don't care about this part for alignment detection + } +} +} +\keyword{internal} diff --git a/tests/testthat/alignment/fun-decs-in.R b/tests/testthat/alignment/fun-decs-in.R new file mode 100644 index 000000000..68b5f335f --- /dev/null +++ b/tests/testthat/alignment/fun-decs-in.R @@ -0,0 +1,38 @@ +# aligned +function(x = NULL, + tt = NULL, + ayz = NULL) {} + + +# aligned +k <- function(x = NULL, + aq = NULL, + ayz = NULL) {} + + +# aligned, eq right +function(x = 2, + tt = 1, + ayz = 99) {} + +# aligned, eq left +function(x = 2, + tt = 1, + ayz = 99) {} + + +# not aligned +k <- function(x = fish, + aq = 21, + ayz = t(322)) {} + +# aligned +k <- function(x = flus(we), + aq = x - 22, k = 22, + ayz = m(jk5), xfea = 3) {} + + +# aligned +k <- function(x = flus(we), + aq = x - 22, k = 22, + ayz = m(jk5), xfea = 3) {} diff --git a/tests/testthat/alignment/fun-decs-out.R b/tests/testthat/alignment/fun-decs-out.R new file mode 100644 index 000000000..fcefee559 --- /dev/null +++ b/tests/testthat/alignment/fun-decs-out.R @@ -0,0 +1,38 @@ +# aligned +function(x = NULL, + tt = NULL, + ayz = NULL) {} + + +# aligned +k <- function(x = NULL, + aq = NULL, + ayz = NULL) {} + + +# aligned, eq right +function(x = 2, + tt = 1, + ayz = 99) {} + +# aligned, eq left +function(x = 2, + tt = 1, + ayz = 99) {} + + +# not aligned +k <- function(x = fish, + aq = 21, + ayz = t(322)) {} + +# aligned +k <- function(x = flus(we), + aq = x - 22, k = 22, + ayz = m(jk5), xfea = 3) {} + + +# aligned +k <- function(x = flus(we), + aq = x - 22, k = 22, + ayz = m(jk5), xfea = 3) {} From 8a830397fe2c3216df81f3a1a5b88883e382613f Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 6 Aug 2022 20:12:05 +0200 Subject: [PATCH 2/2] bump