diff --git a/DESCRIPTION b/DESCRIPTION index 825816d2dc..5c6f096dce 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -54,6 +54,7 @@ Collate: 'any_is_na_linter.R' 'assignment_linter.R' 'backport_linter.R' + 'brace_linter.R' 'cache.R' 'class_equals_linter.R' 'closed_curly_linter.R' @@ -67,7 +68,6 @@ 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' @@ -82,11 +82,9 @@ Collate: 'expect_type_linter.R' 'extract.R' 'extraction_operator_linter.R' - 'function_brace_linter.R' 'function_left_parentheses.R' 'get_source_expressions.R' 'ids_with_token.R' - 'if_else_match_braces_linter.R' 'ifelse_censor_linter.R' 'implicit_integer_linter.R' 'infix_spaces_linter.R' diff --git a/NAMESPACE b/NAMESPACE index 907c8c62e4..12d3e025c5 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -19,6 +19,7 @@ export(assignment_linter) export(available_linters) export(available_tags) export(backport_linter) +export(brace_linter) export(checkstyle_output) export(class_equals_linter) export(clear_cache) @@ -34,7 +35,6 @@ 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) @@ -49,11 +49,9 @@ export(expect_s4_class_linter) export(expect_true_false_linter) export(expect_type_linter) export(extraction_operator_linter) -export(function_brace_linter) export(function_left_parentheses_linter) export(get_source_expressions) export(ids_with_token) -export(if_else_match_braces_linter) export(ifelse_censor_linter) export(implicit_integer_linter) export(infix_spaces_linter) diff --git a/NEWS.md b/NEWS.md index 194798948a..61f76e5d7e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,15 @@ * Consistent access to linters through a function call, even for linters without parameters (#245, @fangly, @AshesITR, and @MichaelChirico) * Removed deprecated functions `absolute_paths_linter`, `camel_case_linter`, `multiple_dots_linter`, `snake_case_linter`, and `trailing_semicolons_linter`. They have been marked as deprecated since v1.0.1, which was released in 2017. * Rename `semicolon_terminator_linter` to `semicolon_linter` for better consistency. `semicolon_terminator_linter` survives but is marked for deprecation. The new linter also has a new signature, taking arguments `allow_compound` and `allow_trailing` to replace the old single argument `semicolon=`, again for signature consistency with other linters. +* Combined several curly brace related linters into a new `brace_linter`: + + `closed_curly_linter()`, also allowing `}]` in addition to `})` and `},` as exceptions. +* 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) * The `...` arguments for `lint()`, `lint_dir()`, and `lint_package()` have promoted to an earlier position to better match the [Tidyverse design principal](https://design.tidyverse.org/args-data-details.html) of data->descriptor->details. This change enables passing objects to `...` without needing to specify non-required arguments, e.g. `lint_dir("/path/to/dir", linter())` now works without the need to specify `relative_path`. This affects some code that uses positional arguments. (#935, @michaelchirico) + For `lint()`, `...` is now the 3rd argument, where earlier this was `cache=` + For `lint_dir()` and `lint_package()`, `...` is now the 2nd argument, where earlier this was `relative_path=` @@ -117,7 +126,6 @@ function calls. (#850, #851, @renkun-ken) + Extended for #1067 to exclude `$` extractions like `expect_equal(x$"key", 2)` * `expect_identical_linter()` Require usage of `expect_identical()` by default, and `expect_equal()` only by exception * `expect_comparison_linter()` Require usage of `expect_gt(x, y)` over `expect_true(x > y)` and similar - * `if_else_match_braces_linter()` Require balanced usage of `{}` in `if`/`else` conditions * `vector_logic_linter()` Require use of scalar logical operators (`&&` and `||`) inside `if()` conditions and similar * `any_is_na_linter()` Require usage of `anyNA(x)` over `any(is.na(x))` * `class_equals_linter()` Prevent comparing `class(x)` with `==`, `!=`, or `%in%`, where `inherits()` is typically preferred @@ -131,7 +139,10 @@ function calls. (#850, #851, @renkun-ken) * `nested_ifelse_linter()` Prevent nested calls to `ifelse()` like `ifelse(A, x, ifelse(B, y, z))`, and similar * `condition_message_linter` Prevent condition messages from being constructed like `stop(paste(...))` (where just `stop(...)` is preferable) * `redundant_ifelse_linter()` Prevent usage like `ifelse(A & B, TRUE, FALSE)` or `ifelse(C, 0, 1)` (the latter is `as.numeric(!C)`) - * `else_same_line_linter()` Require `else` to come on the same line as the preceding `}`, if present + * Extensions to `brace_linter()` + + Require `else` to come on the same line as the preceding `}`, if present + + Require balanced usage of `{}` in `if`/`else` conditions + + Require functions spanning multiple lines to use curly braces * `unreachable_code_linter()` Prevent code after `return()` and `stop()` statements that will never be reached (extended for #1051 thanks to early user testing, thanks @bersbersbers!) * `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 diff --git a/R/brace_linter.R b/R/brace_linter.R new file mode 100644 index 0000000000..40ebfa4a93 --- /dev/null +++ b/R/brace_linter.R @@ -0,0 +1,159 @@ +#' Brace linter +#' +#' 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 +#' does. +#' - Functions spanning multiple lines use curly braces. +#' +#' @param allow_single_line if `TRUE`, allow an open and closed curly pair on the same line. +#' +#' @evalRd rd_tags("brace_linter") +#' @seealso [linters] for a complete list of linters available in lintr. \cr +#' \cr +#' +#' @export +brace_linter <- function(allow_single_line = FALSE) { + Linter(function(source_expression) { + if (length(source_expression$xml_parsed_content) == 0L) { + return(list()) + } + + lints <- list() + + xp_cond_open <- xp_and(c( + # matching } is on same line + if (isTRUE(allow_single_line)) { + "(@line1 != following-sibling::OP-LEFT-BRACE/@line1)" + }, + # double curly + "not( + (@line1 = parent::expr/preceding-sibling::OP-LEFT-BRACE/@line1) or + (@line1 = following-sibling::expr/OP-LEFT-BRACE/@line1) + )" + )) + + # TODO (AshesITR): if c_style_braces is TRUE, invert the preceding-sibling condition + xp_open_curly <- glue::glue("//OP-LEFT-BRACE[ + { xp_cond_open } and ( + not(@line1 = parent::expr/preceding-sibling::*/@line2) or + @line1 = following-sibling::*[1][not(self::COMMENT)]/@line1 + ) + ]") + + lints <- c(lints, lapply( + xml2::xml_find_all(source_expression$xml_parsed_content, xp_open_curly), + xml_nodes_to_lint, + source_file = source_expression, + lint_message = paste( + "Opening curly braces should never go on their own line and", + "should always be followed by a new line." + ) + )) + + 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)) { + "(@line1 != preceding-sibling::OP-LEFT-BRACE/@line1)" + }, + # immediately followed by ",", "]" or ")" + "not( + @line1 = ancestor::expr/following-sibling::*[1][ + self::OP-COMMA or self::OP-RIGHT-BRACKET or self::OP-RIGHT-PAREN + ]/@line1 + )", + # double curly + "not( + (@line1 = parent::expr/following-sibling::OP-RIGHT-BRACE/@line1) or + (@line1 = preceding-sibling::expr/OP-RIGHT-BRACE/@line1) + )" + )) + + # TODO (AshesITR): if c_style_braces is TRUE, skip the not(ELSE) condition + xp_closed_curly <- glue::glue("//OP-RIGHT-BRACE[ + { xp_cond_closed } and ( + (@line1 = preceding-sibling::*[1]/@line2) or + (@line1 = parent::expr/following-sibling::*[1][not(self::ELSE)]/@line1) + ) + ]") + + lints <- c(lints, lapply( + xml2::xml_find_all(source_expression$xml_parsed_content, xp_closed_curly), + xml_nodes_to_lint, + source_file = source_expression, + lint_message = paste( + "Closing curly-braces should always be on their own line,", + "unless they are followed by an else." + ) + )) + + xp_else_closed_curly <- "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 = ...). + # TODO (AshesITR): if c_style_braces is TRUE, this needs to be @line2 + 1 + xp_else_same_line <- glue::glue("//ELSE[{xp_else_closed_curly} and @line1 != {xp_else_closed_curly}/@line2]") + + lints <- c(lints, lapply( + xml2::xml_find_all(source_expression$xml_parsed_content, xp_else_same_line), + xml_nodes_to_lint, + source_file = source_expression, + lint_message = "`else` should come on the same line as the previous `}`." + )) + + xp_function_brace <- "//expr[FUNCTION and @line1 != @line2 and not(expr[OP-LEFT-BRACE])]" + + lints <- c(lints, lapply( + xml2::xml_find_all(source_expression$xml_parsed_content, xp_function_brace), + xml_nodes_to_lint, + source_file = source_expression, + lint_message = "Any function spanning multiple lines should use curly braces." + )) + + # if (x) { ... } else if (y) { ... } else { ... } is OK; fully exact pairing + # of if/else would require this to be + # if (x) { ... } else { if (y) { ... } else { ... } } since there's no + # elif operator/token in R, which is pretty unseemly + xp_if_else_match_brace <- " + //IF[ + following-sibling::expr[2][OP-LEFT-BRACE] + and following-sibling::ELSE + /following-sibling::expr[1][not(OP-LEFT-BRACE or IF/following-sibling::expr[2][OP-LEFT-BRACE])] + ] + + | + + //ELSE[ + following-sibling::expr[1][OP-LEFT-BRACE] + and preceding-sibling::IF/following-sibling::expr[2][not(OP-LEFT-BRACE)] + ] + " + + lints <- c(lints, lapply( + xml2::xml_find_all(source_expression$xml_parsed_content, xp_if_else_match_brace), + xml_nodes_to_lint, + source_file = source_expression, + lint_message = "Either both or neither branch in `if`/`else` should use curly braces." + )) + + lints + }) +} diff --git a/R/closed_curly_linter.R b/R/closed_curly_linter.R index bdcca2192a..a6efbff5bb 100644 --- a/R/closed_curly_linter.R +++ b/R/closed_curly_linter.R @@ -9,6 +9,7 @@ #' #' @export closed_curly_linter <- function(allow_single_line = FALSE) { + lintr_deprecated("closed_curly_linter", new = "brace_linter", version = "2.0.1.9001", type = "Linter") Linter(function(source_file) { lapply(ids_with_token(source_file, "'}'"), function(id) { @@ -66,7 +67,8 @@ closed_curly_linter <- function(allow_single_line = FALSE) { "unless they are followed by an else." ), line = source_file$lines[as.character(parsed$line1)] - )} + ) + } } ) }) diff --git a/R/else_same_line_linter.R b/R/else_same_line_linter.R deleted file mode 100644 index bfe409eb0c..0000000000 --- a/R/else_same_line_linter.R +++ /dev/null @@ -1,31 +0,0 @@ -#' 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" - )) - }) -} diff --git a/R/function_brace_linter.R b/R/function_brace_linter.R deleted file mode 100644 index c7f2e213f2..0000000000 --- a/R/function_brace_linter.R +++ /dev/null @@ -1,30 +0,0 @@ -#' Require multi-line functions to use braces -#' -#' This linter catches function definitions spanning multiple lines of code -#' that aren't wrapped in braces -#' -#' @evalRd rd_tags("function_brace_linter") -#' @seealso -#' [linters] for a complete list of linters available in lintr. \cr -#' -#' @export -function_brace_linter <- function() { - Linter(function(source_file) { - if (length(source_file$xml_parsed_content) == 0L) { - return(list()) - } - - xml <- source_file$xml_parsed_content - - bad_expr_xpath <- "//expr[FUNCTION and @line1 != @line2 and not(expr[OP-LEFT-BRACE])]" - bad_expr <- xml2::xml_find_all(xml, bad_expr_xpath) - - return(lapply( - bad_expr, - xml_nodes_to_lint, - source_file = source_file, - lint_message = "Any function spanning multiple lines must use curly braces.", - type = "style" - )) - }) -} diff --git a/R/if_else_match_braces_linter.R b/R/if_else_match_braces_linter.R deleted file mode 100644 index e35132649f..0000000000 --- a/R/if_else_match_braces_linter.R +++ /dev/null @@ -1,48 +0,0 @@ -#' Require both or neither if/else branches to use curly braces -#' -#' This linter catches `if`/`else` clauses where the `if` branch is wrapped -#' in `{...}` but the `else` branch is not, or vice versa, i.e., it ensures -#' that either both branches use `{...}` or neither does. -#' -#' @evalRd rd_tags("if_else_match_braces_linter") -#' @seealso -#' [linters] for a complete list of linters available in lintr. \cr -#' -#' @export -if_else_match_braces_linter <- function() { - Linter(function(source_file) { - if (length(source_file$xml_parsed_content) == 0L) { - return(list()) - } - - xml <- source_file$xml_parsed_content - - # if (x) { ... } else if (y) { ... } else { ... } is OK; fully exact pairing - # of if/else would require this to be - # if (x) { ... } else { if (y) { ... } else { ... } } since there's no - # elif operator/token in R, which is pretty unseemly - xpath <- " - //IF[ - following-sibling::expr[2][OP-LEFT-BRACE] - and following-sibling::ELSE - /following-sibling::expr[1][not(OP-LEFT-BRACE or IF/following-sibling::expr[2][OP-LEFT-BRACE])] - ] - - | - - //ELSE[ - following-sibling::expr[1][OP-LEFT-BRACE] - and preceding-sibling::IF/following-sibling::expr[2][not(OP-LEFT-BRACE)] - ] - " - bad_expr <- xml2::xml_find_all(xml, xpath) - - return(lapply( - bad_expr, - xml_nodes_to_lint, - source_file = source_file, - lint_message = "Either both or neither branch in `if`/`else` should use curly braces.", - type = "warning" - )) - }) -} diff --git a/R/open_curly_linter.R b/R/open_curly_linter.R index 29be743bd5..2350217d99 100644 --- a/R/open_curly_linter.R +++ b/R/open_curly_linter.R @@ -9,6 +9,7 @@ #' #' @export open_curly_linter <- function(allow_single_line = FALSE) { + lintr_deprecated("open_curly_linter", new = "brace_linter", version = "2.0.1.9001", type = "Linter") Linter(function(source_file) { lapply( ids_with_token(source_file, "'{'"), diff --git a/R/paren_brace_linter.R b/R/paren_brace_linter.R index c2523a22e7..e7548719e8 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/semicolon_linter.R b/R/semicolon_linter.R index 9ce7326e32..295e6ff793 100644 --- a/R/semicolon_linter.R +++ b/R/semicolon_linter.R @@ -54,7 +54,7 @@ semicolon_linter <- function(allow_compound = FALSE, allow_trailing = FALSE) { #' } #' @export semicolon_terminator_linter <- function(semicolon = c("compound", "trailing")) { - lintr_deprecated(old = "semicolon_terminator_linter", new = "semicolon_linter", version = "2.0.1.9001") + lintr_deprecated(old = "semicolon_terminator_linter", new = "semicolon_linter", version = "2.0.1.9001", type = "Linter") semicolon <- match.arg(semicolon, several.ok = TRUE) allow_compound <- !"compound" %in% semicolon allow_trailing <- !"trailing" %in% semicolon diff --git a/R/zzz.R b/R/zzz.R index 0d5216dce3..0e8062f8c1 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -13,23 +13,19 @@ default_linters <- with_defaults( default = list(), assignment_linter(), - closed_curly_linter(), + brace_linter(), commas_linter(), commented_code_linter(), cyclocomp_linter(), equals_na_linter(), - function_brace_linter(), function_left_parentheses_linter(), - if_else_match_braces_linter(), infix_spaces_linter(), line_length_linter(), no_tab_linter(), object_length_linter(), object_name_linter(), object_usage_linter(), - open_curly_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 1723312292..1405e97005 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -4,8 +4,9 @@ any_duplicated_linter,efficiency best_practices any_is_na_linter,efficiency best_practices assignment_linter,style consistency default backport_linter,robustness configurable package_development +brace_linter,style readability default configurable class_equals_linter,best_practices robustness consistency -closed_curly_linter,style readability default configurable +closed_curly_linter,style readability configurable commas_linter,style readability default commented_code_linter,style readability best_practices default condition_message_linter,best_practices consistency @@ -13,7 +14,6 @@ conjunct_test_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 @@ -26,9 +26,7 @@ expect_s4_class_linter,package_development best_practices expect_true_false_linter,package_development best_practices readability expect_type_linter,package_development best_practices extraction_operator_linter,style best_practices -function_brace_linter,default style readability function_left_parentheses_linter,style readability default -if_else_match_braces_linter,default style readability ifelse_censor_linter,best_practices efficiency implicit_integer_linter,style consistency best_practices infix_spaces_linter,style readability default @@ -45,11 +43,11 @@ numeric_leading_zero_linter,style consistency readability object_length_linter,style readability default configurable object_name_linter,style consistency default configurable object_usage_linter,style readability correctness default -open_curly_linter,style readability default configurable +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 new file mode 100644 index 0000000000..da1f91e5ce --- /dev/null +++ b/man/brace_linter.Rd @@ -0,0 +1,33 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/brace_linter.R +\name{brace_linter} +\alias{brace_linter} +\title{Brace linter} +\usage{ +brace_linter(allow_single_line = FALSE) +} +\arguments{ +\item{allow_single_line}{if \code{TRUE}, allow an open and closed curly pair on the same line.} +} +\description{ +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 +does. +\item Functions spanning multiple lines use curly braces. +} +} +\seealso{ +\link{linters} for a complete list of linters available in lintr. \cr +\url{https://style.tidyverse.org/syntax.html#indenting} \cr +\url{https://style.tidyverse.org/syntax.html#if-statements} +} +\section{Tags}{ +\link[=configurable_linters]{configurable}, \link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +} diff --git a/man/closed_curly_linter.Rd b/man/closed_curly_linter.Rd index f9007741ad..f4a03083f4 100644 --- a/man/closed_curly_linter.Rd +++ b/man/closed_curly_linter.Rd @@ -17,5 +17,5 @@ Check that closed curly braces are on their own line unless they follow an else. \url{https://style.tidyverse.org/syntax.html#indenting} } \section{Tags}{ -\link[=configurable_linters]{configurable}, \link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +\link[=configurable_linters]{configurable}, \link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/man/configurable_linters.Rd b/man/configurable_linters.Rd index d1a7e76494..4bb47b88c9 100644 --- a/man/configurable_linters.Rd +++ b/man/configurable_linters.Rd @@ -14,6 +14,7 @@ The following linters are tagged with 'configurable': \itemize{ \item{\code{\link{absolute_path_linter}}} \item{\code{\link{backport_linter}}} +\item{\code{\link{brace_linter}}} \item{\code{\link{closed_curly_linter}}} \item{\code{\link{cyclocomp_linter}}} \item{\code{\link{duplicate_argument_linter}}} diff --git a/man/default_linters.Rd b/man/default_linters.Rd index 262b351b09..2a0aedfba1 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 28. +An object of class \code{list} of length 24. } \usage{ default_linters @@ -25,23 +25,19 @@ argument(s), see \verb{?} for details): The following linters are tagged with 'default': \itemize{ \item{\code{\link{assignment_linter}}} -\item{\code{\link{closed_curly_linter}}} +\item{\code{\link{brace_linter}}} \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} \item{\code{\link{cyclocomp_linter}}} \item{\code{\link{equals_na_linter}}} -\item{\code{\link{function_brace_linter}}} \item{\code{\link{function_left_parentheses_linter}}} -\item{\code{\link{if_else_match_braces_linter}}} \item{\code{\link{infix_spaces_linter}}} \item{\code{\link{line_length_linter}}} \item{\code{\link{no_tab_linter}}} \item{\code{\link{object_length_linter}}} \item{\code{\link{object_name_linter}}} \item{\code{\link{object_usage_linter}}} -\item{\code{\link{open_curly_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/else_same_line_linter.Rd b/man/else_same_line_linter.Rd deleted file mode 100644 index cbd0545123..0000000000 --- a/man/else_same_line_linter.Rd +++ /dev/null @@ -1,18 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/else_same_line_linter.R -\name{else_same_line_linter} -\alias{else_same_line_linter} -\title{Require else to come on the same line as \}, if present} -\usage{ -else_same_line_linter() -} -\description{ -This linter catches \code{if}/\verb{else} clauses where \code{if} uses \verb{\\\{} and its terminal -\verb{\\\}} is on a different line than the matched \verb{else}. -} -\seealso{ -\link{linters} for a complete list of linters available in lintr. -} -\section{Tags}{ -\link[=readability_linters]{readability}, \link[=style_linters]{style} -} diff --git a/man/function_brace_linter.Rd b/man/function_brace_linter.Rd deleted file mode 100644 index 033381570a..0000000000 --- a/man/function_brace_linter.Rd +++ /dev/null @@ -1,19 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/function_brace_linter.R -\name{function_brace_linter} -\alias{function_brace_linter} -\title{Require multi-line functions to use braces} -\usage{ -function_brace_linter() -} -\description{ -This linter catches function definitions spanning multiple lines of code -that aren't wrapped in braces -} -\seealso{ -\link{linters} for a complete list of linters available in lintr. \cr -\url{https://style.tidyverse.org/syntax.html#indenting} -} -\section{Tags}{ -\link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} -} diff --git a/man/if_else_match_braces_linter.Rd b/man/if_else_match_braces_linter.Rd deleted file mode 100644 index 7e3a94b918..0000000000 --- a/man/if_else_match_braces_linter.Rd +++ /dev/null @@ -1,20 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/if_else_match_braces_linter.R -\name{if_else_match_braces_linter} -\alias{if_else_match_braces_linter} -\title{Require both or neither if/else branches to use curly braces} -\usage{ -if_else_match_braces_linter() -} -\description{ -This linter catches \code{if}/\verb{else} clauses where the \code{if} branch is wrapped -in \code{{...}} but the \verb{else} branch is not, or vice versa, i.e., it ensures -that either both branches use \code{{...}} or neither does. -} -\seealso{ -\link{linters} for a complete list of linters available in lintr. \cr -\url{https://style.tidyverse.org/syntax.html#if-statements} -} -\section{Tags}{ -\link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} -} diff --git a/man/linters.Rd b/man/linters.Rd index d906992c6e..e7d36e2f8f 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -19,15 +19,15 @@ The following tags exist: \itemize{ \item{\link[=best_practices_linters]{best_practices} (34 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (5 linters)} -\item{\link[=configurable_linters]{configurable} (17 linters)} +\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} (28 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} (37 linters)} +\item{\link[=readability_linters]{readability} (35 linters)} \item{\link[=robustness_linters]{robustness} (12 linters)} -\item{\link[=style_linters]{style} (37 linters)} +\item{\link[=style_linters]{style} (35 linters)} } } \section{Linters}{ @@ -38,8 +38,9 @@ The following linters exist: \item{\code{\link{any_is_na_linter}} (tags: best_practices, efficiency)} \item{\code{\link{assignment_linter}} (tags: consistency, default, style)} \item{\code{\link{backport_linter}} (tags: configurable, package_development, robustness)} +\item{\code{\link{brace_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{class_equals_linter}} (tags: best_practices, consistency, robustness)} -\item{\code{\link{closed_curly_linter}} (tags: configurable, default, readability, style)} +\item{\code{\link{closed_curly_linter}} (tags: configurable, readability, style)} \item{\code{\link{commas_linter}} (tags: default, readability, style)} \item{\code{\link{commented_code_linter}} (tags: best_practices, default, readability, style)} \item{\code{\link{condition_message_linter}} (tags: best_practices, consistency)} @@ -47,7 +48,6 @@ The following linters exist: \item{\code{\link{consecutive_stopifnot_linter}} (tags: consistency, readability, style)} \item{\code{\link{cyclocomp_linter}} (tags: best_practices, configurable, default, readability, style)} \item{\code{\link{duplicate_argument_linter}} (tags: common_mistakes, configurable, correctness)} -\item{\code{\link{else_same_line_linter}} (tags: readability, style)} \item{\code{\link{equals_na_linter}} (tags: common_mistakes, correctness, default, robustness)} \item{\code{\link{expect_comparison_linter}} (tags: best_practices, package_development)} \item{\code{\link{expect_identical_linter}} (tags: package_development)} @@ -60,9 +60,7 @@ The following linters exist: \item{\code{\link{expect_true_false_linter}} (tags: best_practices, package_development, readability)} \item{\code{\link{expect_type_linter}} (tags: best_practices, package_development)} \item{\code{\link{extraction_operator_linter}} (tags: best_practices, style)} -\item{\code{\link{function_brace_linter}} (tags: default, readability, style)} \item{\code{\link{function_left_parentheses_linter}} (tags: default, readability, style)} -\item{\code{\link{if_else_match_braces_linter}} (tags: default, readability, style)} \item{\code{\link{ifelse_censor_linter}} (tags: best_practices, efficiency)} \item{\code{\link{implicit_integer_linter}} (tags: best_practices, consistency, style)} \item{\code{\link{infix_spaces_linter}} (tags: default, readability, style)} @@ -79,11 +77,11 @@ The following linters exist: \item{\code{\link{object_length_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{object_name_linter}} (tags: configurable, consistency, default, style)} \item{\code{\link{object_usage_linter}} (tags: correctness, default, readability, style)} -\item{\code{\link{open_curly_linter}} (tags: configurable, default, readability, style)} +\item{\code{\link{open_curly_linter}} (tags: configurable, readability, style)} \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/open_curly_linter.Rd b/man/open_curly_linter.Rd index 971cb8f990..640eaff0d6 100644 --- a/man/open_curly_linter.Rd +++ b/man/open_curly_linter.Rd @@ -17,5 +17,5 @@ Check that opening curly braces are never on their own line and are always follo \url{https://style.tidyverse.org/syntax.html#indenting} } \section{Tags}{ -\link[=configurable_linters]{configurable}, \link[=default_linters]{default}, \link[=readability_linters]{readability}, \link[=style_linters]{style} +\link[=configurable_linters]{configurable}, \link[=readability_linters]{readability}, \link[=style_linters]{style} } diff --git a/man/paren_brace_linter.Rd b/man/paren_brace_linter.Rd index ef8d54b7a2..369571b692 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/man/readability_linters.Rd b/man/readability_linters.Rd index 1f906e4ceb..91c38b71b1 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -12,20 +12,18 @@ Linters highlighting readability issues, such as missing whitespace. \section{Linters}{ The following linters are tagged with 'readability': \itemize{ +\item{\code{\link{brace_linter}}} \item{\code{\link{closed_curly_linter}}} \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} \item{\code{\link{conjunct_test_linter}}} \item{\code{\link{consecutive_stopifnot_linter}}} \item{\code{\link{cyclocomp_linter}}} -\item{\code{\link{else_same_line_linter}}} \item{\code{\link{expect_length_linter}}} \item{\code{\link{expect_named_linter}}} \item{\code{\link{expect_not_linter}}} \item{\code{\link{expect_true_false_linter}}} -\item{\code{\link{function_brace_linter}}} \item{\code{\link{function_left_parentheses_linter}}} -\item{\code{\link{if_else_match_braces_linter}}} \item{\code{\link{infix_spaces_linter}}} \item{\code{\link{inner_combine_linter}}} \item{\code{\link{line_length_linter}}} diff --git a/man/style_linters.Rd b/man/style_linters.Rd index eb0e974b21..e33d35c18d 100644 --- a/man/style_linters.Rd +++ b/man/style_linters.Rd @@ -13,16 +13,14 @@ Linters highlighting code style issues. The following linters are tagged with 'style': \itemize{ \item{\code{\link{assignment_linter}}} +\item{\code{\link{brace_linter}}} \item{\code{\link{closed_curly_linter}}} \item{\code{\link{commas_linter}}} \item{\code{\link{commented_code_linter}}} \item{\code{\link{consecutive_stopifnot_linter}}} \item{\code{\link{cyclocomp_linter}}} -\item{\code{\link{else_same_line_linter}}} \item{\code{\link{extraction_operator_linter}}} -\item{\code{\link{function_brace_linter}}} \item{\code{\link{function_left_parentheses_linter}}} -\item{\code{\link{if_else_match_braces_linter}}} \item{\code{\link{implicit_integer_linter}}} \item{\code{\link{infix_spaces_linter}}} \item{\code{\link{line_length_linter}}} diff --git a/tests/testthat/default_linter_testcode.R b/tests/testthat/default_linter_testcode.R index ef3a20e636..99f7a3cefb 100644 --- a/tests/testthat/default_linter_testcode.R +++ b/tests/testthat/default_linter_testcode.R @@ -2,7 +2,7 @@ # assignment # function_left_parentheses -# closed_curly +# brace_linter # commas # paren_brace f = function (x,y = 1){} @@ -12,7 +12,7 @@ f = function (x,y = 1){} # cyclocomp # equals_na -# if_else_match_braces_linter +# brace_linter # infix_spaces # line_length # object_length diff --git a/tests/testthat/test-brace_linter.R b/tests/testthat/test-brace_linter.R new file mode 100644 index 0000000000..dff057344f --- /dev/null +++ b/tests/testthat/test-brace_linter.R @@ -0,0 +1,305 @@ +test_that("brace_linter lints braces correctly", { + open_curly_msg <- rex::rex( + "Opening curly braces should never go on their own line and should always be followed by a new line." + ) + closed_curly_msg <- rex::rex(paste( + "Closing curly-braces should always be on their own line,", + "unless they are followed by an else." + )) + + linter <- brace_linter() + expect_lint("blah", NULL, linter) + expect_lint("a <- function() {\n}", NULL, linter) + expect_lint("a <- function() { \n}", NULL, linter) + + expect_lint("a <- function() { 1 }", list(open_curly_msg, closed_curly_msg), linter) + # allowed by allow_single_line + expect_lint("a <- function() { 1 }", NULL, brace_linter(allow_single_line = TRUE)) + + expect_lint( + trim_some(" + a <- if(1) { + 1} else { + 2 + } + "), + closed_curly_msg, + linter + ) + expect_lint( + trim_some(" + a <- if(1) { + 1 + } else { + 2} + "), + closed_curly_msg, + linter + ) + + expect_lint( + trim_some(" + a <- if(1) { + 1} else { + 2} + "), + list( + closed_curly_msg, + closed_curly_msg + ), + linter + ) + + # }) is allowed + expect_lint("eval(bquote({\n...\n}))", NULL, linter) + # }] is too + expect_lint("df[, {\n...\n}]", NULL, linter) + + # }, is allowed + expect_lint( + trim_some(" + fun({ + statements + }, param)"), + NULL, + linter + ) + expect_lint( + trim_some(" + fun(function(a) { + statements + }, param)"), + NULL, + linter + ) + + expect_lint( + trim_some(" + out <- lapply(stuff, function(i) { + do_something(i) + }) %>% unlist + "), + NULL, + linter + ) + + # {{ }} is allowed + expect_lint("{{ x }}", NULL, linter) + + expect_lint( + trim_some(" + pkg_name <- function(path = find_package()) { + if (is.null(path)) { + return(NULL) + } else { + read.dcf(file.path(path, \"DESCRIPTION\"), fields = \"Package\")[1] + } + } + "), + NULL, + linter + ) + + expect_lint("a <- function()\n{\n 1 \n}", open_curly_msg, linter) + expect_lint("a <- function()\n {\n 1 \n}", open_curly_msg, linter) + expect_lint("a <- function()\n\t{\n 1 \n}", open_curly_msg, linter) + + # trailing comments are allowed + expect_lint( + trim_some(' + if ("P" != "NP") { # what most people expect + print("Cryptomania is possible") + } + '), + NULL, + linter + ) +}) + +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() + expect_lint("if (TRUE) 1 else 2", NULL, linter) + expect_lint("if (TRUE) 1", NULL, linter) + + lines_brace <- trim_some(" + if (TRUE) { + 1 + } else { + 2 + } + ") + expect_lint(lines_brace, NULL, 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, linter) + + 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 `}`."), + linter + ) +}) + +test_that("brace_linter lints function expressions correctly", { + linter <- brace_linter() + expect_lint("function(x) 4", NULL, linter) + + lines <- trim_some(" + function(x) { + x + 4 + } + ") + expect_lint(lines, NULL, linter) + + lines <- trim_some(" + function(x) + x+4 + ") + expect_lint( + lines, + rex::rex("Any function spanning multiple lines should use curly braces."), + linter + ) +}) + +test_that("brace_linter lints if/else matching braces correctly", { + linter <- brace_linter() + expect_lint("if (TRUE) 1 else 2", NULL, linter) + expect_lint("if (TRUE) 1", NULL, linter) + + lines_brace <- trim_some(" + if (TRUE) { + 1 + } else { + 2 + } + ") + expect_lint(lines_brace, NULL, 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, linter) + + # else if is OK + lines_else_if <- trim_some(" + if (x) { + 1 + } else if (y) { + 2 + } else { + 3 + } + ") + expect_lint(lines_else_if, NULL, linter) + + lines_if <- trim_some(" + foo <- function(x) { + if (x) { + 1 + } else 2 + } + ") + expect_lint( + lines_if, + rex::rex("Either both or neither branch in `if`/`else` should use curly braces."), + linter + ) + + lines_else <- trim_some(" + foo <- function(x) { + if (x) 1 else { + 2 + } + } + ") + expect_lint( + lines_else, + rex::rex("Either both or neither branch in `if`/`else` should use curly braces."), + linter + ) +}) diff --git a/tests/testthat/test-closed_curly_linter.R b/tests/testthat/test-closed_curly_linter.R index 2c9293086d..d5f73d22fe 100644 --- a/tests/testthat/test-closed_curly_linter.R +++ b/tests/testthat/test-closed_curly_linter.R @@ -5,52 +5,58 @@ test_that("returns the correct linting", { "unless they are followed by an else." )) + expect_warning( + linter <- closed_curly_linter(), + "Linter closed_curly_linter was deprecated", + fixed = TRUE + ) + expect_lint("blah", - NULL, - closed_curly_linter()) + NULL, + linter) expect_lint("a <- function() {\n}", - NULL, - closed_curly_linter()) + NULL, + linter) expect_lint("a <- function() { 1 }", - closed_curly_message_regex, - closed_curly_linter()) + closed_curly_message_regex, + linter) expect_lint("a <- function() { 1 }", - closed_curly_message_regex, - closed_curly_linter()) + closed_curly_message_regex, + linter) expect_lint("a <- function() { 1 }", - NULL, - closed_curly_linter(allow_single_line = TRUE)) + NULL, + suppressWarnings(closed_curly_linter(allow_single_line = TRUE))) expect_lint("a <- if(1) {\n 1} else {\n 2\n}", - closed_curly_message_regex, - closed_curly_linter()) + closed_curly_message_regex, + linter) expect_lint("a <- if(1) {\n 1\n} else {\n 2}", - closed_curly_message_regex, - closed_curly_linter()) + closed_curly_message_regex, + linter) expect_lint("a <- if(1) {\n 1} else {\n 2}", - list( - closed_curly_message_regex, - closed_curly_message_regex - ), - closed_curly_linter()) + list( + closed_curly_message_regex, + closed_curly_message_regex + ), + linter) expect_lint("eval(bquote({...}))", - NULL, - closed_curly_linter()) + NULL, + linter) expect_lint("fun({\n statements\n}, param)", - NULL, - closed_curly_linter()) + NULL, + linter) expect_lint("out <- lapply(stuff, function(i) {\n do_something(i)\n}) %>% unlist", - NULL, - closed_curly_linter()) + NULL, + linter) - expect_lint("{{x}}", NULL, closed_curly_linter()) + expect_lint("{{x}}", NULL, linter) }) diff --git a/tests/testthat/test-else_same_line_linter.R b/tests/testthat/test-else_same_line_linter.R deleted file mode 100644 index 567ba2b0d0..0000000000 --- a/tests/testthat/test-else_same_line_linter.R +++ /dev/null @@ -1,42 +0,0 @@ -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() - ) -}) diff --git a/tests/testthat/test-function_brace_linter.R b/tests/testthat/test-function_brace_linter.R deleted file mode 100644 index e18fb4eb64..0000000000 --- a/tests/testthat/test-function_brace_linter.R +++ /dev/null @@ -1,22 +0,0 @@ -test_that("function_brace_linter skips allowed usages", { - expect_lint("function(x) 4", NULL, function_brace_linter()) - - lines <- trim_some(" - function(x) { - x + 4 - } - ") - expect_lint(lines, NULL, function_brace_linter()) -}) - -test_that("function_brace_linter blocks disallowed usage", { - lines <- trim_some(" - function(x) - x+4 - ") - expect_lint( - lines, - rex::rex("Any function spanning multiple lines must use curly braces."), - function_brace_linter() - ) -}) diff --git a/tests/testthat/test-if_else_match_braces_linter.R b/tests/testthat/test-if_else_match_braces_linter.R deleted file mode 100644 index d52762c608..0000000000 --- a/tests/testthat/test-if_else_match_braces_linter.R +++ /dev/null @@ -1,64 +0,0 @@ -test_that("if_else_match_braces_linter skips allowed usages", { - expect_lint("if (TRUE) 1 else 2", NULL, if_else_match_braces_linter()) - expect_lint("if (TRUE) 1", NULL, if_else_match_braces_linter()) - - lines_brace <- trim_some(" - if (TRUE) { - 1 - } else { - 2 - } - ") - expect_lint(lines_brace, NULL, if_else_match_braces_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, if_else_match_braces_linter()) - - # else if is OK - lines_else_if <- trim_some(" - if (x) { - 1 - } else if (y) { - 2 - } else { - 3 - } - ") - expect_lint(lines_else_if, NULL, if_else_match_braces_linter()) -}) - -test_that("if_else_match_braces_linter blocks disallowed usage", { - lines_if <- trim_some(" - foo <- function(x) { - if (x) { - 1 - } else 2 - } - ") - expect_lint( - lines_if, - rex::rex("Either both or neither branch in `if`/`else` should use curly braces."), - if_else_match_braces_linter() - ) - - lines_else <- trim_some(" - foo <- function(x) { - if (x) 1 else { - 2 - } - } - ") - expect_lint( - lines_else, - rex::rex("Either both or neither branch in `if`/`else` should use curly braces."), - if_else_match_braces_linter() - ) -}) diff --git a/tests/testthat/test-open_curly_linter.R b/tests/testthat/test-open_curly_linter.R index 5e02c3cf49..acd4a969ea 100644 --- a/tests/testthat/test-open_curly_linter.R +++ b/tests/testthat/test-open_curly_linter.R @@ -1,49 +1,56 @@ test_that("returns the correct linting", { + msg <- rex("Opening curly braces should never go on their own line and should always be followed by a new line.") - expect_lint("blah", NULL, open_curly_linter()) + expect_warning( + linter <- open_curly_linter(), + "Linter open_curly_linter was deprecated", + fixed = TRUE + ) + + expect_lint("blah", NULL, linter) - expect_lint("a <- function() {\n}", NULL, open_curly_linter()) + expect_lint("a <- function() {\n}", NULL, linter) expect_lint( -"pkg_name <- function(path = find_package()) { - if (is.null(path)) { - return(NULL) - } else { - read.dcf(file.path(path, \"DESCRIPTION\"), fields = \"Package\")[1] - } -}", NULL, open_curly_linter()) + "pkg_name <- function(path = find_package()) { + if (is.null(path)) { + return(NULL) + } else { + read.dcf(file.path(path, \"DESCRIPTION\"), fields = \"Package\")[1] + } + }", NULL, linter) expect_lint("a <- function()\n{\n 1 \n}", - rex("Opening curly braces should never go on their own line and should always be followed by a new line."), - open_curly_linter()) + msg, + linter) expect_lint("a <- function()\n {\n 1 \n}", - rex("Opening curly braces should never go on their own line and should always be followed by a new line."), - open_curly_linter()) + msg, + linter) expect_lint("a <- function()\n\t{\n 1 \n}", - rex("Opening curly braces should never go on their own line and should always be followed by a new line."), - open_curly_linter()) + msg, + linter) expect_lint("a <- function() { \n}", - rex("Opening curly braces should never go on their own line and should always be followed by a new line."), - open_curly_linter()) + msg, + linter) expect_lint("a <- function() { 1 }", - rex("Opening curly braces should never go on their own line and should always be followed by a new line."), - open_curly_linter()) + msg, + linter) expect_lint("a <- function() { 1 }", NULL, - open_curly_linter(allow_single_line = TRUE)) + suppressWarnings(open_curly_linter(allow_single_line = TRUE))) expect_lint( 'if ("P" != "NP") { # what most people expect print("Cryptomania is possible") }', NULL, - open_curly_linter() +linter ) - expect_lint("{{x}}", NULL, open_curly_linter()) + expect_lint("{{x}}", NULL, linter) }) diff --git a/tests/testthat/test-paren_brace_linter.R b/tests/testthat/test-paren_brace_linter.R index 1bce78215e..b5058f2a9c 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) diff --git a/tests/testthat/test-semicolon_linter.R b/tests/testthat/test-semicolon_linter.R index db4e2d5aca..fb7ab4ab5f 100644 --- a/tests/testthat/test-semicolon_linter.R +++ b/tests/testthat/test-semicolon_linter.R @@ -79,7 +79,7 @@ test_that("Trailing semicolons only", { test_that("deprecation notices for semicolon_terminator_linter succeed, and the deprecated version works", { expect_warning( linter <- semicolon_terminator_linter(), - "Function semicolon_terminator_linter was deprecated", + "Linter semicolon_terminator_linter was deprecated", fixed = TRUE ) expect_lint("a <- 1", NULL, linter) @@ -89,7 +89,7 @@ test_that("deprecation notices for semicolon_terminator_linter succeed, and the # old string argument gets translated to new boolean arguments expect_warning( linter <- semicolon_terminator_linter("compound"), - "Function semicolon_terminator_linter was deprecated", + "Linter semicolon_terminator_linter was deprecated", fixed = TRUE ) expect_lint("a <- 1", NULL, linter) @@ -98,7 +98,7 @@ test_that("deprecation notices for semicolon_terminator_linter succeed, and the expect_warning( linter <- semicolon_terminator_linter("trailing"), - "Function semicolon_terminator_linter was deprecated", + "Linter semicolon_terminator_linter was deprecated", fixed = TRUE ) expect_lint("a <- 1", NULL, linter)