diff --git a/DESCRIPTION b/DESCRIPTION index 252af9701..be441700a 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -58,9 +58,11 @@ Encoding: UTF-8 Roxygen: list(markdown = TRUE) RoxygenNote: 7.2.3 Collate: - 'T_and_F_symbol_linter.R' + 'make_linter_from_xpath.R' + 'xp_utils.R' 'utils.R' - 'aaa.R' + 'AAA.R' + 'T_and_F_symbol_linter.R' 'absolute_path_linter.R' 'actions.R' 'addins.R' @@ -126,7 +128,6 @@ Collate: 'lintr-package.R' 'literal_coercion_linter.R' 'make_linter_from_regex.R' - 'make_linter_from_xpath.R' 'matrix_apply_linter.R' 'methods.R' 'missing_argument_linter.R' @@ -182,7 +183,6 @@ Collate: 'with.R' 'with_id.R' 'xml_nodes_to_lints.R' - 'xp_utils.R' 'yoda_test_linter.R' 'zzz.R' Language: en-US diff --git a/R/aaa.R b/R/AAA.R similarity index 93% rename from R/aaa.R rename to R/AAA.R index ed1f08ea1..d9fd7d886 100644 --- a/R/aaa.R +++ b/R/AAA.R @@ -1,4 +1,6 @@ #' @include utils.R +#' @include xp_utils.R +#' @include make_linter_from_xpath.R NULL #' Available linters diff --git a/R/empty_assignment_linter.R b/R/empty_assignment_linter.R index fbb2338ef..2ea602763 100644 --- a/R/empty_assignment_linter.R +++ b/R/empty_assignment_linter.R @@ -30,9 +30,9 @@ #' @evalRd rd_tags("empty_assignment_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -empty_assignment_linter <- function() { +empty_assignment_linter <- make_linter_from_xpath( # for some reason, the parent in the `=` case is , not , hence parent::expr - xpath <- " + xpath = " //OP-LEFT-BRACE[following-sibling::*[1][self::OP-RIGHT-BRACE]] /parent::expr[ preceding-sibling::LEFT_ASSIGN @@ -40,23 +40,6 @@ empty_assignment_linter <- function() { or following-sibling::RIGHT_ASSIGN ] /parent::* - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = - "Assign NULL explicitly or, whenever possible, allocate the empty object with the right type and size.", - type = "warning" - ) - }) -} + ", + lint_message = "Assign NULL explicitly or, whenever possible, allocate the empty object with the right type and size." +) diff --git a/R/expect_not_linter.R b/R/expect_not_linter.R index 22826ceb7..5c68b80a2 100644 --- a/R/expect_not_linter.R +++ b/R/expect_not_linter.R @@ -22,28 +22,12 @@ #' @evalRd rd_tags("expect_not_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -expect_not_linter <- function() { - xpath <- " +expect_not_linter <- make_linter_from_xpath( + xpath = " //SYMBOL_FUNCTION_CALL[text() = 'expect_true' or text() = 'expect_false'] /parent::expr /following-sibling::expr[OP-EXCLAMATION] /parent::expr - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "expect_false(x) is better than expect_true(!x), and vice versa.", - type = "warning" - ) - }) -} + ", + lint_message = "expect_false(x) is better than expect_true(!x), and vice versa." +) diff --git a/R/for_loop_index_linter.R b/R/for_loop_index_linter.R index 154a2cf9f..244dc98af 100644 --- a/R/for_loop_index_linter.R +++ b/R/for_loop_index_linter.R @@ -29,30 +29,14 @@ #' @evalRd rd_tags("for_loop_index_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -for_loop_index_linter <- function() { - xpath <- " +for_loop_index_linter <- make_linter_from_xpath( + xpath = " //forcond /SYMBOL[text() = following-sibling::expr //SYMBOL[not(parent::expr[OP-DOLLAR or OP-AT or preceding-sibling::OP-LEFT-BRACKET])] /text() ] - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "Don't re-use any sequence symbols as the index symbol in a for loop.", - type = "warning" - ) - }) -} + ", + lint_message = "Don't re-use any sequence symbols as the index symbol in a for loop." +) diff --git a/R/function_return_linter.R b/R/function_return_linter.R index 252cca808..acd95af4d 100644 --- a/R/function_return_linter.R +++ b/R/function_return_linter.R @@ -55,26 +55,10 @@ #' @evalRd rd_tags("function_return_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -function_return_linter <- function() { - xpath <- " +function_return_linter <- make_linter_from_xpath( + xpath = " //SYMBOL_FUNCTION_CALL[text() = 'return'] /parent::expr/parent::expr/expr[LEFT_ASSIGN or RIGHT_ASSIGN] - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "Move the assignment outside of the return() clause, or skip assignment altogether.", - type = "warning" - ) - }) -} + ", + lint_message = "Move the assignment outside of the return() clause, or skip assignment altogether." +) diff --git a/R/length_levels_linter.R b/R/length_levels_linter.R index 58fc63538..f4c2165ba 100644 --- a/R/length_levels_linter.R +++ b/R/length_levels_linter.R @@ -18,28 +18,12 @@ #' @evalRd rd_tags("length_levels_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -length_levels_linter <- function() { - xpath <- " +length_levels_linter <- make_linter_from_xpath( + xpath = " //SYMBOL_FUNCTION_CALL[text() = 'levels'] /parent::expr /parent::expr /parent::expr[expr/SYMBOL_FUNCTION_CALL[text() = 'length']] - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "nlevels(x) is better than length(levels(x)).", - type = "warning" - ) - }) -} + ", + lint_message = "nlevels(x) is better than length(levels(x))." +) diff --git a/R/lengths_linter.R b/R/lengths_linter.R index 48a04230a..69ea9eaa7 100644 --- a/R/lengths_linter.R +++ b/R/lengths_linter.R @@ -30,28 +30,14 @@ #' @evalRd rd_tags("lengths_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -lengths_linter <- function() { +lengths_linter <- local({ loop_funs <- c("sapply", "vapply", "map_int", "map_dbl") - xpath <- glue(" - //SYMBOL_FUNCTION_CALL[ {xp_text_in_table(loop_funs)} ] - /parent::expr - /parent::expr[expr/SYMBOL[text() = 'length']] - ") - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "Use lengths() to find the length of each element in a list.", - type = "warning" - ) - }) -} + make_linter_from_xpath( + xpath = glue(" + //SYMBOL_FUNCTION_CALL[ {xp_text_in_table(loop_funs)} ] + /parent::expr + /parent::expr[expr/SYMBOL[text() = 'length']] + "), + lint_message = "Use lengths() to find the length of each element in a list." + ) +}) diff --git a/R/numeric_leading_zero_linter.R b/R/numeric_leading_zero_linter.R index c7b755802..ef281d02e 100644 --- a/R/numeric_leading_zero_linter.R +++ b/R/numeric_leading_zero_linter.R @@ -29,28 +29,12 @@ #' @evalRd rd_tags("numeric_leading_zero_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -numeric_leading_zero_linter <- function() { +numeric_leading_zero_linter <- make_linter_from_xpath( # NB: # 1. negative constants are split to two components: # OP-MINUS, NUM_CONST # 2. complex constants are split to three components: # NUM_CONST, OP-PLUS/OP-MINUS, NUM_CONST - xpath <- "//NUM_CONST[starts-with(text(), '.')]" - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "Include the leading zero for fractional numeric constants.", - type = "warning" - ) - }) -} + xpath = "//NUM_CONST[starts-with(text(), '.')]", + lint_message = "Include the leading zero for fractional numeric constants." +) diff --git a/R/paren_body_linter.R b/R/paren_body_linter.R index 1fdccfdc1..083f5fe1c 100644 --- a/R/paren_body_linter.R +++ b/R/paren_body_linter.R @@ -21,13 +21,13 @@ #' - [linters] for a complete list of linters available in lintr. #' - #' @export -paren_body_linter <- function() { +paren_body_linter <- make_linter_from_xpath( # careful to do recursive search to the less common OP-RIGHT-PAREN # and forcond nodes (vs. //expr) for performance -- there can # be O(100K) nodes but in all but pathological examples, # these other nodes will only be a small fraction of this amount. # note also that only has one following-sibling::expr. - xpath <- " + xpath = " //OP-RIGHT-PAREN[ @end = following-sibling::expr[1]/@start - 1 and @line1 = following-sibling::expr[1]/@line1 @@ -45,20 +45,7 @@ paren_body_linter <- function() { and OP-RIGHT-PAREN/@col1 = following-sibling::expr/@col1 - 1 ] /following-sibling::expr - " - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - matched_expressions <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - matched_expressions, - source_expression = source_expression, - lint_message = "There should be a space between a right parenthesis and a body expression." - ) - }) -} + ", + lint_message = "There should be a space between a right parenthesis and a body expression.", + type = "style" +) diff --git a/R/routine_registration_linter.R b/R/routine_registration_linter.R index 6ff5769a6..1d0fe2e47 100644 --- a/R/routine_registration_linter.R +++ b/R/routine_registration_linter.R @@ -31,29 +31,15 @@ #' - #' #' @export -routine_registration_linter <- function() { +routine_registration_linter <- local({ native_routine_callers <- c(".C", ".Call", ".Fortran", ".External") - xpath <- glue(" - //SYMBOL_FUNCTION_CALL[ {xp_text_in_table(native_routine_callers)} ] - /parent::expr - /following-sibling::expr[1]/STR_CONST - /parent::expr - ") - - Linter(function(source_expression) { - if (!is_lint_level(source_expression, "expression")) { - return(list()) - } - - xml <- source_expression$xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = "Register your native code routines with useDynLib and R_registerRoutines().", - type = "warning" - ) - }) -} + make_linter_from_xpath( + xpath = glue(" + //SYMBOL_FUNCTION_CALL[ {xp_text_in_table(native_routine_callers)} ] + /parent::expr + /following-sibling::expr[1]/STR_CONST + /parent::expr + "), + lint_message = "Register your native code routines with useDynLib and R_registerRoutines()." + ) +}) diff --git a/R/unnecessary_nested_if_linter.R b/R/unnecessary_nested_if_linter.R index 354b167a3..5702fc7d7 100644 --- a/R/unnecessary_nested_if_linter.R +++ b/R/unnecessary_nested_if_linter.R @@ -24,8 +24,8 @@ #' @evalRd rd_tags("unnecessary_nested_if_linter") #' @seealso [linters] for a complete list of linters available in lintr. #' @export -unnecessary_nested_if_linter <- function() { - xpath <- paste0( +unnecessary_nested_if_linter <- make_linter_from_xpath( + xpath = paste0( "//IF/parent::expr[not(ELSE)]/OP-RIGHT-PAREN/", c( "following-sibling::expr[IF and not(ELSE)]", # catch if (cond) if (other_cond) { ... } @@ -33,29 +33,12 @@ unnecessary_nested_if_linter <- function() { /expr[IF and not(ELSE)]" # catch if (cond) { if (other_cond) { ... } } ), collapse = " | " - ) - - Linter(function(source_expression) { - # need the full file to also catch usages at the top level - if (!is_lint_level(source_expression, "file")) { - return(list()) - } - - xml <- source_expression$full_xml_parsed_content - - bad_expr <- xml_find_all(xml, xpath) - - lint_message <- paste( - "Don't use nested `if` statements,", - "where a single `if` with the combined conditional expression will do.", - "For example, instead of `if (x) { if (y) { ... }}`, use `if (x && y) { ... }`." - ) - - xml_nodes_to_lints( - bad_expr, - source_expression = source_expression, - lint_message = lint_message, - type = "warning" - ) - }) -} + ), + lint_message = paste( + "Don't use nested `if` statements,", + "where a single `if` with the combined conditional expression will do.", + "For example, instead of `if (x) { if (y) { ... }}`, use `if (x && y) { ... }`." + ), + # need the full file to also catch usages at the top level + level = "file" +) diff --git a/man/linters.Rd b/man/linters.Rd index e5bcd6aed..cbb4f8d03 100644 --- a/man/linters.Rd +++ b/man/linters.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/aaa.R +% Please edit documentation in R/AAA.R \name{linters} \alias{linters} \title{Available linters}