cleanup internal namespace, use shared_constants.R for shared objects (#2056)

Author: MichaelChirico

DESCRIPTION

@@ -153,6 +153,7 @@ Collate:
     'seq_linter.R'
     'settings.R'
     'settings_utils.R'
+    'shared_constants.R'
     'sort_linter.R'
     'spaces_inside_linter.R'
     'spaces_left_parentheses_linter.R'

R/expect_s3_class_linter.R

@@ -45,6 +45,21 @@ expect_s3_class_linter <- function() {
     ]
     /parent::expr[not(SYMBOL_SUB[text() = 'info' or text() = 'label' or text() = 'expected.label'])]
   "
+  # NB: there is no easy way to make an exhaustive list of places where an
+  #   is.<x> call can be replaced by expect_s3_class(); this list was manually
+  #   populated from the default R packages by inspection. For example,
+  #   is.matrix(x) cannot be replaced by expect_s3_class(x, "matrix") because
+  #   it is not actually an S3 class (is.object(x) is not TRUE).
+  #   Further, there are functions named is.<x> that have nothing to do with
+  #   object type, e.g. is.finite(), is.nan(), or is.R().
+  is_s3_class_calls <- paste0("is.", c(
+    # base
+    "data.frame", "factor", "numeric_version", "ordered", "package_version", "qr", "table",
+    #      utils grDevices     tcltk    tcltk    grid    grid
+    "relistable", "raster", "tclObj", "tkwin", "grob", "unit",
+    # stats
+    "mts", "stepfun", "ts", "tskernel"
+  ))
   is_class_call <- xp_text_in_table(c(is_s3_class_calls, "inherits"))
   expect_true_xpath <- glue("
   //SYMBOL_FUNCTION_CALL[text() = 'expect_true']
@@ -76,19 +91,3 @@ expect_s3_class_linter <- function() {
     )
   })
 }
-
-# NB: there is no easy way to make an exhaustive list of places where an
-#   is.<x> call can be replaced by expect_s3_class(); this list was manually
-#   populated from the default R packages by inspection. For example,
-#   is.matrix(x) cannot be replaced by expect_s3_class(x, "matrix") because
-#   it is not actually an S3 class (is.object(x) is not TRUE).
-#   Further, there are functions named is.<x> that have nothing to do with
-#   object type, e.g. is.finite(), is.nan(), or is.R().
-is_s3_class_calls <- paste0("is.", c(
-  # base
-  "data.frame", "factor", "numeric_version", "ordered", "package_version", "qr", "table",
-  #      utils grDevices     tcltk    tcltk    grid    grid
-  "relistable", "raster", "tclObj", "tkwin", "grob", "unit",
-  # stats
-  "mts", "stepfun", "ts", "tskernel"
-))

R/expect_type_linter.R

@@ -27,6 +27,20 @@
 #' @seealso [linters] for a complete list of linters available in lintr.
 #' @export
 expect_type_linter <- function() {
+  # NB: the full list of values that can arise from `typeof(x)` is available
+  #   in ?typeof (or, slightly more robustly, in the R source: src/main/util.c.
+  #   Not all of them are available in is.<type> form, e.g. 'any' or
+  #   'special'. 'builtin' and 'closure' are special cases, corresponding to
+  #   is.primitive and is.function (essentially).
+  base_types <- c(
+    "raw", "logical", "integer", "double", "complex", "character", "list",
+    "numeric", "function", "primitive", "environment", "pairlist", "promise",
+    # Per ?is.language, it's the same as is.call || is.name || is.expression.
+    #   so by blocking it, we're forcing more precise tests of one of
+    #   those directly ("language", "symbol", and "expression", resp.)
+    # NB: is.name and is.symbol are identical.
+    "language", "call", "name", "symbol", "expression"
+  )
   base_type_tests <- xp_text_in_table(paste0("is.", base_types))
   expect_equal_identical_xpath <- "
   //SYMBOL_FUNCTION_CALL[text() = 'expect_equal' or text() = 'expect_identical']
@@ -67,18 +81,3 @@ expect_type_linter <- function() {
     )
   })
 }
-
-# NB: the full list of values that can arise from `typeof(x)` is available
-#   in ?typeof (or, slightly more robustly, in the R source: src/main/util.c.
-#   Not all of them are available in is.<type> form, e.g. 'any' or
-#   'special'. 'builtin' and 'closure' are special cases, corresponding to
-#   is.primitive and is.function (essentially).
-base_types <- c(
-  "raw", "logical", "integer", "double", "complex", "character", "list",
-  "numeric", "function", "primitive", "environment", "pairlist", "promise",
-  # Per ?is.language, it's the same as is.call || is.name || is.expression.
-  #   so by blocking it, we're forcing more precise tests of one of
-  #   those directly ("language", "symbol", and "expression", resp.)
-  # NB: is.name and is.symbol are identical.
-  "language", "call", "name", "symbol", "expression"
-)

R/fixed_regex_linter.R

@@ -141,104 +141,3 @@ fixed_regex_linter <- function() {
     )
   })
 }
-
-rx_non_active_char <- rex::rex(none_of("^${(.*+?|[\\"))
-rx_static_escape <- local({
-  rx_char_escape <- rex::rex(or(
-    group("\\", none_of(alnum)),
-    group("\\x", between(xdigit, 1L, 2L)),
-    group("\\", between("0":"7", 1L, 3L)),
-    group("\\u{", between(xdigit, 1L, 4L), "}"),
-    group("\\u", between(xdigit, 1L, 4L)),
-    group("\\U{", between(xdigit, 1L, 8L), "}"),
-    group("\\U", between(xdigit, 1L, 8L))
-  ))
-  rx_trivial_char_group <- rex::rex(
-    "[",
-    or(
-      any,
-      group("\\", none_of("dswDSW")), # character classes, e.g. \d are enabled in [] too if perl = TRUE
-      rx_char_escape
-    ),
-    "]"
-  )
-  rex::rex(or(
-    capture(rx_char_escape, name = "char_escape"),
-    capture(rx_trivial_char_group, name = "trivial_char_group")
-  ))
-})
-
-rx_static_token <- local({
-  rex::rex(or(
-    rx_non_active_char,
-    rx_static_escape
-  ))
-})
-
-rx_static_regex <- paste0("(?s)", rex::rex(start, zero_or_more(rx_static_token), end))
-rx_first_static_token <- paste0("(?s)", rex::rex(start, zero_or_more(rx_non_active_char), rx_static_escape))
-
-#' Determine whether a regex pattern actually uses regex patterns
-#'
-#' Note that is applies to the strings that are found on the XML parse tree,
-#'   _not_ plain strings. This is important for backslash escaping, which
-#'   happens at different layers of escaping than one might expect. So testing
-#'   this function is best done through testing the expected results of a lint
-#'   on a given file, rather than passing strings to this function, which can
-#'   be confusing.
-#'
-#' @param str A character vector.
-#' @return A logical vector, `TRUE` wherever `str` could be replaced by a
-#'   string with `fixed = TRUE`.
-#' @noRd
-is_not_regex <- function(str) {
-  # need to add single-line option to allow literal newlines
-  grepl(rx_static_regex, str, perl = TRUE)
-}
-
-#' Compute a fixed string equivalent to a static regular expression
-#'
-#' @param static_regex A regex for which `is_not_regex()` returns `TRUE`
-#' @return A string such that `grepl(static_regex, x)` is equivalent to
-#' `grepl(get_fixed_string(static_regex), x, fixed = TRUE)`
-#'
-#' @noRd
-get_fixed_string <- function(static_regex) {
-  if (length(static_regex) == 0L) {
-    return(character())
-  } else if (length(static_regex) > 1L) {
-    return(vapply(static_regex, get_fixed_string, character(1L)))
-  }
-  fixed_string <- ""
-  current_match <- regexpr(rx_first_static_token, static_regex, perl = TRUE)
-  while (current_match != -1L) {
-    token_type <- attr(current_match, "capture.names")[attr(current_match, "capture.start") > 0L]
-    token_start <- max(attr(current_match, "capture.start"))
-    if (token_start > 1L) {
-      fixed_string <- paste0(fixed_string, substr(static_regex, 1L, token_start - 1L))
-    }
-    consume_to <- attr(current_match, "match.length")
-    token_content <- substr(static_regex, token_start, consume_to)
-    fixed_string <- paste0(fixed_string, get_token_replacement(token_content, token_type))
-    static_regex <- substr(static_regex, start = consume_to + 1L, stop = nchar(static_regex))
-    current_match <- regexpr(rx_first_static_token, static_regex, perl = TRUE)
-  }
-  paste0(fixed_string, static_regex)
-}
-
-get_token_replacement <- function(token_content, token_type) {
-  if (token_type == "trivial_char_group") {
-    token_content <- substr(token_content, start = 2L, stop = nchar(token_content) - 1L)
-    if (startsWith(token_content, "\\")) { # escape within trivial char group
-      get_token_replacement(token_content, "char_escape")
-    } else {
-      token_content
-    }
-  } else { # char_escape token
-    if (rex::re_matches(token_content, rex::rex("\\", one_of("^${}().*+?|[]\\<>=:;/_-!@#%&,~")))) {
-      substr(token_content, start = 2L, stop = nchar(token_content))
-    } else {
-      eval(parse(text = paste0('"', token_content, '"')))
-    }
-  }
-}

R/indentation_linter.R

@@ -130,15 +130,11 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al
 
   hanging_indent_style <- match.arg(hanging_indent_style)
 
-  if (hanging_indent_style == "tidy") {
-    find_indent_type <- build_indentation_style_tidy()
-  } else if (hanging_indent_style == "always") {
-    find_indent_type <- build_indentation_style_always()
-  } else { # "never"
-    find_indent_type <- function(change) {
-      "block"
-    }
-  }
+  find_indent_type <- switch(hanging_indent_style,
+    tidy = build_indentation_style_tidy(),
+    always = build_indentation_style_always(),
+    never = function(change) "block"
+  )
 
   if (isTRUE(assignment_as_infix)) {
     suppressing_tokens <- c("LEFT_ASSIGN", "EQ_ASSIGN", "EQ_SUB", "EQ_FORMALS")
@@ -313,15 +309,12 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al
 }
 
 find_new_indent <- function(current_indent, change_type, indent, hanging_indent) {
-  if (change_type == "suppress") {
-    current_indent
-  } else if (change_type == "hanging") {
-    hanging_indent
-  } else if (change_type == "double") {
-    current_indent + 2L * indent
-  } else {
-    current_indent + indent
-  }
+  switch(change_type,
+    suppress = current_indent,
+    hanging = hanging_indent,
+    double = current_indent + 2L * indent,
+    block = current_indent + indent
+  )
 }
 
 build_indentation_style_tidy <- function() {

R/infix_spaces_linter.R

@@ -1,75 +1,3 @@
-# some metadata about infix operators on the R parse tree.
-#   xml_tag gives the XML tag as returned by xmlparsedata::xml_parse_data().
-#   r_string gives the operator as you would write it in R code.
-# NB: this metadata is used elsewhere in lintr, e.g. spaces_left_parentheses_linter.
-#   because of that, even though some rows of this table are currently unused, but
-#   we keep them around because it's useful to keep this info in one place.
-
-# styler: off
-infix_metadata <- data.frame(stringsAsFactors = FALSE, matrix(byrow = TRUE, ncol = 2L, c(
-  "OP-PLUS",         "+",
-  "OP-MINUS",        "-",
-  "OP-TILDE",        "~",
-  "GT",              ">",
-  "GE",              ">=",
-  "LT",              "<",
-  "LE",              "<=",
-  "EQ",              "==",
-  "NE",              "!=",
-  "AND",              "&",
-  "OR",              "|",
-  "AND2",            "&&",
-  "OR2",             "||",
-  "LEFT_ASSIGN",     "<-",  # also includes := and <<-
-  "RIGHT_ASSIGN",    "->",  # also includes ->>
-  "EQ_ASSIGN",       "=",
-  "EQ_SUB",          "=",   # in calls: foo(x = 1)
-  "EQ_FORMALS",      "=",   # in definitions: function(x = 1)
-  "PIPE",            "|>",
-  "SPECIAL",         "%%",
-  "OP-SLASH",        "/",
-  "OP-STAR",         "*",
-  "OP-COMMA",        ",",
-  "OP-CARET",        "^",   # also includes **
-  "OP-AT",           "@",
-  "OP-EXCLAMATION",  "!",
-  "OP-COLON",        ":",
-  "NS_GET",          "::",
-  "NS_GET_INT",      ":::",
-  "OP-LEFT-BRACE",   "{",
-  "OP-LEFT-BRACKET", "[",
-  "LBB",             "[[",
-  "OP-LEFT-PAREN",   "(",
-  "OP-QUESTION",     "?",
-  "OP-DOLLAR",       "$",
-  NULL
-)))
-# styler: on
-
-names(infix_metadata) <- c("xml_tag", "string_value")
-# utils::getParseData()'s designation for the tokens wouldn't be valid as XML tags
-infix_metadata$parse_tag <- ifelse(
-  startsWith(infix_metadata$xml_tag, "OP-"),
-  quote_wrap(infix_metadata$string_value, "'"),
-  infix_metadata$xml_tag
-)
-# treated separately because spacing rules are different for unary operators
-infix_metadata$unary <- infix_metadata$xml_tag %in% c("OP-PLUS", "OP-MINUS", "OP-TILDE")
-# high-precedence operators are ignored by this linter; see
-#   https://style.tidyverse.org/syntax.html#infix-operators
-infix_metadata$low_precedence <- infix_metadata$string_value %in% c(
-  "+", "-", "~", ">", ">=", "<", "<=", "==", "!=", "&", "&&", "|", "||", "<-", "->", "=", "%%", "/", "*", "|>"
-)
-# comparators come up in several lints
-infix_metadata$comparator <- infix_metadata$string_value %in% c("<", "<=", ">", ">=", "==", "!=")
-
-# undesirable_operator_linter needs to distinguish
-infix_overload <- data.frame(
-  exact_string_value = c("<-", "<<-", "=", "->", "->>", "^", "**"),
-  xml_tag = rep(c("LEFT_ASSIGN", "RIGHT_ASSIGN", "OP-CARET"), c(3L, 2L, 2L)),
-  stringsAsFactors = FALSE
-)
-
 #' Infix spaces linter
 #'
 #' Check that infix operators are surrounded by spaces. Enforces the corresponding Tidyverse style guide rule;

R/keyword_quote_linter.R

@@ -52,6 +52,11 @@
 #   //expr[expr[SYMBOL_FUNCTION_CALL]]/SYMBOL_SUB[starts-with(text(), '`')]
 #   //expr[expr[SYMBOL_FUNCTION_CALL]]/STR_CONST[{is_quoted(text())}]
 keyword_quote_linter <- function() {
+  # Check if a string could be assigned as an R variable.
+  #
+  # See [make.names()] for the description of syntactically valid names in R.
+  is_valid_r_name <- function(x) make.names(x) == x
+
   # NB: xml2 uses xpath 1.0 which doesn't support matches() for regex, so we
   #   have to jump out of xpath to complete this lint.
   # It's also a bit tough to get the escaping through R and then xpath to
@@ -157,10 +162,3 @@ keyword_quote_linter <- function() {
     c(call_arg_lints, string_assignment_lints, assignment_lints, string_extraction_lints, extraction_lints)
   })
 }
-
-#' Check if a string could be assigned as an R variable.
-#'
-#' See [make.names()] for the description of syntactically valid names in R.
-#'
-#' @noRd
-is_valid_r_name <- function(x) make.names(x) == x

R/matrix_apply_linter.R

@@ -149,5 +149,5 @@ craft_colsums_rowsums_msg <- function(var, margin, fun, narm_val) {
   # It's easier to remove this after the fact, rather than having never ending if/elses
   reco <- gsub(", dims = 1", "", reco, fixed = TRUE)
 
-  return(reco)
+  reco
 }

R/nested_ifelse_linter.R

@@ -59,7 +59,3 @@ nested_ifelse_linter <- function() {
     xml_nodes_to_lints(bad_expr, source_expression, lint_message, type = "warning")
   })
 }
-
-# functions equivalent to base::ifelse() for linting purposes
-# NB: this is re-used elsewhere, e.g. in ifelse_censor_linter
-ifelse_funs <- c("ifelse", "if_else", "fifelse")