merge paren_brace_linter into brace_linter

NEWS.md

@@ -12,6 +12,7 @@
 * Combined several curly brace related linters into a new `brace_linter` (#1041, @AshesITR):
   + `closed_curly_linter()`
   + `open_curly_linter()`, no longer linting unnecessary trailing whitespace
+  + `paren_brace_linter()`, also linting `if`/`else` and `repeat` with missing whitespace
   + Require `else` to come on the same line as the preceding `}`, if present (#884, @michaelchirico)
   + Require functions spanning multiple lines to use curly braces (@michaelchirico)
   + Require balanced usage of `{}` in `if`/`else` conditions (@michaelchirico)

R/brace_linter.R

@@ -3,6 +3,7 @@
 #' Perform various style checks related to placement and spacing of curly braces:
 #'
 #'  - Opening curly braces are never on their own line and are always followed by a newline.
+#'  - Opening curly braces have a space before them.
 #'  - Closing curly braces are on their own line unless they are followed by an `else`.
 #'  - Closing curly braces in `if` conditions are on the same line as the corresponding `else`.
 #'  - Either both or neither branch in `if`/`else` use curly braces, i.e., either both branches use `{...}` or neither
@@ -54,6 +55,21 @@ brace_linter <- function(allow_single_line = FALSE) {
       )
     ))
 
+    xp_open_preceding <- "parent::expr/preceding-sibling::*[1][self::OP-RIGHT-PAREN or self::ELSE or self::REPEAT]"
+
+    xp_paren_brace <- glue::glue("//OP-LEFT-BRACE[
+      @line1 = { xp_open_preceding }/@line1
+      and
+      @col1 = { xp_open_preceding }/@col2 + 1
+    ]")
+
+    lints <- c(lints, lapply(
+      xml2::xml_find_all(source_expression$xml_parsed_content, xp_paren_brace),
+      xml_nodes_to_lint,
+      source_file = source_expression,
+      lint_message = "There should be a space before an opening curly brace."
+    ))
+
     xp_cond_closed <- xp_and(c(
       # matching { is on same line
       if (isTRUE(allow_single_line)) {

R/paren_brace_linter.R

@@ -6,6 +6,7 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 paren_brace_linter <- function() {
+  lintr_deprecated("paren_brace_linter", new = "brace_linter", version = "2.0.1.9001", type = "Linter")
   Linter(function(source_file) {
     if (is.null(source_file$xml_parsed_content)) {
       return(NULL)

R/zzz.R

@@ -26,7 +26,6 @@ default_linters <- with_defaults(
   object_name_linter(),
   object_usage_linter(),
   paren_body_linter(),
-  paren_brace_linter(),
   pipe_continuation_linter(),
   semicolon_linter(),
   seq_linter(),

inst/lintr/linters.csv

@@ -47,7 +47,7 @@ open_curly_linter,style readability configurable
 outer_negation_linter,readability efficiency best_practices
 package_hooks_linter,style correctness package_development
 paren_body_linter,style readability default
-paren_brace_linter,style readability default
+paren_brace_linter,style readability
 paste_linter,best_practices consistency
 pipe_call_linter,style readability
 pipe_continuation_linter,style readability default

tests/testthat/test-brace_linter.R

@@ -113,6 +113,65 @@ test_that("brace_linter lints braces correctly", {
   )
 })
 
+test_that("brace_linter lints spaces before open braces", {
+  linter <- brace_linter()
+  msg <- rex::rex("There should be a space before an opening curly brace.")
+
+  expect_lint(
+    "blah <- function(){\n}",
+    list(
+      message = msg,
+      column_number = 19L
+    ),
+    linter
+  )
+
+  expect_lint(
+    "\nblah <- function(){\n\n\n}",
+    list(
+      message = msg,
+      column_number = 19L
+    ),
+    linter
+  )
+
+  # should also lint if/else
+  expect_lint(
+    "a <- if (a){\n} else{\n}",
+    list(
+      list(message = msg, line_number = 1L, column_number = 12L),
+      list(message = msg, line_number = 2L, column_number = 7L)
+    ),
+    linter
+  )
+
+  # should lint repeat{
+  expect_lint(
+    "repeat{\nblah\n}",
+    list(message = msg, line_number = 1L, column_number = 7L),
+    linter
+  )
+
+  # should ignore strings and comments, as in regexes:
+  expect_lint("grepl('(iss){2}', 'Mississippi')", NULL, linter)
+  expect_lint(
+    "x <- 123 # dont flag (paren){brace} if inside a comment",
+    NULL,
+    linter
+  )
+  # should not be thrown when the brace lies on subsequent line
+  expect_lint(
+    trim_some("
+      x <- function()
+                     {2}
+    "),
+    list(
+      rex::rex("Opening curly braces should never go on their own line and should always be followed by a new line."),
+      rex::rex("Closing curly-braces should always be on their own line, unless they are followed by an else.")
+    ), #, but not msg
+    linter
+  )
+})
 
 test_that("brace_linter lints else correctly", {
   linter <- brace_linter()

tests/testthat/test-paren_brace_linter.R

@@ -1,5 +1,9 @@
 test_that("returns the correct linting", {
-  linter <- paren_brace_linter()
+  expect_warning(
+    linter <- paren_brace_linter(),
+    "Linter paren_brace_linter was deprecated",
+    fixed = TRUE
+  )
   msg <- rex("There should be a space between right parenthesis and an opening curly brace.")
 
   expect_lint("blah", NULL, linter)