diff --git a/.lintr b/.lintr index a9af68366..31ebf6feb 100644 --- a/.lintr +++ b/.lintr @@ -2,6 +2,20 @@ linters: all_linters( backport_linter("3.6.0", except = c("R_user_dir", "deparse1", "...names")), line_length_linter(120L), object_overwrite_linter(allow_names = c("line", "lines", "pipe", "symbols")), + todo_comment_linter( + except_regex = rex::rex( + "TODO(", + group(or( + # GitHub issue number #1234, possibly from another repo org/repo#5678 + list(maybe(one_or_more(alnum, "-"), "/", one_or_more(alnum, ".", "-", "_")), "#", one_or_more(digit)), + # GitHub user + one_or_more(alnum, "-"), + # version requirement for deprecation notes, e.g. '>3.2.0' --> once 3.2.0 is released, do X + list(">", one_or_more(digit, ".")) + )), + ")" + ) + ), undesirable_function_linter(modify_defaults( defaults = default_undesirable_functions, library = NULL, diff --git a/DESCRIPTION b/DESCRIPTION index 987e74b42..5cfc49721 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -32,7 +32,7 @@ Imports: rex, stats, utils, - xml2 (>= 1.0.0), + xml2 (>= 1.3.6), xmlparsedata (>= 1.0.5) Suggests: bookdown, diff --git a/NAMESPACE b/NAMESPACE index 00d9ad2e0..6441d5523 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -195,7 +195,7 @@ importFrom(xml2,xml_children) importFrom(xml2,xml_find_all) importFrom(xml2,xml_find_chr) importFrom(xml2,xml_find_first) +importFrom(xml2,xml_find_int) importFrom(xml2,xml_find_lgl) -importFrom(xml2,xml_find_num) importFrom(xml2,xml_name) importFrom(xml2,xml_text) diff --git a/R/brace_linter.R b/R/brace_linter.R index eebdb90ec..27a35c5a5 100644 --- a/R/brace_linter.R +++ b/R/brace_linter.R @@ -74,7 +74,7 @@ brace_linter <- function(allow_single_line = FALSE) { )") )) - # TODO (AshesITR): if c_style_braces is TRUE, invert the preceding-sibling condition + # TODO(AshesITR): if c_style_braces is TRUE, invert the preceding-sibling condition xp_open_curly <- glue("//OP-LEFT-BRACE[ { xp_cond_open } and ( @@ -109,7 +109,7 @@ brace_linter <- function(allow_single_line = FALSE) { )" )) - # TODO (AshesITR): if c_style_braces is TRUE, skip the not(ELSE) condition + # TODO(AshesITR): if c_style_braces is TRUE, skip the not(ELSE) condition xp_closed_curly <- glue("//OP-RIGHT-BRACE[ { xp_cond_closed } and ( @@ -121,7 +121,7 @@ brace_linter <- function(allow_single_line = FALSE) { 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 + # TODO(AshesITR): if c_style_braces is TRUE, this needs to be @line2 + 1 xp_else_same_line <- glue("//ELSE[{xp_else_closed_curly} and @line1 != {xp_else_closed_curly}/@line2]") xp_function_brace <- "(//FUNCTION | //OP-LAMBDA)/parent::expr[@line1 != @line2 and not(expr[OP-LEFT-BRACE])]" diff --git a/R/indentation_linter.R b/R/indentation_linter.R index 4c968ffff..0e97ec575 100644 --- a/R/indentation_linter.R +++ b/R/indentation_linter.R @@ -233,7 +233,7 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al for (change in indent_changes) { change_type <- find_indent_type(change) change_begin <- as.integer(xml_attr(change, "line1")) + 1L - change_end <- xml_find_num(change, xp_block_ends) + change_end <- xml_find_int(change, xp_block_ends) if (isTRUE(change_begin <= change_end)) { to_indent <- seq(from = change_begin, to = change_end) expected_indent_levels[to_indent] <- find_new_indent( diff --git a/R/lint.R b/R/lint.R index e7915d856..541006dd7 100644 --- a/R/lint.R +++ b/R/lint.R @@ -24,8 +24,6 @@ #' @param text Optional argument for supplying a string or lines directly, e.g. if the file is already in memory or #' linting is being done ad hoc. #' -#' @aliases lint_file -# TODO(next release after 3.0.0): remove the alias #' @return An object of class `c("lints", "list")`, each element of which is a `"list"` object. #' #' @examplesIf requireNamespace("withr", quietly = TRUE) diff --git a/R/lintr-package.R b/R/lintr-package.R index 898db1445..5531c3f66 100644 --- a/R/lintr-package.R +++ b/R/lintr-package.R @@ -15,7 +15,7 @@ #' @importFrom utils capture.output getParseData getTxtProgressBar globalVariables head relist #' setTxtProgressBar tail txtProgressBar #' @importFrom xml2 as_list -#' xml_attr xml_children xml_find_all xml_find_chr xml_find_lgl xml_find_num xml_find_first xml_name xml_text +#' xml_attr xml_children xml_find_all xml_find_chr xml_find_int xml_find_lgl xml_find_first xml_name xml_text #' @rawNamespace #' if (getRversion() >= "4.0.0") { #' importFrom(tools, R_user_dir) diff --git a/R/missing_argument_linter.R b/R/missing_argument_linter.R index 3565ab5db..419f6535c 100644 --- a/R/missing_argument_linter.R +++ b/R/missing_argument_linter.R @@ -58,8 +58,7 @@ missing_argument_linter <- function(except = c("alist", "quote", "switch"), allo named_idx <- xml_name(missing_args) == "EQ_SUB" arg_id <- character(length(missing_args)) arg_id[named_idx] <- sQuote(xml_find_chr(missing_args[named_idx], "string(preceding-sibling::SYMBOL_SUB[1])"), "'") - # TODO(r-lib/xml2#412-->CRAN): use xml_find_int() instead - arg_id[!named_idx] <- xml_find_num(missing_args[!named_idx], "count(preceding-sibling::OP-COMMA)") + 1.0 + arg_id[!named_idx] <- xml_find_int(missing_args[!named_idx], "count(preceding-sibling::OP-COMMA)") + 1L xml_nodes_to_lints( missing_args, diff --git a/R/object_usage_linter.R b/R/object_usage_linter.R index 5d222e232..443c73bc3 100644 --- a/R/object_usage_linter.R +++ b/R/object_usage_linter.R @@ -87,8 +87,6 @@ object_usage_linter <- function(interpret_glue = TRUE, skip_with = TRUE) { skip_with = skip_with ) - # TODO handle assignment functions properly - # e.g. `not_existing<-`(a, b) res$name <- re_substitutes(res$name, rex("<-"), "") lintable_symbols <- xml_find_all(fun_assignment, xpath_culprit_symbol) @@ -211,7 +209,7 @@ parse_check_usage <- function(expression, # nocov start is_missing <- is.na(res$message) if (any(is_missing)) { - # TODO (AshesITR): Remove this in the future, if no bugs arise from this safeguard + # TODO(AshesITR): Remove this in the future, if no bugs arise from this safeguard warning( "Possible bug in lintr: Couldn't parse usage message ", sQuote(vals[is_missing][[1L]]), ". ", "Ignoring ", sum(is_missing), " usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.", diff --git a/R/shared_constants.R b/R/shared_constants.R index 8e6255f20..d581c7e35 100644 --- a/R/shared_constants.R +++ b/R/shared_constants.R @@ -243,18 +243,7 @@ extract_glued_symbols <- function(expr, interpret_glue) { if (!isTRUE(interpret_glue)) { return(character()) } - # TODO support more glue functions - # Package glue: - # - glue_sql - # - glue_safe - # - glue_col - # - glue_data - # - glue_data_sql - # - glue_data_safe - # - glue_data_col - # - # Package stringr: - # - str_interp + # TODO(#2448): support more glue functions # NB: position() > 1 because position=1 is glue_call_xpath <- " descendant::SYMBOL_FUNCTION_CALL[text() = 'glue'] diff --git a/R/todo_comment_linter.R b/R/todo_comment_linter.R index e2161fd78..cbdbb5619 100644 --- a/R/todo_comment_linter.R +++ b/R/todo_comment_linter.R @@ -9,22 +9,17 @@ #' @examples #' # will produce lints #' lint( -#' text = "x + y # TODO", -#' linters = todo_comment_linter() -#' ) -#' -#' lint( -#' text = "pi <- 1.0 # FIXME", -#' linters = todo_comment_linter() +#' text = "x + y # TOODOO", +#' linters = todo_comment_linter(todo = "toodoo") #' ) #' #' lint( -#' text = "x <- TRUE # hack", -#' linters = todo_comment_linter(todo = c("todo", "fixme", "hack")) +#' text = "pi <- 1.0 # FIIXMEE", +#' linters = todo_comment_linter(todo = "fiixmee") #' ) #' #' lint( -#' text = "x <- TRUE # TODO(#1234): Fix this hack.", +#' text = "x <- TRUE # TOODOO(#1234): Fix this hack.", #' linters = todo_comment_linter() #' ) #' diff --git a/R/unnecessary_concatenation_linter.R b/R/unnecessary_concatenation_linter.R index ed263bfb1..14d000b2d 100644 --- a/R/unnecessary_concatenation_linter.R +++ b/R/unnecessary_concatenation_linter.R @@ -100,7 +100,7 @@ unnecessary_concatenation_linter <- function(allow_single_expression = TRUE) { # c_calls <- xml_find_all(xml_calls, call_xpath) # bump count(args) by 1 if inside a pipeline - num_args <- as.integer(xml_find_num(c_calls, num_args_xpath)) + + num_args <- xml_find_int(c_calls, num_args_xpath) + as.integer(!is.na(xml_find_first(c_calls, to_pipe_xpath))) # NB: the xpath guarantees num_args is 0, 1, or 2. 2 comes # in "a" %>% c("b"). diff --git a/R/xml_nodes_to_lints.R b/R/xml_nodes_to_lints.R index 323a0f5be..a7a081117 100644 --- a/R/xml_nodes_to_lints.R +++ b/R/xml_nodes_to_lints.R @@ -5,7 +5,7 @@ #' #' @details #' The location XPaths, `column_number_xpath`, `range_start_xpath` and `range_end_xpath` are evaluated using -#' [xml2::xml_find_num()] and will usually be of the form `"number(./relative/xpath)"`. +#' [xml2::xml_find_int()] and will usually be of the form `"number(./relative/xpath)"`. #' Note that the location line number cannot be changed and lints spanning multiple lines will ignore `range_end_xpath`. #' `column_number_xpath` and `range_start_xpath` are assumed to always refer to locations on the starting line of the #' `xml` node. diff --git a/R/xp_utils.R b/R/xp_utils.R index 621bc13b4..a46cea5ab 100644 --- a/R/xp_utils.R +++ b/R/xp_utils.R @@ -107,7 +107,7 @@ xp_find_location <- function(xml, xpath) { } else if (identical(xpath, "number(./@col2)")) { as.integer(xml_attr(xml, "col2")) } else { - as.integer(xml_find_num(xml, xpath)) + xml_find_int(xml, xpath) } } @@ -118,7 +118,7 @@ xp_find_location <- function(xml, xpath) { #' way to XPath 2.0-ish support by writing this simple function to remove comments. #' #' @noRd -xpath_comment_re <- rex::rex( +xpath_comment_re <- rex( "(:", zero_or_more(not(":)")), ":)" diff --git a/man/lint.Rd b/man/lint.Rd index 54d1bbe48..efc4d2925 100644 --- a/man/lint.Rd +++ b/man/lint.Rd @@ -2,7 +2,6 @@ % Please edit documentation in R/lint.R \name{lint} \alias{lint} -\alias{lint_file} \alias{lint_dir} \alias{lint_package} \title{Lint a file, directory, or package} diff --git a/man/xml_nodes_to_lints.Rd b/man/xml_nodes_to_lints.Rd index 6a53b05a1..78afb95a9 100644 --- a/man/xml_nodes_to_lints.Rd +++ b/man/xml_nodes_to_lints.Rd @@ -48,7 +48,7 @@ linter logic into a \code{\link[=Lint]{Lint()}} object to return. } \details{ The location XPaths, \code{column_number_xpath}, \code{range_start_xpath} and \code{range_end_xpath} are evaluated using -\code{\link[xml2:xml_find_all]{xml2::xml_find_num()}} and will usually be of the form \code{"number(./relative/xpath)"}. +\code{\link[xml2:xml_find_all]{xml2::xml_find_int()}} and will usually be of the form \code{"number(./relative/xpath)"}. Note that the location line number cannot be changed and lints spanning multiple lines will ignore \code{range_end_xpath}. \code{column_number_xpath} and \code{range_start_xpath} are assumed to always refer to locations on the starting line of the \code{xml} node. diff --git a/tests/testthat/test-get_source_expressions.R b/tests/testthat/test-get_source_expressions.R index 917a414ed..d58f547ba 100644 --- a/tests/testthat/test-get_source_expressions.R +++ b/tests/testthat/test-get_source_expressions.R @@ -240,8 +240,8 @@ test_that("returned data structure is complete", { expect_identical(full_expr$content, lines_with_attr) expect_identical(nrow(full_expr$full_parsed_content), 2L * length(lines)) expect_identical( - xml2::xml_find_num(full_expr$full_xml_parsed_content, "count(//SYMBOL)"), - as.numeric(length(lines)) + xml2::xml_find_int(full_expr$full_xml_parsed_content, "count(//SYMBOL)"), + length(lines) ) expect_true(full_expr$terminal_newline) diff --git a/tests/testthat/test-todo_comment_linter.R b/tests/testthat/test-todo_comment_linter.R index 922819344..103f9c8fc 100644 --- a/tests/testthat/test-todo_comment_linter.R +++ b/tests/testthat/test-todo_comment_linter.R @@ -1,5 +1,5 @@ test_that("returns the correct linting", { - linter <- todo_comment_linter(todo = c("todo", "fixme")) + linter <- todo_comment_linter() lint_msg <- rex::rex("Remove TODO comments.") expect_lint('a <- "you#need#to#fixme"', NULL, linter)