tidy up own lints

r-lib · Dec 15, 2023 · 98ab4ef · 98ab4ef
1 parent 96fbbd4
commit 98ab4ef
Show file tree

Hide file tree

Showing 18 changed files with 37 additions and 45 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <expr><SYMBOL_FUNCTION_CALL>
  glue_call_xpath <- "
  descendant::SYMBOL_FUNCTION_CALL[text() = 'glue']

diff --git 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
@@ -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
@@ -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
@@ -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
diff --git a/man/xml_nodes_to_lints.Rd b/man/xml_nodes_to_lints.Rd
diff --git 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
@@ -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)