Add ubiquitously-called internal functions to NAMESPACE (#2031)

* keyword_quote_linter for unnecessary quoting

* trim_some

* import glue

* glue::glue -> glue

* other glue functions

* xml2:: replacement

* more namespace importing

* ue namespace imports for ubiquitous internal functions

* remove test file too

* delete from metadata

* revert spurious edits to comments

* fix whitespace in XPaths

* revert inside string

* restore :: for seldom-used calls

* new :: usage

* unimport rare calls

NAMESPACE

@@ -144,6 +144,7 @@ export(xml_nodes_to_lints)
 export(yoda_test_linter)
 importFrom(cyclocomp,cyclocomp)
 importFrom(glue,glue)
+importFrom(glue,glue_collapse)
 importFrom(rex,character_class)
 importFrom(rex,re_matches)
 importFrom(rex,re_substitutes)
@@ -156,6 +157,9 @@ importFrom(utils,head)
 importFrom(utils,relist)
 importFrom(utils,tail)
 importFrom(xml2,as_list)
+importFrom(xml2,xml_attr)
 importFrom(xml2,xml_find_all)
+importFrom(xml2,xml_find_chr)
 importFrom(xml2,xml_find_first)
+importFrom(xml2,xml_find_num)
 importFrom(xml2,xml_text)

R/T_and_F_symbol_linter.R

@@ -48,11 +48,11 @@ T_and_F_symbol_linter <- function() { # nolint: object_name.
       return(list())
     }
 
-    bad_usage <- xml2::xml_find_all(source_expression$xml_parsed_content, usage_xpath)
-    bad_assignment <- xml2::xml_find_all(source_expression$xml_parsed_content, assignment_xpath)
+    bad_usage <- xml_find_all(source_expression$xml_parsed_content, usage_xpath)
+    bad_assignment <- xml_find_all(source_expression$xml_parsed_content, assignment_xpath)
 
     make_lints <- function(expr, fmt) {
-      symbol <- xml2::xml_text(expr)
+      symbol <- xml_text(expr)
       lint_message <- sprintf(fmt, replacement_map[symbol], symbol)
       xml_nodes_to_lints(
         xml = expr,

R/any_duplicated_linter.R

@@ -54,7 +54,7 @@ any_duplicated_linter <- function() {
   #  the final parent::expr/expr gets us to the expr on the other side of EQ;
   #  this lets us match on either side of EQ, where following-sibling
   #  assumes we are before EQ, preceding-sibling assumes we are after EQ.
-  length_unique_xpath_parts <- glue::glue("
+  length_unique_xpath_parts <- glue("
   //{ c('EQ', 'NE', 'GT', 'LT') }
     /parent::expr
     /expr[
@@ -92,17 +92,17 @@ any_duplicated_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    any_duplicated_expr <- xml2::xml_find_all(xml, any_duplicated_xpath)
+    any_duplicated_expr <- xml_find_all(xml, any_duplicated_xpath)
     any_duplicated_lints <- xml_nodes_to_lints(
       any_duplicated_expr,
       source_expression = source_expression,
       lint_message = "anyDuplicated(x, ...) > 0 is better than any(duplicated(x), ...).",
       type = "warning"
     )
 
-    length_unique_expr <- xml2::xml_find_all(xml, length_unique_xpath)
+    length_unique_expr <- xml_find_all(xml, length_unique_xpath)
     lint_message <- ifelse(
-      is.na(xml2::xml_find_first(length_unique_expr, uses_nrow_xpath)),
+      is.na(xml_find_first(length_unique_expr, uses_nrow_xpath)),
       "anyDuplicated(x) == 0L is better than length(unique(x)) == length(x).",
       "anyDuplicated(DF$col) == 0L is better than length(unique(DF$col)) == nrow(DF)"
     )

R/any_is_na_linter.R

@@ -53,7 +53,7 @@ any_is_na_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
     xml_nodes_to_lints(
       bad_expr,

R/assignment_linter.R

@@ -106,20 +106,20 @@ assignment_linter <- function(allow_cascading_assign = TRUE,
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
     if (length(bad_expr) == 0L) {
       return(list())
     }
 
-    operator <- xml2::xml_text(bad_expr)
+    operator <- xml_text(bad_expr)
     lint_message_fmt <- rep("Use <-, not %s, for assignment.", length(operator))
     lint_message_fmt[operator %in% c("<<-", "->>")] <-
       "%s can have hard-to-predict behavior; prefer assigning to a specific environment instead (with assign() or <-)."
     lint_message_fmt[operator == "%<>%"] <-
       "Avoid the assignment pipe %s; prefer using <- and %%>%% separately."
 
     if (!allow_trailing) {
-      bad_trailing_expr <- xml2::xml_find_all(xml, trailing_assign_xpath)
+      bad_trailing_expr <- xml_find_all(xml, trailing_assign_xpath)
       trailing_assignments <- xml2::xml_attrs(bad_expr) %in% xml2::xml_attrs(bad_trailing_expr)
       lint_message_fmt[trailing_assignments] <- "Assignment %s should not be trailing at the end of a line."
     }

R/backport_linter.R

@@ -50,8 +50,8 @@ backport_linter <- function(r_version = getRversion(), except = character()) {
 
     xml <- source_expression$xml_parsed_content
 
-    all_names_nodes <- xml2::xml_find_all(xml, names_xpath)
-    all_names <- xml2::xml_text(all_names_nodes)
+    all_names_nodes <- xml_find_all(xml, names_xpath)
+    all_names <- xml_text(all_names_nodes)
 
     # not sapply/vapply, which may over-simplify to vector
     # rbind makes sure we have a matrix with dimensions [n_versions x n_names]

R/boolean_arithmetic_linter.R

@@ -34,7 +34,7 @@ boolean_arithmetic_linter <- function() {
   # TODO(#1581): extend to include all()-alike expressions
   zero_expr <- "(EQ or NE or GT or LE) and expr[NUM_CONST[text() = '0' or text() = '0L']]"
   one_expr <- "(LT or GE) and expr[NUM_CONST[text() = '1' or text() = '1L']]"
-  length_xpath <- glue::glue("
+  length_xpath <- glue("
   //SYMBOL_FUNCTION_CALL[text() = 'which' or text() = 'grep']
     /parent::expr
     /parent::expr
@@ -43,7 +43,7 @@ boolean_arithmetic_linter <- function() {
       and parent::expr[ ({zero_expr}) or ({one_expr})]
     ]
   ")
-  sum_xpath <- glue::glue("
+  sum_xpath <- glue("
   //SYMBOL_FUNCTION_CALL[text() = 'sum']
     /parent::expr
     /parent::expr[
@@ -62,7 +62,7 @@ boolean_arithmetic_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    any_expr <- xml2::xml_find_all(xml, any_xpath)
+    any_expr <- xml_find_all(xml, any_xpath)
 
     xml_nodes_to_lints(
       any_expr,

R/brace_linter.R

@@ -75,7 +75,7 @@ brace_linter <- function(allow_single_line = FALSE) {
   ))
 
   # TODO (AshesITR): if c_style_braces is TRUE, invert the preceding-sibling condition
-  xp_open_curly <- glue::glue("//OP-LEFT-BRACE[
+  xp_open_curly <- glue("//OP-LEFT-BRACE[
     { xp_cond_open }
     and (
       not(@line1 = parent::expr/preceding-sibling::*/@line2)
@@ -85,7 +85,7 @@ brace_linter <- function(allow_single_line = FALSE) {
 
   xp_open_preceding <- "parent::expr/preceding-sibling::*[1][self::OP-RIGHT-PAREN or self::ELSE or self::REPEAT]"
 
-  xp_paren_brace <- glue::glue("//OP-LEFT-BRACE[
+  xp_paren_brace <- glue("//OP-LEFT-BRACE[
     @line1 = { xp_open_preceding }/@line1
     and @col1 = { xp_open_preceding }/@col2 + 1
   ]")
@@ -110,7 +110,7 @@ brace_linter <- function(allow_single_line = FALSE) {
   ))
 
   # TODO (AshesITR): if c_style_braces is TRUE, skip the not(ELSE) condition
-  xp_closed_curly <- glue::glue("//OP-RIGHT-BRACE[
+  xp_closed_curly <- glue("//OP-RIGHT-BRACE[
     { xp_cond_closed }
     and (
       (@line1 = preceding-sibling::*[1][not(self::OP-LEFT-BRACE)]/@line2)
@@ -122,7 +122,7 @@ brace_linter <- function(allow_single_line = FALSE) {
   # 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
-  xp_else_same_line <- glue::glue("//ELSE[{xp_else_closed_curly} and @line1 != {xp_else_closed_curly}/@line2]")
+  xp_else_same_line <- glue("//ELSE[{xp_else_closed_curly} and @line1 != {xp_else_closed_curly}/@line2]")
 
   xp_function_brace <- "//FUNCTION/parent::expr[@line1 != @line2 and not(expr[OP-LEFT-BRACE])]"
 
@@ -157,7 +157,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_open_curly),
+        xml_find_all(xml, xp_open_curly),
         source_expression = source_expression,
         lint_message =
           "Opening curly braces should never go on their own line and should always be followed by a new line."
@@ -167,7 +167,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_paren_brace),
+        xml_find_all(xml, xp_paren_brace),
         source_expression = source_expression,
         lint_message = "There should be a space before an opening curly brace."
       )
@@ -176,7 +176,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_closed_curly),
+        xml_find_all(xml, xp_closed_curly),
         source_expression = source_expression,
         lint_message =
           "Closing curly-braces should always be on their own line, unless they are followed by an else."
@@ -186,7 +186,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_else_same_line),
+        xml_find_all(xml, xp_else_same_line),
         source_expression = source_expression,
         lint_message = "`else` should come on the same line as the previous `}`."
       )
@@ -195,7 +195,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_function_brace),
+        xml_find_all(xml, xp_function_brace),
         source_expression = source_expression,
         lint_message = "Any function spanning multiple lines should use curly braces."
       )
@@ -204,7 +204,7 @@ brace_linter <- function(allow_single_line = FALSE) {
     lints <- c(
       lints,
       xml_nodes_to_lints(
-        xml2::xml_find_all(xml, xp_if_else_match_brace),
+        xml_find_all(xml, xp_if_else_match_brace),
         source_expression = source_expression,
         lint_message = "Either both or neither branch in `if`/`else` should use curly braces."
       )

R/class_equals_linter.R

@@ -51,9 +51,9 @@ class_equals_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
-    operator <- xml2::xml_find_chr(bad_expr, "string(*[2])")
+    operator <- xml_find_chr(bad_expr, "string(*[2])")
     lint_message <- sprintf(
       "Instead of comparing class(x) with %s, use inherits(x, 'class-name') or is.<class> or is(x, 'class')",
       operator

R/commas_linter.R

@@ -67,15 +67,15 @@ commas_linter <- function() {
     xml <- source_expression$xml_parsed_content
 
     before_lints <- xml_nodes_to_lints(
-      xml2::xml_find_all(xml, xpath_before),
+      xml_find_all(xml, xpath_before),
       source_expression = source_expression,
       lint_message = "Commas should never have a space before.",
       range_start_xpath = "number(./preceding-sibling::*[1]/@col2 + 1)", # start after preceding expression
       range_end_xpath = "number(./@col1 - 1)" # end before comma
     )
 
     after_lints <- xml_nodes_to_lints(
-      xml2::xml_find_all(xml, xpath_after),
+      xml_find_all(xml, xpath_after),
       source_expression = source_expression,
       lint_message = "Commas should always have a space after.",
       range_start_xpath = "number(./@col2 + 1)", # start and end after comma

R/comment_linters.R

@@ -79,8 +79,8 @@ commented_code_linter <- function() {
     if (!is_lint_level(source_expression, "file")) {
       return(list())
     }
-    all_comment_nodes <- xml2::xml_find_all(source_expression$full_xml_parsed_content, "//COMMENT")
-    all_comments <- xml2::xml_text(all_comment_nodes)
+    all_comment_nodes <- xml_find_all(source_expression$full_xml_parsed_content, "//COMMENT")
+    all_comments <- xml_text(all_comment_nodes)
     code_candidates <- re_matches(all_comments, code_candidate_regex, global = FALSE, locations = TRUE)
     extracted_code <- code_candidates[, "code"]
     # ignore trailing ',' when testing for parsability

R/condition_message_linter.R

@@ -42,7 +42,7 @@
 #' @export
 condition_message_linter <- function() {
   translators <- c("packageStartupMessage", "message", "warning", "stop")
-  xpath <- glue::glue("
+  xpath <- glue("
   //SYMBOL_FUNCTION_CALL[
     ({xp_text_in_table(translators)})
     and not(preceding-sibling::OP-DOLLAR or preceding-sibling::OP-AT)
@@ -62,7 +62,7 @@ condition_message_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
     sep_value <- get_r_string(bad_expr, xpath = "./expr/SYMBOL_SUB[text() = 'sep']/following-sibling::expr/STR_CONST")
 
     bad_expr <- bad_expr[is.na(sep_value) | sep_value %in% c("", " ")]

R/conjunct_test_linter.R

@@ -48,7 +48,7 @@ conjunct_test_linter <- function(allow_named_stopifnot = TRUE) {
   /following-sibling::expr[1][AND2]
   "
   named_stopifnot_condition <- if (allow_named_stopifnot) "and not(preceding-sibling::*[1][self::EQ_SUB])" else ""
-  stopifnot_xpath <- glue::glue("
+  stopifnot_xpath <- glue("
   //SYMBOL_FUNCTION_CALL[text() = 'stopifnot']
   /parent::expr
   /following-sibling::expr[1][AND2 {named_stopifnot_condition}]
@@ -72,14 +72,14 @@ conjunct_test_linter <- function(allow_named_stopifnot = TRUE) {
 
     xml <- source_expression$full_xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
     if (length(bad_expr) == 0L) {
       return(list())
     }
 
     matched_fun <- xp_call_name(bad_expr)
-    operator <- xml2::xml_find_chr(bad_expr, "string(expr/*[self::AND2 or self::OR2])")
+    operator <- xml_find_chr(bad_expr, "string(expr/*[self::AND2 or self::OR2])")
     replacement_fmt <- ifelse(
       matched_fun %in% c("expect_true", "expect_false"),
       "write multiple expectations like %1$s(A) and %1$s(B)",

R/consecutive_assertion_linter.R

@@ -55,7 +55,7 @@ consecutive_assertion_linter <- function() {
 
     xml <- source_expression$full_xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
     matched_function <- xp_call_name(bad_expr)
     xml_nodes_to_lints(

R/declared_functions.R

@@ -15,5 +15,5 @@ declared_s3_generics <- function(x) {
     "/expr/SYMBOL/text()"
   )
 
-  as.character(xml2::xml_find_all(x, xpath))
+  as.character(xml_find_all(x, xpath))
 }

R/duplicate_argument_linter.R

@@ -47,14 +47,14 @@ duplicate_argument_linter <- function(except = c("mutate", "transmute")) {
 
     xml <- source_expression$full_xml_parsed_content
 
-    calls <- xml2::xml_find_all(xml, xpath_call_with_args)
+    calls <- xml_find_all(xml, xpath_call_with_args)
 
     if (length(except) > 0L) {
       calls_text <- get_r_string(xp_call_name(calls))
       calls <- calls[!(calls_text %in% except)]
     }
 
-    all_arg_nodes <- lapply(calls, xml2::xml_find_all, xpath_arg_name)
+    all_arg_nodes <- lapply(calls, xml_find_all, xpath_arg_name)
     arg_names <- lapply(all_arg_nodes, get_r_string)
     is_duplicated <- lapply(arg_names, duplicated)
 

R/empty_assignment_linter.R

@@ -49,7 +49,7 @@ empty_assignment_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
     xml_nodes_to_lints(
       bad_expr,

R/equals_na_linter.R

@@ -32,7 +32,7 @@
 equals_na_linter <- function() {
   na_table <- xp_text_in_table(c("NA", "NA_integer_", "NA_real_", "NA_complex_", "NA_character_"))
 
-  xpath <- glue::glue("
+  xpath <- glue("
   //NUM_CONST[ {na_table} ]
     /parent::expr
     /parent::expr[EQ or NE]
@@ -45,7 +45,7 @@ equals_na_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
     xml_nodes_to_lints(
       bad_expr,

R/exclude.R

@@ -181,7 +181,7 @@ add_exclusions <- function(exclusions, lines, linters_string, exclude_linter_sep
         bad <- excluded_linters[!matched]
         warning(
           "Could not find linter", if (length(bad) > 1L) "s" else "", " named ",
-          glue::glue_collapse(sQuote(bad), sep = ", ", last = " and "),
+          glue_collapse(sQuote(bad), sep = ", ", last = " and "),
           " in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix."
         )
       }

R/expect_comparison_linter.R

@@ -50,7 +50,7 @@
 expect_comparison_linter <- function() {
   # != doesn't have a clean replacement
   comparator_nodes <- setdiff(as.list(infix_metadata$xml_tag[infix_metadata$comparator]), "NE")
-  xpath <- glue::glue("
+  xpath <- glue("
   //SYMBOL_FUNCTION_CALL[text() = 'expect_true']
     /parent::expr
     /following-sibling::expr[ {xp_or(comparator_nodes)} ]
@@ -70,9 +70,9 @@ expect_comparison_linter <- function() {
 
     xml <- source_expression$xml_parsed_content
 
-    bad_expr <- xml2::xml_find_all(xml, xpath)
+    bad_expr <- xml_find_all(xml, xpath)
 
-    comparator <- xml2::xml_find_chr(bad_expr, "string(expr[2]/*[2])")
+    comparator <- xml_find_chr(bad_expr, "string(expr[2]/*[2])")
     expectation <- comparator_expectation_map[comparator]
     lint_message <- sprintf("%s(x, y) is better than expect_true(x %s y).", expectation, comparator)
     xml_nodes_to_lints(bad_expr, source_expression, lint_message = lint_message, type = "warning")