diff --git a/DESCRIPTION b/DESCRIPTION index f16a46112..e69c40e88 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -112,6 +112,7 @@ Collate: 'ifelse_censor_linter.R' 'implicit_assignment_linter.R' 'implicit_integer_linter.R' + 'in_linter.R' 'indentation_linter.R' 'infix_spaces_linter.R' 'inner_combine_linter.R' diff --git a/NAMESPACE b/NAMESPACE index 1b2702104..5eb2897fa 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -78,6 +78,7 @@ export(if_switch_linter) export(ifelse_censor_linter) export(implicit_assignment_linter) export(implicit_integer_linter) +export(in_linter) export(indentation_linter) export(infix_spaces_linter) export(inner_combine_linter) diff --git a/R/in_linter.R b/R/in_linter.R new file mode 100644 index 000000000..cc4ac1d63 --- /dev/null +++ b/R/in_linter.R @@ -0,0 +1,35 @@ +#' Chained equality check linter +#' +#' Report the use of chained equality checks where `%in%` would be more +#' appropriate +#' +#' @examples +#' # lints +#' lint( +#' text = "x == 'a' | x == 'b'", +#' linters = in_linter() +#' ) +#' +#' # This only makes sense in the case x if of length 1 +#' lint( +#' text = "x == 'a' || x == 'b'", +#' linters = in_linter() +#' ) +#' +#' # okay +#' lint( +#' text = "x %in% c('a', 'b')", +#' linters = in_linter() +#' ) +#' +#' @evalRd rd_tags("in_linter") +#' @seealso [linters] for a complete list of linters available in lintr. +#' @export +in_linter <- make_linter_from_xpath( + xpath = "(//OR|//OR2)[ + preceding-sibling::expr[EQ] + and following-sibling::expr[EQ] + and preceding-sibling::expr/expr/SYMBOL/text() = following-sibling::expr/expr/SYMBOL/text() + ]", + lint_message = "Use %in% to test equality of a variable against multiple values." +) diff --git a/inst/lintr/linters.csv b/inst/lintr/linters.csv index 95af98b61..5a5f55310 100644 --- a/inst/lintr/linters.csv +++ b/inst/lintr/linters.csv @@ -42,6 +42,7 @@ if_switch_linter,best_practices readability consistency efficiency configurable ifelse_censor_linter,best_practices efficiency implicit_assignment_linter,style best_practices readability configurable implicit_integer_linter,style consistency best_practices configurable +in_linter,readability efficiency best_practices indentation_linter,style readability default configurable infix_spaces_linter,style readability default configurable inner_combine_linter,efficiency consistency readability diff --git a/man/best_practices_linters.Rd b/man/best_practices_linters.Rd index 9e55cb99e..7d3889b4f 100644 --- a/man/best_practices_linters.Rd +++ b/man/best_practices_linters.Rd @@ -40,6 +40,7 @@ The following linters are tagged with 'best_practices': \item{\code{\link{ifelse_censor_linter}}} \item{\code{\link{implicit_assignment_linter}}} \item{\code{\link{implicit_integer_linter}}} +\item{\code{\link{in_linter}}} \item{\code{\link{is_numeric_linter}}} \item{\code{\link{length_levels_linter}}} \item{\code{\link{lengths_linter}}} diff --git a/man/efficiency_linters.Rd b/man/efficiency_linters.Rd index 37cb8b0d7..d5df5260f 100644 --- a/man/efficiency_linters.Rd +++ b/man/efficiency_linters.Rd @@ -19,6 +19,7 @@ The following linters are tagged with 'efficiency': \item{\code{\link{fixed_regex_linter}}} \item{\code{\link{if_switch_linter}}} \item{\code{\link{ifelse_censor_linter}}} +\item{\code{\link{in_linter}}} \item{\code{\link{inner_combine_linter}}} \item{\code{\link{length_test_linter}}} \item{\code{\link{lengths_linter}}} diff --git a/man/in_linter.Rd b/man/in_linter.Rd new file mode 100644 index 000000000..e4f6281ac --- /dev/null +++ b/man/in_linter.Rd @@ -0,0 +1,38 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/in_linter.R +\name{in_linter} +\alias{in_linter} +\title{Chained equality check linter} +\usage{ +in_linter() +} +\description{ +Report the use of chained equality checks where \code{\%in\%} would be more +appropriate +} +\examples{ +# lints +lint( + text = "x == 'a' | x == 'b'", + linters = in_linter() +) + +# This only makes sense in the case x if of length 1 +lint( + text = "x == 'a' || x == 'b'", + linters = in_linter() +) + +# okay +lint( + text = "x \%in\% c('a', 'b')", + linters = in_linter() +) + +} +\seealso{ +\link{linters} for a complete list of linters available in lintr. +} +\section{Tags}{ +\link[=best_practices_linters]{best_practices}, \link[=efficiency_linters]{efficiency}, \link[=readability_linters]{readability} +} diff --git a/man/linters.Rd b/man/linters.Rd index 394bd6126..affd21d7f 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -17,18 +17,18 @@ see also \code{\link[=available_tags]{available_tags()}}. \section{Tags}{ The following tags exist: \itemize{ -\item{\link[=best_practices_linters]{best_practices} (63 linters)} +\item{\link[=best_practices_linters]{best_practices} (64 linters)} \item{\link[=common_mistakes_linters]{common_mistakes} (11 linters)} \item{\link[=configurable_linters]{configurable} (44 linters)} \item{\link[=consistency_linters]{consistency} (32 linters)} \item{\link[=correctness_linters]{correctness} (7 linters)} \item{\link[=default_linters]{default} (25 linters)} \item{\link[=deprecated_linters]{deprecated} (6 linters)} -\item{\link[=efficiency_linters]{efficiency} (32 linters)} +\item{\link[=efficiency_linters]{efficiency} (33 linters)} \item{\link[=executing_linters]{executing} (6 linters)} \item{\link[=package_development_linters]{package_development} (14 linters)} \item{\link[=pkg_testthat_linters]{pkg_testthat} (12 linters)} -\item{\link[=readability_linters]{readability} (64 linters)} +\item{\link[=readability_linters]{readability} (65 linters)} \item{\link[=regex_linters]{regex} (4 linters)} \item{\link[=robustness_linters]{robustness} (17 linters)} \item{\link[=style_linters]{style} (40 linters)} @@ -78,6 +78,7 @@ The following linters exist: \item{\code{\link{ifelse_censor_linter}} (tags: best_practices, efficiency)} \item{\code{\link{implicit_assignment_linter}} (tags: best_practices, configurable, readability, style)} \item{\code{\link{implicit_integer_linter}} (tags: best_practices, configurable, consistency, style)} +\item{\code{\link{in_linter}} (tags: best_practices, efficiency, readability)} \item{\code{\link{indentation_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{infix_spaces_linter}} (tags: configurable, default, readability, style)} \item{\code{\link{inner_combine_linter}} (tags: consistency, efficiency, readability)} diff --git a/man/readability_linters.Rd b/man/readability_linters.Rd index 372d2fd9e..6f16b965d 100644 --- a/man/readability_linters.Rd +++ b/man/readability_linters.Rd @@ -33,6 +33,7 @@ The following linters are tagged with 'readability': \item{\code{\link{if_not_else_linter}}} \item{\code{\link{if_switch_linter}}} \item{\code{\link{implicit_assignment_linter}}} +\item{\code{\link{in_linter}}} \item{\code{\link{indentation_linter}}} \item{\code{\link{infix_spaces_linter}}} \item{\code{\link{inner_combine_linter}}} diff --git a/tests/testthat/test-in_linter.R b/tests/testthat/test-in_linter.R new file mode 100644 index 000000000..87be6b507 --- /dev/null +++ b/tests/testthat/test-in_linter.R @@ -0,0 +1,54 @@ +test_that("in_linter catches unnecessary OR chains", { + linter <- in_linter() + lint_msg <- rex::rex("%in%") + + expect_lint( + "x == 'a' | x == 'b'", + lint_msg, + linter + ) + + # Works with short-circuit operator + expect_lint( + "x == 'a' || x == 'b'", + lint_msg, + linter + ) + + # Works with yoda tests + expect_lint( + "'a' == x || 'b' == x", + lint_msg, + linter + ) + + # Works with 'semi-yoda' tests + expect_lint( + "x == 'a' || 'b' == x", + lint_msg, + linter + ) + + # Works with longer chains + expect_lint( + "x == 'a' | x == 'b' | x == 'c'", + lint_msg, + linter + ) +}) + +test_that("common in_linter negative cases", { + linter <- in_linter() + + expect_lint( + "x == 'a' | y == 'b'", + NULL, + linter + ) + + expect_lint( + "x == 'a' || y == 'b'", + NULL, + linter + ) +})