diff --git a/DESCRIPTION b/DESCRIPTION index 7e3899917..917946399 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -149,6 +149,7 @@ Collate: 'redundant_equals_linter.R' 'redundant_ifelse_linter.R' 'regex_subset_linter.R' + 'repeat_linter.R' 'routine_registration_linter.R' 'scalar_in_linter.R' 'semicolon_linter.R' diff --git a/NAMESPACE b/NAMESPACE index 6c63fae68..6469aa96e 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -113,6 +113,7 @@ export(quotes_linter) export(redundant_equals_linter) export(redundant_ifelse_linter) export(regex_subset_linter) +export(repeat_linter) export(routine_registration_linter) export(sarif_output) export(scalar_in_linter) diff --git a/NEWS.md b/NEWS.md index f6c5cbd7f..be1c847cd 100644 --- a/NEWS.md +++ b/NEWS.md @@ -34,6 +34,7 @@ * `seq_linter()` recommends `rev()` in the lint message for lints like `nrow(x):1` (#1542, @MichaelChirico). * New `xp_call_name()` helper to facilitate writing custom linters (#2023, @MichaelChirico). This helper converts a matched XPath to the R function to which it corresponds. This is useful for including the "offending" function in the lint's message. * `function_argument_linter()` detects usage of `missing()` for the linted argument (#1546, @MichaelChirico). The simplest fix for `function_argument_linter()` lints is typically to set that argument to `NULL` by default, in which case it's usually preferable to update function logic checking `missing()` to check `is.null()` instead. +* `commas_linter()` gains an option `allow_trailing` (default `FALSE`) to allow trailing commas while indexing. (#2104, @MEO265) ### New linters @@ -42,7 +43,7 @@ * `length_levels_linter()` for using the specific function `nlevels()` instead of checking `length(levels(x))` (part of #884, @MichaelChirico). * `scalar_in_linter()` for discouraging `%in%` when the right-hand side is a scalar, e.g. `x %in% 1` (part of #884, @MichaelChirico). * `if_not_else_linter()` for encouraging `if` statements to be structured as `if (A) x else y` instead of `if (!A) y else x` (part of #884, @MichaelChirico). -* `commas_linter()` gains an option `allow_trailing` (default `FALSE`) to allow trailing commas while indexing. (#2104, @MEO265) +* `repeat_linter()` for encouraging `repeat` for infinite loops instead of `while (TRUE)` (#2106, @MEO265). ## Changes to defaults diff --git a/R/repeat_linter.R b/R/repeat_linter.R new file mode 100644 index 000000000..fd10d5cbc --- /dev/null +++ b/R/repeat_linter.R @@ -0,0 +1,41 @@ +#' Repeat linter +#' +#' Check that `while (TRUE)` is not used for infinite loops. +#' +#' @examples +#' # will produce lints +#' lint( +#' text = "while (TRUE) { }", +#' linters = repeat_linter() +#' ) +#' +#' +#' # okay +#' lint( +#' text = "repeat { }", +#' linters = repeat_linter() +#' ) +#' +#' +#' @evalRd rd_tags("repeat_linter") +#' @seealso [linters] for a complete list of linters available in lintr. +#' @export +repeat_linter <- function() { + xpath <- "//WHILE[following-sibling::expr[1]/NUM_CONST[text() = 'TRUE']]" + + Linter(function(source_expression) { + if (!is_lint_level(source_expression, "expression")) { + return(list()) + } + xml <- source_expression$xml_parsed_content + lints <- xml_find_all(xml, xpath) + + xml_nodes_to_lints( + lints, + source_expression = source_expression, + lint_message = "Use 'repeat' instead of 'while (TRUE)' for infinite loops.", + range_start_xpath = "number(./@col1)", + range_end_xpath = "number(./following-sibling::*[3]/@col2)" + ) + }) +} diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index 39521e81a..e4bda43a6 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -71,6 +71,7 @@ quotes_linter,style consistency readability default configurable redundant_equals_linter,best_practices readability efficiency common_mistakes redundant_ifelse_linter,best_practices efficiency consistency configurable regex_subset_linter,best_practices efficiency +repeat_linter,style readability routine_registration_linter,best_practices efficiency robustness scalar_in_linter,readability consistency best_practices efficiency semicolon_linter,style readability default configurable diff --git a/man/conjunct_test_linter.Rd b/man/conjunct_test_linter.Rd index 308e11b64..dd24799f2 100644 --- a/man/conjunct_test_linter.Rd +++ b/man/conjunct_test_linter.Rd @@ -44,10 +44,7 @@ lint( ) lint( - text = "dplyr::filter( - mtcars, - mpg > 20 & vs == 0 - )", + text = "dplyr::filter(mtcars, mpg > 20 & vs == 0)", linters = conjunct_test_linter() ) @@ -63,10 +60,7 @@ lint( ) lint( - text = "dplyr::filter( - mtcars, - mpg > 20 & vs == 0 - )", + text = "dplyr::filter(mtcars, mpg > 20 & vs == 0)", linters = conjunct_test_linter(allow_filter = TRUE) ) diff --git a/man/linters.Rd b/man/linters.Rd index 4b96359e2..e5bcd6aed 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -28,9 +28,9 @@ The following tags exist: \item{\link[=executing_linters]{executing} (5 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=pkg_testthat_linters]{pkg_testthat} (12 linters)} -\item{\link[=readability_linters]{readability} (52 linters)} +\item{\link[=readability_linters]{readability} (53 linters)} \item{\link[=robustness_linters]{robustness} (14 linters)} -\item{\link[=style_linters]{style} (36 linters)} +\item{\link[=style_linters]{style} (37 linters)} } } \section{Linters}{ @@ -103,6 +103,7 @@ The following linters exist: \item{\code{\link{redundant_equals_linter}} (tags: best_practices, common_mistakes, efficiency, readability)} \item{\code{\link{redundant_ifelse_linter}} (tags: best_practices, configurable, consistency, efficiency)} \item{\code{\link{regex_subset_linter}} (tags: best_practices, efficiency)} +\item{\code{\link{repeat_linter}} (tags: readability, style)} \item{\code{\link{routine_registration_linter}} (tags: best_practices, efficiency, robustness)} \item{\code{\link{scalar_in_linter}} (tags: best_practices, consistency, efficiency, readability)} \item{\code{\link{semicolon_linter}} (tags: configurable, default, readability, style)} diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index a3ff8f846..2b3d18f96 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -50,6 +50,7 @@ The following linters are tagged with 'readability': \item{\code{\link{pipe_continuation_linter}}} \item{\code{\link{quotes_linter}}} \item{\code{\link{redundant_equals_linter}}} +\item{\code{\link{repeat_linter}}} \item{\code{\link{scalar_in_linter}}} \item{\code{\link{semicolon_linter}}} \item{\code{\link{sort_linter}}} diff --git a/man/repeat_linter.Rd b/man/repeat_linter.Rd new file mode 100644 index 000000000..dd3100115 --- /dev/null +++ b/man/repeat_linter.Rd @@ -0,0 +1,33 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/repeat_linter.R +\name{repeat_linter} +\alias{repeat_linter} +\title{Repeat linter} +\usage{ +repeat_linter() +} +\description{ +Check that \verb{while (TRUE)} is not used for infinite loops. +} +\examples{ +# will produce lints +lint( + text = "while (TRUE) { }", + linters = repeat_linter() +) + + +# okay +lint( + text = "repeat { }", + linters = repeat_linter() +) + + +} +\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/style_linters.Rd b/man/style_linters.Rd index 1c8337301..fba8f2799 100644 --- a/man/style_linters.Rd +++ b/man/style_linters.Rd @@ -37,6 +37,7 @@ The following linters are tagged with 'style': \item{\code{\link{pipe_call_linter}}} \item{\code{\link{pipe_continuation_linter}}} \item{\code{\link{quotes_linter}}} +\item{\code{\link{repeat_linter}}} \item{\code{\link{semicolon_linter}}} \item{\code{\link{spaces_inside_linter}}} \item{\code{\link{spaces_left_parentheses_linter}}} diff --git a/tests/testthat/test-repeat_linter.R b/tests/testthat/test-repeat_linter.R new file mode 100644 index 000000000..9c3cb16ee --- /dev/null +++ b/tests/testthat/test-repeat_linter.R @@ -0,0 +1,27 @@ +test_that("test repeat_linter", { + linter <- repeat_linter() + msg <- rex::rex("Use 'repeat' instead of 'while (TRUE)' for infinite loops.") + + expect_lint("repeat { }", NULL, linter) + expect_lint("while (FALSE) { }", NULL, linter) + expect_lint("while (i < 5) { }", NULL, linter) + expect_lint("while (j < 5) TRUE", NULL, linter) + expect_lint("while (TRUE && j < 5) { ... }", NULL, linter) + + expect_lint("while (TRUE) { }", msg, linter) + expect_lint("for (i in 1:10) { while (TRUE) { if (i == 5) { break } } }", msg, linter) + expect_lint("while (TRUE) { while (TRUE) { } }", list(msg, msg), linter) + expect_lint( + trim_some("{ + while (TRUE) { + } + while (TRUE) { + } + }"), + list( + list(message = msg, line_number = 2L, column_number = 3L, ranges = list(c(3L, 14L))), + list(message = msg, line_number = 4L, column_number = 3L, ranges = list(c(3L, 14L))) + ), + linter + ) +})