New else_same_line_linter

DESCRIPTION

@@ -66,6 +66,7 @@ Collate:
     'declared_functions.R'
     'deprecated.R'
     'duplicate_argument_linter.R'
+    'else_same_line_linter.R'
     'equals_na_linter.R'
     'exclude.R'
     'expect_comparison_linter.R'

NAMESPACE

@@ -32,6 +32,7 @@ export(default_settings)
 export(default_undesirable_functions)
 export(default_undesirable_operators)
 export(duplicate_argument_linter)
+export(else_same_line_linter)
 export(equals_na_linter)
 export(expect_comparison_linter)
 export(expect_identical_linter)

NEWS.md

@@ -116,6 +116,7 @@ function calls. (#850, #851, @renkun-ken)
    * `literal_coercion_linter()` Require using correctly-typed literals instead of direct coercion, e.g. `1L` instead of `as.numeric(1)`
    * `paste_sep_linter()` Require usage of `paste0()` over `paste(sep = "")`
    * `nested_ifelse_linter()` Prevent nested calls to `ifelse()` like `ifelse(A, x, ifelse(B, y, z))`, and similar
+   * `else_same_line_linter()` Require `else` to come on the same line as the preceding `}`, if present
    * `unreachable_code_linter()` Prevent code after `return()` and `stop()` statements that will never be reached
    * `regex_subset_linter()` Require usage of `grep(ptn, x, value = TRUE)` over `x[grep(ptn, x)]` and similar
    * `consecutive_stopifnot_linter()` Require consecutive calls to `stopifnot()` to be unified into one

R/else_same_line_linter.R

@@ -0,0 +1,31 @@
+#' Require else to come on the same line as \}, if present
+#'
+#' This linter catches `if`/`else` clauses where `if` uses `\{` and its terminal
+#'   `\}` is on a different line than the matched `else`.
+#'
+#' @evalRd rd_tags("else_same_line_linter")
+#' @seealso [linters] for a complete list of linters available in lintr.
+#' @export
+else_same_line_linter <- function() {
+  Linter(function(source_file) {
+    if (length(source_file$xml_parsed_content) == 0L) {
+      return(list())
+    }
+
+    xml <- source_file$xml_parsed_content
+
+    previous_curly_path <- "preceding-sibling::IF/following-sibling::expr[2]/OP-RIGHT-BRACE"
+    # need to (?) repeat previous_curly_path since != will return true if there is
+    #   no such node. ditto for approach with not(@line1 = ...).
+    bad_expr_xpath <- glue::glue("//ELSE[{previous_curly_path} and @line1 != {previous_curly_path}/@line2]")
+    bad_expr <- xml2::xml_find_all(xml, bad_expr_xpath)
+
+    return(lapply(
+      bad_expr,
+      xml_nodes_to_lint,
+      source_file = source_file,
+      lint_message = "`else` should come on the same line as the previous `}`.",
+      type = "warning"
+    ))
+  })
+}

inst/lintr/linters.csv

@@ -12,6 +12,7 @@ conjunct_expectation_linter,package_development best_practices readability
 consecutive_stopifnot_linter,style readability consistency
 cyclocomp_linter,style readability best_practices default configurable
 duplicate_argument_linter,correctness common_mistakes configurable
+else_same_line_linter,style readability
 equals_na_linter,robustness correctness common_mistakes default
 expect_comparison_linter,package_development best_practices
 expect_identical_linter,package_development

tests/testthat/test-else_same_line_linter.R

@@ -0,0 +1,42 @@
+test_that("else_same_line_linter skips allowed usages", {
+  expect_lint("if (TRUE) 1 else 2", NULL, else_same_line_linter())
+  expect_lint("if (TRUE) 1", NULL, else_same_line_linter())
+
+  lines_brace <- trim_some("
+    if (TRUE) {
+      1
+    } else {
+      2
+    }
+  ")
+  expect_lint(lines_brace, NULL, else_same_line_linter())
+
+  # such usage is also not allowed by the style guide, but test anyway
+  lines_unbrace <- trim_some("
+    foo <- function(x) {
+      if (TRUE)
+        1
+      else
+        2
+    }
+  ")
+  expect_lint(lines_unbrace, NULL, else_same_line_linter())
+})
+
+test_that("else_same_line_linter blocks disallowed usage", {
+  lines <- trim_some("
+    foo <- function(x) {
+      if (x) {
+        1
+      }
+      else {
+        2
+      }
+    }
+  ")
+  expect_lint(
+    lines,
+    rex::rex("`else` should come on the same line as the previous `}`."),
+    else_same_line_linter()
+  )
+})