Check for performance improvements with if() + else() instead of ifelse() (#1006)

* get started

* more if + else

* some more conversions

* A couple more

* Update token-create.R

* A couple more

* pre-commit

* Update rules-line-breaks.R

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: IndrajeetPatil

R/detect-alignment-utils.R

@@ -11,12 +11,12 @@ alignment_ensure_no_closing_brace <- function(pd_by_line,
     return(pd_by_line)
   }
   last <- last(pd_by_line)
-  if (nrow(last) == 1) {
+  if (nrow(last) == 1L) {
     # can drop last line completely
     pd_by_line[-length(pd_by_line)]
   } else {
     # only drop last elment of last line
-    pd_by_line[[length(pd_by_line)]] <- last[seq2(1, nrow(last) - 1), ]
+    pd_by_line[[length(pd_by_line)]] <- last[seq2(1L, nrow(last) - 1L), ]
     pd_by_line
   }
 }

R/detect-alignment.R

@@ -53,16 +53,18 @@ token_is_on_aligned_line <- function(pd_flat) {
   if (length(pd_by_line) < 1L) {
     return(TRUE)
   }
-  last_line_is_closing_brace_only <- nrow(last(pd_by_line)) == 1
-  relevant_idx <- seq2(2, ifelse(last_line_is_closing_brace_only,
-    length(pd_by_line) - 1,
+  last_line_is_closing_brace_only <- nrow(last(pd_by_line)) == 1L
+  last_idx <- if (last_line_is_closing_brace_only) {
+    length(pd_by_line) - 1L
+  } else {
     length(pd_by_line)
-  ))
+  }
+  relevant_idx <- seq2(2L, last_idx)
   pd_by_line <- pd_by_line[relevant_idx]
 
   relevant_lag_spaces_col_1 <- map_int(pd_by_line, ~ .x$.lag_spaces[1])
 
-  col1_is_aligned <- length(unique(relevant_lag_spaces_col_1)) == 1
+  col1_is_aligned <- length(unique(relevant_lag_spaces_col_1)) == 1L
   if (!col1_is_aligned) {
     return(FALSE)
   }
@@ -105,7 +107,11 @@ token_is_on_aligned_line <- function(pd_flat) {
   # over columns.
   n_cols <- map_int(pd_by_line, ~ sum(.x$token == "','"))
   previous_line <- current_col <- 0L
-  start_eval <- ifelse(alignment_col1_all_named(pd_by_line), 1L, 2L)
+  start_eval <- if (alignment_col1_all_named(pd_by_line)) {
+    1L
+  } else {
+    2L
+  }
   for (column in seq2(1L, max(n_cols))) {
     by_line <- alignment_serialize_column(pd_by_line, column) %>%
       compact() %>%

R/expr-is.R

@@ -75,16 +75,16 @@ is_comment <- function(pd) {
 #' expression (like `~column`), in the second row if it is a symmetric tilde
 #' expression (like `a~b`).
 #' @keywords internal
-is_tilde_expr <- function(pd, tilde_pos = c(1, 2)) {
-  if (is.null(pd) || nrow(pd) == 1) {
+is_tilde_expr <- function(pd, tilde_pos = c(1L, 2L)) {
+  if (is.null(pd) || nrow(pd) == 1L) {
     return(FALSE)
   }
   any(pd$token[tilde_pos] == "'~'")
 }
 
 #' @rdname is_tilde_expr
 is_asymmetric_tilde_expr <- function(pd) {
-  is_tilde_expr(pd, tilde_pos = 1)
+  is_tilde_expr(pd, tilde_pos = 1L)
 }
 
 #' @rdname is_tilde_expr

R/nested-to-tree.R

@@ -48,10 +48,12 @@ create_tree_from_pd_with_default_style_attributes <- function(pd,
 #' @keywords internal
 create_node_from_nested_root <- function(pd_nested, structure_only) {
   assert_data.tree_installation()
-  n <- data.tree::Node$new(ifelse(
-    structure_only, "Hierarchical structure",
+  name <- if (structure_only) {
+    "Hierarchical structure"
+  } else {
     "ROOT (token: short_text [lag_newlines/spaces] {pos_id})"
-  ))
+  }
+  n <- data.tree::Node$new(name)
   create_node_from_nested(pd_nested, n, structure_only)
   n
 }

R/relevel.R

@@ -52,13 +52,16 @@ flatten_operators_one <- function(pd_nested) {
 #'   from left or from right.
 #' @keywords internal
 flatten_pd <- function(pd_nested, token, child_token = token, left = TRUE) {
-  token_pos_candidates <- which(pd_nested$token[-1] %in% token) + 1
+  token_pos_candidates <- which(pd_nested$token[-1] %in% token) + 1L
   if (length(token_pos_candidates) == 0L) {
     return(pd_nested)
   }
-  token_pos <- token_pos_candidates[
-    ifelse(left, 1, length(token_pos_candidates))
-  ]
+  token_pos_idx <- if (left) {
+    1L
+  } else {
+    length(token_pos_candidates)
+  }
+  token_pos <- token_pos_candidates[token_pos_idx]
   if (left) {
     pos <- previous_non_comment(pd_nested, token_pos)
   } else {

R/rules-line-breaks.R

@@ -65,11 +65,12 @@ set_line_break_before_curly_opening <- function(pd) {
       ~ next_terminal(pd[.x, ], vars = "token_after")$token_after
     ) != "'{'"
     last_expr_idx <- max(which(pd$token == "expr"))
-    is_last_expr <- ifelse(any(c("IF", "WHILE") == pd$token[1]),
+    is_last_expr <- if (any(c("IF", "WHILE") == pd$token[1])) {
       # rule not applicable for if and while
-      TRUE, (line_break_to_set_idx + 1L) == last_expr_idx
-    )
-
+      TRUE
+    } else {
+      (line_break_to_set_idx + 1L) == last_expr_idx
+    }
     no_line_break_before_curly_idx <- any(pd$token[line_break_to_set_idx] == "EQ_SUB")
     linebreak_before_curly <- ifelse(is_function_call(pd),
       # if in function call and has pipe, it is not recognized as function call
@@ -338,7 +339,11 @@ set_line_break_after_opening_if_call_is_multi_line <- function(pd,
 #' @keywords internal
 find_line_break_position_in_multiline_call <- function(pd) {
   candidate <- (which(pd$token == "EQ_SUB") - 1L)[1]
-  ifelse(is.na(candidate), 3L, candidate)
+  if (is.na(candidate)) {
+    3L
+  } else {
+    candidate
+  }
 }
 
 
@@ -368,10 +373,10 @@ remove_line_break_in_fun_call <- function(pd, strict) {
     # no blank lines within function calls
     if (strict) {
       pd$lag_newlines[
-        lag(pd$token == "','") & pd$lag_newlines > 1 & pd$token != "COMMENT"
+        lag(pd$token == "','") & pd$lag_newlines > 1L & pd$token != "COMMENT"
       ] <- 1L
     }
-    if (nrow(pd) == 3) {
+    if (nrow(pd) == 3L) {
       pd$lag_newlines[3] <- 0L
     }
   }
@@ -386,7 +391,7 @@ set_linebreak_after_ggplot2_plus <- function(pd) {
     first_plus <- which(is_plus_raw)[1]
     next_non_comment <- next_non_comment(pd, first_plus)
     is_plus_or_comment_after_plus_before_fun_call <-
-      lag(is_plus_raw, next_non_comment - first_plus - 1, default = FALSE) &
+      lag(is_plus_raw, next_non_comment - first_plus - 1L, default = FALSE) &
         (pd$token_after == "SYMBOL_FUNCTION_CALL" | pd$token_after == "SYMBOL_PACKAGE")
     if (any(is_plus_or_comment_after_plus_before_fun_call, na.rm = TRUE)) {
       gg_call <- pd$child[[previous_non_comment(pd, first_plus)]]$child[[1]]

R/set-assert-args.R

@@ -25,10 +25,11 @@ assert_transformers <- function(transformers) {
   no_name <- is.null(transformers$style_guide_name)
   no_version <- is.null(transformers$style_guide_version)
   if (no_name || no_version) {
-    action <- ifelse(utils::packageVersion("styler") >= version_cutoff,
-      "are not supported anymore",
+    action <- if (utils::packageVersion("styler") >= version_cutoff) {
+      "are not supported anymore"
+    } else {
       "depreciated and will be removed in a future version of styler."
-    )
+    }
     message <- paste(
       "Style guides without a name and a version field are",
       action, "\nIf you are a user: Open an issue on",

R/style-guides.R

@@ -490,7 +490,7 @@ scope_normalize <- function(scope, name = substitute(scope)) {
 
   if (inherits(scope, "AsIs")) {
     factor(as.character(scope), levels = levels, ordered = TRUE)
-  } else if (length(scope) == 1) {
+  } else if (length(scope) == 1L) {
     scope <- levels[as.logical(rev(cumsum(scope == rev(levels))))]
     factor(scope, levels = levels, ordered = TRUE)
   } else {

R/token-create.R

@@ -78,7 +78,11 @@ create_tokens <- function(tokens,
 #' @family token creators
 #' @keywords internal
 create_pos_ids <- function(pd, pos, by = 0.1, after = FALSE, n = 1L) {
-  direction <- ifelse(after, 1L, -1L)
+  direction <- if (after) {
+    1L
+  } else {
+    -1L
+  }
   first <- find_start_pos_id(pd, pos, by, direction, after)
   new_ids <- seq(first,
     to = first + direction * (n - 1) * by, by = by * direction
@@ -105,13 +109,28 @@ find_start_pos_id <- function(pd,
                               candidates = NULL) {
   candidates <- append(candidates, pd$pos_id[pos])
   if (is.null(pd$child[[pos]])) {
-    ifelse(after, max(candidates), min(candidates)) + by * direction
+    start_pos_idx <- if (after) {
+      max(candidates)
+    } else {
+      min(candidates)
+    }
+    start_pos_idx <- start_pos_idx + (by * direction)
   } else {
-    find_start_pos_id(
-      pd$child[[pos]], ifelse(after, nrow(pd$child[[pos]]), 1L),
-      by, direction, after, candidates
+    start_pos_idx <- find_start_pos_id(
+      pd$child[[pos]],
+      if (after) {
+        nrow(pd$child[[pos]])
+      } else {
+        1L
+      },
+      by,
+      direction,
+      after,
+      candidates
     )
   }
+
+  start_pos_idx
 }
 
 
@@ -129,7 +148,12 @@ find_start_pos_id <- function(pd,
 #' @family token creators
 #' @keywords internal
 validate_new_pos_ids <- function(new_ids, after) {
-  ref <- ifelse(after, floor(new_ids), ceiling(new_ids))
+  ref <- if (after) {
+    floor(new_ids)
+  } else {
+    ceiling(new_ids)
+  }
+
   if (any(abs(new_ids - ref) > 0.5)) abort("too many ids assigned.")
 }
 

R/transform-files.R

@@ -66,7 +66,15 @@ transform_file <- function(path,
   }
   changed <- transform_code(path, fun = fun, ..., dry = dry)
 
-  bullet <- ifelse(is.na(changed), "warning", ifelse(changed, "info", "tick"))
+  bullet <- if (is.na(changed)) {
+    "warning"
+  } else {
+    if (changed) {
+      "info"
+    } else {
+      "tick"
+    }
+  }
 
   if (!getOption("styler.quiet", FALSE)) {
     cli::cat_bullet(bullet = bullet)
@@ -208,7 +216,11 @@ split_roxygen_segments <- function(text, roxygen_examples) {
   active_segment <- as.integer(all_lines %in% roxygen_examples)
   segment_id <- cumsum(abs(c(0L, diff(active_segment)))) + 1L
   separated <- split(text, factor(segment_id))
-  restyle_selector <- ifelse(roxygen_examples[1] == 1L, odd_index, even_index)
+  restyle_selector <- if (roxygen_examples[1] == 1L) {
+    odd_index
+  } else {
+    even_index
+  }
 
   list(separated = separated, selectors = restyle_selector(separated))
 }