Merge pull request #903 from lorenzwalthert/issue-901

Stricter alignemnt detection for unequal number of tokens on different lines

Author: lorenzwalthert

NEWS.md

@@ -35,6 +35,9 @@
 * `Warning: Unknown or uninitialised column:` was fixed (#885).
 * Unaligned expressions with quoted key (e.g. `c("x" = 2)`) are now correctly
   detected (#881).
+* function calls with unequal number of token on different lines are no longer 
+  deemed aligned if there are arbitrary spaces around the tokens on the lines 
+  with most tokens (#902).
 * ensure a trailing blank line also if the input is cached (#867).
 * Preserve trailing blank line in roxygen examples to simplify concatenation of
   examples (#880).

R/detect-alignment.R

@@ -84,7 +84,10 @@ token_is_on_aligned_line <- function(pd_flat) {
   if (any(starting_with_comma)) {
     return(FALSE)
   }
-  pd_is_multi_line <- map_lgl(pd_by_line, ~ any(.x$multi_line > 0L, na.rm = TRUE))
+  pd_is_multi_line <- map_lgl(
+    pd_by_line,
+    ~ any(.x$multi_line > 0L, na.rm = TRUE)
+  )
   if (any(pd_is_multi_line)) {
     return(FALSE)
   }
@@ -98,9 +101,9 @@ token_is_on_aligned_line <- function(pd_flat) {
     alignment_ensure_trailing_comma()
   # now, pd only contains arguments separated by values, ideal for iterating
   # over columns.
-
   n_cols <- map_int(pd_by_line, ~ sum(.x$token == "','"))
   previous_line <- 0
+  current_col <- 0
   start_eval <- ifelse(alignment_col1_all_named(pd_by_line), 1, 2)
   for (column in seq2(1, max(n_cols))) {
     by_line <- alignment_serialize_column(pd_by_line, column) %>%
@@ -109,33 +112,47 @@ token_is_on_aligned_line <- function(pd_flat) {
       trimws(which = "right")
     # check 1: match by comma
     # might have fewer lines in subsequent columns.
-    current_col <- nchar(by_line)
+    max_previous_col <- max(current_col)
+
+    # first col has no leading ,
+    current_col <- nchar(by_line) - as.integer(column > 1)
     if (column > 1) {
-      previous_line <- previous_line[intersect(names(previous_line), names(by_line))]
+      previous_line <- previous_line[
+        intersect(names(previous_line), names(by_line))
+      ]
       # must add previous columns, as first column might not align
       current_col <- current_col + previous_line
     }
 
     is_aligned <- length(unique(current_col)) == 1L
-    if (!is_aligned) {
+    if (!is_aligned || length(current_col) < 2) {
       # check 2: left aligned after ,
-      current_col <- nchar(gsub("^(,[\\s\\t]*)[^ ]*.*$", "\\1", by_line, perl = TRUE))
+      current_col <- "^(,[\\s\\t]*)[^ ]*.*$" %>%
+        gsub("\\1", by_line, perl = TRUE) %>%
+        nchar() %>%
+        magrittr::subtract(1)
 
       if (column > 1) {
         # must add previous columns, as first column might not align
-        current_col <- current_col + previous_line
+        current_col <- previous_line + current_col
       }
-      is_aligned <- length(unique(current_col)) == 1L
+      if (length(current_col) > 1) {
+        is_aligned <- length(unique(current_col)) == 1L
+      } else {
+        is_aligned <- current_col - max_previous_col == 1
+        current_col <- max_previous_col + current_col
+      }
+
       if (is_aligned) {
         # if left aligned after ,
         start_eval <- 2
       }
     }
     if (is_aligned) {
-      previous_line <- previous_line + nchar(by_line)
+      previous_line <- current_col
       next
     }
-    # check 3: match by = (no extra spaces around it allowed.)
+    # check 3: match by = (no extra spaces around it allowed)
     # match left aligned after =
     start_after_eq <- regexpr("= [^ ]", by_line)
     names(start_after_eq) <- names(by_line)
@@ -145,14 +162,17 @@ token_is_on_aligned_line <- function(pd_flat) {
       if (length(start_after_eq) == 0) {
         return(FALSE)
       }
-      # when match via comma unsuccessful, matching by = must yield at least one =
+      # when match via , unsuccessful, matching by = must yield at least one =
       if (column == 1) {
         current_col <- start_after_eq
       } else {
         current_col <- start_after_eq +
           previous_line[intersect(names(previous_line), names(start_after_eq))]
       }
-      is_aligned <- length(unique(current_col)) == 1 && length(start_after_eq) > 1
+      is_aligned <- all(
+        length(unique(current_col)) == 1,
+        length(start_after_eq) > 1
+      )
       if (!is_aligned) {
         return(FALSE)
       }

tests/testthat/alignment/cols-with-one-row-in.R

@@ -0,0 +1,16 @@
+c(
+  "x",       "z",
+  "cgjhg", "thi", "z"
+)
+
+
+c(
+  "x",       "z",
+  "cgjhg",  "thi", "z"
+)
+
+
+c(
+  "x", "y", "z",  "m", "n",  "o", "p",
+  "c", "d"
+)

tests/testthat/alignment/cols-with-one-row-in_tree

@@ -0,0 +1,16 @@
+c(
+  "x",       "z",
+  "cgjhg", "thi", "z"
+)
+
+
+c(
+  "x", "z",
+  "cgjhg", "thi", "z"
+)
+
+
+c(
+  "x", "y", "z", "m", "n", "o", "p",
+  "c", "d"
+)

tests/testthat/alignment/cols-with-one-row-out.R

@@ -57,7 +57,7 @@ call(
 # algorithm: aligned. human: aligned.
 call(
   x  = 1,
-  xy = 2,   n    = 33, z = "333"
+  xy = 2, n = 33, z = "333"
 )
 
 # algorithm: aligned. human: aligned.

tests/testthat/alignment/named-in.R

@@ -57,7 +57,7 @@ call(
 # algorithm: aligned. human: aligned.
 call(
   x  = 1,
-  xy = 2,   n    = 33, z = "333"
+  xy = 2, n = 33, z = "333"
 )
 
 # algorithm: aligned. human: aligned.