diff --git a/NAMESPACE b/NAMESPACE index e70060a02..1b52606b8 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -143,6 +143,7 @@ export(whitespace_linter) export(with_defaults) export(with_id) export(xml_nodes_to_lints) +export(xp_call_name) export(yoda_test_linter) importFrom(cyclocomp,cyclocomp) importFrom(glue,glue) diff --git a/NEWS.md b/NEWS.md index e12772ccd..b1d30912e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -30,6 +30,7 @@ * `conjunct_test_linter()` also lints usage like `dplyr::filter(x, A & B)` in favor of using `dplyr::filter(x, A, B)` (part of #884, @MichaelChirico). * `sort_linter()` checks for code like `x == sort(x)` which is better served by using the function `is.unsorted()` (part of #884, @MichaelChirico). * `paste_linter()` gains detection for file paths that are better constructed with `file.path()`, e.g. `paste0(dir, "/", file)` would be better as `file.path(dir, file)` (part of #884, @MichaelChirico). +* New `xp_call_name()` helper to facilitate writing custom linters (#2023, @MichaelChirico). This helper converts a matched XPath to the R function to which it corresponds. This is useful for including the "offending" function in the lint's message. ### New linters diff --git a/R/xp_utils.R b/R/xp_utils.R index e5bec73e9..8717dc810 100644 --- a/R/xp_utils.R +++ b/R/xp_utils.R @@ -44,7 +44,41 @@ xp_and <- function(...) paren_wrap(..., sep = "and") #' @noRd xp_or <- function(...) paren_wrap(..., sep = "or") +#' Get the name of the function matched by an XPath +#' +#' Often, it is more helpful to custom-tailer the `message` of a lint to record +#' which function was matched by the lint logic. This function encapsualtes +#' the logic to pull out the matched call in common situations. +#' +#' @param expr An `xml_node` or `xml_nodeset`, e.g. from [xml2::xml_find_all()]. +#' @param depth Integer, default `1L`. How deep in the AST represented by `expr` +#' should we look to find the call? By default, we assume `expr` is matched +#' to an `` node under which the corresponding `` +#' node is found directly. `depth = 0L` means `expr` is matched directly +#' to the `SYMBOL_FUNCTION_CALL`; `depth > 1L` means `depth` total `` +#' nodes must be traversed before finding the call. +#' @param condition An additional (XPath condition on the `SYMBOL_FUNCTION_CALL` +#' required for a match. The default (`NULL`) is no condition. See examples. +#' +#' @examples +#' xml_from_code <- function(str) { +#' xml2::read_xml(xmlparsedata::xml_parse_data(parse(text = str, keep.source = TRUE))) +#' } +#' xml <- xml_from_code("sum(1:10)") +#' xp_call_name(xml, depth = 2L) +#' +#' xp_call_name(xml2::xml_find_first(xml, "expr")) +#' +#' xml <- xml_from_code(c("sum(1:10)", "sd(1:10)")) +#' xp_call_name(xml, depth = 2L, condition = "text() = 'sum'") +#' +#' @export xp_call_name <- function(expr, depth = 1L, condition = NULL) { + stopifnot( + inherits(expr, c("xml_node", "xml_nodeset")), + is.numeric(depth), depth >= 0L, + is.null(condition) || is.character(condition) + ) if (is.null(condition)) { node <- "SYMBOL_FUNCTION_CALL" } else { diff --git a/_pkgdown.yaml b/_pkgdown.yaml index 7c13de7df..290571031 100644 --- a/_pkgdown.yaml +++ b/_pkgdown.yaml @@ -41,6 +41,7 @@ reference: - get_r_string - use_lintr - xml_nodes_to_lints + - xp_call_name - title: Meta-tooling contents: diff --git a/man/xp_call_name.Rd b/man/xp_call_name.Rd new file mode 100644 index 000000000..51822306e --- /dev/null +++ b/man/xp_call_name.Rd @@ -0,0 +1,39 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/xp_utils.R +\name{xp_call_name} +\alias{xp_call_name} +\title{Get the name of the function matched by an XPath} +\usage{ +xp_call_name(expr, depth = 1L, condition = NULL) +} +\arguments{ +\item{expr}{An \code{xml_node} or \code{xml_nodeset}, e.g. from \code{\link[xml2:xml_find_all]{xml2::xml_find_all()}}.} + +\item{depth}{Integer, default \code{1L}. How deep in the AST represented by \code{expr} +should we look to find the call? By default, we assume \code{expr} is matched +to an \verb{} node under which the corresponding \verb{} +node is found directly. \code{depth = 0L} means \code{expr} is matched directly +to the \code{SYMBOL_FUNCTION_CALL}; \code{depth > 1L} means \code{depth} total \verb{} +nodes must be traversed before finding the call.} + +\item{condition}{An additional (XPath condition on the \code{SYMBOL_FUNCTION_CALL} +required for a match. The default (\code{NULL}) is no condition. See examples.} +} +\description{ +Often, it is more helpful to custom-tailer the \code{message} of a lint to record +which function was matched by the lint logic. This function encapsualtes +the logic to pull out the matched call in common situations. +} +\examples{ +xml_from_code <- function(str) { + xml2::read_xml(xmlparsedata::xml_parse_data(parse(text = str, keep.source = TRUE))) +} +xml <- xml_from_code("sum(1:10)") +xp_call_name(xml, depth = 2L) + +xp_call_name(xml2::xml_find_first(xml, "expr")) + +xml <- xml_from_code(c("sum(1:10)", "sd(1:10)")) +xp_call_name(xml, depth = 2L, condition = "text() = 'sum'") + +} diff --git a/tests/testthat/test-xp_utils.R b/tests/testthat/test-xp_utils.R new file mode 100644 index 000000000..eb8cf7c69 --- /dev/null +++ b/tests/testthat/test-xp_utils.R @@ -0,0 +1,21 @@ +test_that("xp_call_name works", { + xml_from_code <- function(str) { + xml2::read_xml(xmlparsedata::xml_parse_data(parse(text = str, keep.source = TRUE))) + } + xml <- xml_from_code("sum(1:10)") + expect_identical(xp_call_name(xml, depth = 2L), "sum") + + expect_identical(xp_call_name(xml2::xml_find_first(xml, "expr")), "sum") + + xml <- xml_from_code(c("sum(1:10)", "sd(1:10)")) + expect_identical(xp_call_name(xml, depth = 2L, condition = "text() = 'sum'"), "sum") +}) + +test_that("xp_call_name input validation works", { + expect_error(xp_call_name(2L), "inherits(expr", fixed = TRUE) + + xml <- xml2::read_xml("") + expect_error(xp_call_name(xml, depth = -1L), "depth >= 0", fixed = TRUE) + expect_error(xp_call_name(xml, depth = "1"), "is.numeric(depth)", fixed = TRUE) + expect_error(xp_call_name(xml, condition = 1L), "is.character(condition)", fixed = TRUE) +})