diff --git a/NEWS.md b/NEWS.md index b4e4044d8..9dd8cdd18 100644 --- a/NEWS.md +++ b/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) diff --git a/R/brace_linter.R b/R/brace_linter.R index ec9bdf769..2199ee081 100644 --- a/R/brace_linter.R +++ b/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)) { diff --git a/R/paren_brace_linter.R b/R/paren_brace_linter.R index c2523a22e..e7548719e 100644 --- a/R/paren_brace_linter.R +++ b/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) diff --git a/R/zzz.R b/R/zzz.R index 542ab5c9f..0e8062f8c 100644 --- a/R/zzz.R +++ b/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(), diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index f60a646e4..1405e9700 100644 --- a/inst/lintr/linters.csv +++ b/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 diff --git a/man/brace_linter.Rd b/man/brace_linter.Rd index edd5d5004..da1f91e5c 100644 --- a/man/brace_linter.Rd +++ b/man/brace_linter.Rd @@ -15,6 +15,7 @@ Perform various style checks related to placement and spacing of curly braces: \details{ \itemize{ \item Opening curly braces are never on their own line and are always followed by a newline. +\item Opening curly braces have a space before them. \item Closing curly braces are on their own line unless they are followed by an \verb{else}. \item Closing curly braces in \code{if} conditions are on the same line as the corresponding \verb{else}. \item Either both or neither branch in \code{if}/\verb{else} use curly braces, i.e., either both branches use \code{{...}} or neither diff --git a/man/default_linters.Rd b/man/default_linters.Rd index ac904320a..2a0aedfba 100644 --- a/man/default_linters.Rd +++ b/man/default_linters.Rd @@ -5,7 +5,7 @@ \alias{default_linters} \title{Default linters} \format{ -An object of class \code{list} of length 25. +An object of class \code{list} of length 24. } \usage{ default_linters @@ -38,7 +38,6 @@ The following linters are tagged with 'default': \item{\code{\link{object_name_linter}}} \item{\code{\link{object_usage_linter}}} \item{\code{\link{paren_body_linter}}} -\item{\code{\link{paren_brace_linter}}} \item{\code{\link{pipe_continuation_linter}}} \item{\code{\link{semicolon_linter}}} \item{\code{\link{seq_linter}}} diff --git a/man/linters.Rd b/man/linters.Rd index 19adc2a49..e7d36e2f8 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -22,7 +22,7 @@ The following tags exist: \item{\link[=configurable_linters]{configurable} (18 linters)} \item{\link[=consistency_linters]{consistency} (16 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} -\item{\link[=default_linters]{default} (25 linters)} +\item{\link[=default_linters]{default} (24 linters)} \item{\link[=efficiency_linters]{efficiency} (14 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=readability_linters]{readability} (35 linters)} @@ -81,7 +81,7 @@ The following linters exist: \item{\code{\link{outer_negation_linter}} (tags: best_practices, efficiency, readability)} \item{\code{\link{package_hooks_linter}} (tags: correctness, package_development, style)} \item{\code{\link{paren_body_linter}} (tags: default, readability, style)} -\item{\code{\link{paren_brace_linter}} (tags: default, readability, style)} +\item{\code{\link{paren_brace_linter}} (tags: readability, style)} \item{\code{\link{paste_linter}} (tags: best_practices, consistency)} \item{\code{\link{pipe_call_linter}} (tags: readability, style)} \item{\code{\link{pipe_continuation_linter}} (tags: default, readability, style)} diff --git a/man/paren_brace_linter.Rd b/man/paren_brace_linter.Rd index ef8d54b7a..369571b69 100644 --- a/man/paren_brace_linter.Rd +++ b/man/paren_brace_linter.Rd @@ -13,5 +13,5 @@ Check that there is a space between right parentheses and an opening curly brace \link{linters} for a complete list of linters available in lintr. } \section{Tags}{ -\link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +\link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/tests/testthat/test-brace_linter.R b/tests/testthat/test-brace_linter.R index 8e4716ebd..a5e3d79b1 100644 --- a/tests/testthat/test-brace_linter.R +++ b/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() diff --git a/tests/testthat/test-paren_brace_linter.R b/tests/testthat/test-paren_brace_linter.R index 1bce78215..b5058f2a9 100644 --- a/tests/testthat/test-paren_brace_linter.R +++ b/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)