Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New linter for complex conditional expressions #2676

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Collate:
'commas_linter.R'
'commented_code_linter.R'
'comparison_negation_linter.R'
'complex_conditional_linter.R'
'condition_call_linter.R'
'condition_message_linter.R'
'conjunct_test_linter.R'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export(closed_curly_linter)
export(commas_linter)
export(commented_code_linter)
export(comparison_negation_linter)
export(complex_conditional_linter)
export(condition_call_linter)
export(condition_message_linter)
export(conjunct_test_linter)
Expand Down
105 changes: 105 additions & 0 deletions R/complex_conditional_linter.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
#' Complex Conditional Expressions Linter
#'
#' Detects complex conditional expressions and suggests extracting
#' them into Boolean functions or variables for improved readability and reusability.
#'
#' For example, if you have a conditional expression with more than two logical operands,
#'
#' ```
#' if (looks_like_a_duck(x) &&
#' swims_like_a_duck(x) &&
#' quacks_like_a_duck(x)) {
#' ...
#' }
#' ````
#'
#' to improve its readability and reusability, you can extract the conditional expression.
#'
#' You can either extract it into a Boolean function:
#'
#' ```
#' is_duck <- function(x) {
#' looks_like_a_duck(x) &&
#' swims_like_a_duck(x) &&
#' quacks_like_a_duck(x)
#' }
#'
#' if (is_duck(x)) {
#' ...
#' }
#' ```
#'
#' or into a Boolean variable:
#'
#' ```
#' is_duck <- looks_like_a_duck(x) &&
#' swims_like_a_duck(x) &&
#' quacks_like_a_duck(x)
#'
#' if (is_duck) {
#' ...
#' }
#' ```
#'
#' In addition to improving code readability, extracting complex conditional expressions
#' has the added benefit of introducing a reusable abstraction.
#'
#' @param threshold Integer. The maximum number of logical operators (`&&` or `||`)
#' allowed in a conditional expression. The default is `1L`, meaning any conditional expression
#' with more than one logical operator will be flagged.
IndrajeetPatil marked this conversation as resolved.
Show resolved Hide resolved
#'
#' @examples
#' # will produce lints
#' code <- "if (a && b && c) { do_something() }"
#' writeLines(code)
#' lint(
#' text = code,
#' linters = complex_conditional_linter()
#' )
#'
#' # okay
#' ready_to_do_something <- a && b && c
#' code <- "if (ready_to_do_something) { do_something() }"
#' writeLines(code)
#' lint(
#' text = code,
#' linters = complex_conditional_linter()
#' )
#'
#' @evalRd rd_tags("complex_conditional_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
complex_conditional_linter <- function(threshold = 1L) {
stopifnot(is.numeric(threshold), length(threshold) == 1L, threshold >= 1L)
threshold <- as.integer(threshold)

xpath <- glue::glue("//expr[
parent::expr[IF or WHILE]
and
preceding-sibling::*[1][self::OP-LEFT-PAREN]
and
following-sibling::*[1][self::OP-RIGHT-PAREN]
and
count(descendant-or-self::*[AND2 or OR2]) > {threshold}
]")


Linter(linter_level = "expression", function(source_expression) {
xml <- source_expression$xml_parsed_content

nodes <- xml2::xml_find_all(xml, xpath)

lints <- xml_nodes_to_lints(
nodes,
source_expression = source_expression,
lint_message = paste0(
"Complex conditional with more than ",
threshold,
" logical operator(s). Consider extracting into a boolean function or variable for readability and reusability."
),
type = "warning"
)

lints
})
}
2 changes: 1 addition & 1 deletion R/unnecessary_nesting_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ unnecessary_nesting_linter <- function(
unnecessary_else_brace_lints <- xml_nodes_to_lints(
unnecessary_else_brace_expr,
source_expression = source_expression,
lint_message = "Simplify this condition by using 'else if' instead of 'else { if.",
lint_message = "Simplify this condition by using 'else if' instead of 'else { if'.",
type = "warning"
)

Expand Down
1 change: 1 addition & 0 deletions inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ closed_curly_linter,defunct
commas_linter,style readability default configurable
commented_code_linter,style readability best_practices default
comparison_negation_linter,readability consistency
complex_conditional_linter,style readability best_practices configurable
condition_call_linter,style tidy_design best_practices configurable
condition_message_linter,best_practices consistency
conjunct_test_linter,package_development best_practices readability configurable pkg_testthat
Expand Down
1 change: 1 addition & 0 deletions man/best_practices_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

81 changes: 81 additions & 0 deletions man/complex_conditional_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/configurable_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions man/linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/readability_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions man/style_linters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading