Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add xml_find_function_calls() helper to source expressions #2357

Merged
merged 25 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6c1f497
add first implementation of xml_find_function_calls
AshesITR Nov 27, 2023
f840e71
delint
AshesITR Nov 27, 2023
16f3327
Merge branch 'main' into feature/2350-xpath-cache
AshesITR Dec 11, 2023
27bb711
support getting all function calls and using names.
AshesITR Dec 11, 2023
fdc8b41
squash conversions
AshesITR Dec 11, 2023
d874444
review comments
AshesITR Dec 12, 2023
205d9c4
clean up
AshesITR Dec 12, 2023
2dce778
add vignette section and NEWS bullet
AshesITR Dec 12, 2023
82dd1cb
Merge branch 'main' into feature/2350-xpath-cache
AshesITR Dec 12, 2023
8c84446
smarter conjunct_test_linter migration
AshesITR Dec 12, 2023
be7b0e3
smarter consecutive_assertion_linter migration
AshesITR Dec 12, 2023
b825fe6
remove self::SYMBOL_FUNCTION_CALL[text() = ...] xpaths
AshesITR Dec 12, 2023
0e8784c
delint
AshesITR Dec 12, 2023
9b7b385
Update expect_s3_class_linter.R
AshesITR Dec 12, 2023
577e84a
Reference GH issue # in TODO
MichaelChirico Dec 13, 2023
9fab41b
review comments
AshesITR Dec 13, 2023
6c730d9
Merge branch 'main' into feature/2350-xpath-cache
AshesITR Dec 13, 2023
6c869dd
add missing comma in example
AshesITR Dec 13, 2023
dca5390
Update NEWS.md
MichaelChirico Dec 13, 2023
2188ceb
supersede #2365
AshesITR Dec 13, 2023
5d7b12d
Update R/xp_utils.R
AshesITR Dec 13, 2023
430aebe
fix bad commit
AshesITR Dec 13, 2023
78ee9a6
Merge branch 'main' into feature/2350-xpath-cache
AshesITR Dec 13, 2023
20a3fe8
Update NEWS.md
MichaelChirico Dec 13, 2023
451f409
Add an upper bound improvement from r-devel
MichaelChirico Dec 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ Collate:
'settings_utils.R'
'shared_constants.R'
'sort_linter.R'
'source_utils.R'
'spaces_inside_linter.R'
'spaces_left_parentheses_linter.R'
'sprintf_linter.R'
Expand Down
38 changes: 22 additions & 16 deletions R/get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,36 @@
#' \describe{
#' \item{expressions}{a `list` of
#' `n+1` objects. The first `n` elements correspond to each expression in
#' `filename`, and consist of a list of 9 elements:
#' `filename`, and consist of a list of 8 elements:
#' \itemize{
#' \item{`filename` (`character`)}
#' \item{`line` (`integer`) the line in `filename` where this expression begins}
#' \item{`column` (`integer`) the column in `filename` where this expression begins}
#' \item{`filename` (`character`) the name of the file.}
#' \item{`line` (`integer`) the line in the file where this expression begins.}
#' \item{`column` (`integer`) the column in the file where this expression begins.}
#' \item{`lines` (named `character`) vector of all lines spanned by this
#' expression, named with the line number corresponding to `filename`}
#' \item{`parsed_content` (`data.frame`) as given by [utils::getParseData()] for this expression}
#' \item{`xml_parsed_content` (`xml_document`) the XML parse tree of this
#' expression as given by [xmlparsedata::xml_parse_data()]}
#' \item{`content` (`character`) the same as `lines` as a single string (not split across lines)}
#' expression, named with the corresponding line numbers.}
#' \item{`parsed_content` (`data.frame`) as given by [utils::getParseData()] for this expression.}
#' \item{`xml_parsed_content` (`xml_document`) the XML parse tree of this expression as given by
#' [xmlparsedata::xml_parse_data()].}
#' \item{`content` (`character`) the same as `lines` as a single string (not split across lines).}
#' \item{`xml_find_function_calls(function_names)` (`function`) a function that returns all `SYMBOL_FUNCTION_CALL`
#' XML nodes from `xml_parsed_content` with specified function names.}
#' }
#'
#' The final element of `expressions` is a list corresponding to the full file
#' consisting of 6 elements:
#' consisting of 7 elements:
#' \itemize{
#' \item{`filename` (`character`)}
#' \item{`file_lines` (`character`) the [readLines()] output for this file}
#' \item{`filename` (`character`) the name of this file.}
#' \item{`file_lines` (`character`) the [readLines()] output for this file.}
#' \item{`content` (`character`) for .R files, the same as `file_lines`;
#' for .Rmd or .qmd scripts, this is the extracted R source code (as text)}
#' for .Rmd or .qmd scripts, this is the extracted R source code (as text).}
#' \item{`full_parsed_content` (`data.frame`) as given by
#' [utils::getParseData()] for the full content}
#' [utils::getParseData()] for the full content.}
#' \item{`full_xml_parsed_content` (`xml_document`) the XML parse tree of all
#' expressions as given by [xmlparsedata::xml_parse_data()]}
#' expressions as given by [xmlparsedata::xml_parse_data()].}
#' \item{`terminal_newline` (`logical`) records whether `filename` has a terminal
#' newline (as determined by [readLines()] producing a corresponding warning)}
#' newline (as determined by [readLines()] producing a corresponding warning).}
#' \item{`xml_find_function_calls(function_names)` (`function`) a function that returns all `SYMBOL_FUNCTION_CALL`
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
#' XML nodes from `full_xml_parsed_content` with specified function names.}
#' }
#' }
#' \item{error}{A `Lint` object describing any parsing error.}
Expand Down Expand Up @@ -103,6 +107,7 @@ get_source_expressions <- function(filename, lines = NULL) {
)
for (i in seq_along(expressions)) {
expressions[[i]]$xml_parsed_content <- expression_xmls[[i]]
expressions[[i]]$xml_find_function_calls <- build_xml_find_function_calls(expression_xmls[[i]])
}
}

Expand All @@ -113,6 +118,7 @@ get_source_expressions <- function(filename, lines = NULL) {
content = source_expression$lines,
full_parsed_content = parsed_content,
full_xml_parsed_content = xml_parsed_content,
xml_find_function_calls = build_xml_find_function_calls(xml_parsed_content),
terminal_newline = terminal_newline
)
}
Expand Down
16 changes: 16 additions & 0 deletions R/source_utils.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#' Build the `xml_find_function_calls()` helper for a source expression
#'
#' @param xml The XML parse tree as an XML object (`xml_parsed_content` or `full_xml_parsed_content`)
#'
#' @return A fast function to query
#' `xml_find_all(xml, glue::glue("//SYMBOL_FUNCTION_CALL[{ xp_text_in_table(function_names) }]"))`.
AshesITR marked this conversation as resolved.
Show resolved Hide resolved
#'
#' @noRd
build_xml_find_function_calls <- function(xml) {
function_call_cache <- xml_find_all(xml, "//SYMBOL_FUNCTION_CALL")
names(function_call_cache) <- get_r_string(function_call_cache)
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved

function(function_names) {
unname(function_call_cache[names(function_call_cache) %in% function_names])
}
}
36 changes: 20 additions & 16 deletions man/get_source_expressions.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 37 additions & 2 deletions tests/testthat/test-get_source_expressions.R
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@ test_that("returned data structure is complete", {

for (i in seq_along(lines)) {
expr <- exprs$expressions[[i]]
expect_named(expr, c("filename", "line", "column", "lines", "parsed_content", "xml_parsed_content", "content"))
expect_named(expr, c(
"filename", "line", "column", "lines", "parsed_content", "xml_parsed_content", "content",
"xml_find_function_calls"
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
))
expect_identical(expr$filename, temp_file)
expect_identical(expr$line, i)
expect_identical(expr$column, 1L)
Expand All @@ -229,7 +232,8 @@ test_that("returned data structure is complete", {
}
full_expr <- exprs$expressions[[length(lines) + 1L]]
expect_named(full_expr, c(
"filename", "file_lines", "content", "full_parsed_content", "full_xml_parsed_content", "terminal_newline"
"filename", "file_lines", "content", "full_parsed_content", "full_xml_parsed_content", "xml_find_function_calls",
"terminal_newline"
))
expect_identical(full_expr$filename, temp_file)
expect_identical(full_expr$file_lines, lines_with_attr)
Expand All @@ -245,6 +249,37 @@ test_that("returned data structure is complete", {
expect_identical(exprs$lines, lines_with_attr)
})

test_that("xml_find_function_calls works as intended", {
lines <- c("foo()", "bar()", "foo()", "{ foo(); foo(); bar() }")
temp_file <- withr::local_tempfile(lines = lines)

exprs <- get_source_expressions(temp_file)

expect_length(exprs$expressions[[1L]]$xml_find_function_calls("foo"), 1L)
expect_length(exprs$expressions[[1L]]$xml_find_function_calls("bar"), 0L)
expect_identical(
exprs$expressions[[1L]]$xml_find_function_calls("foo"),
xml_find_all(exprs$expressions[[1L]]$xml_parsed_content, "//SYMBOL_FUNCTION_CALL[text() = 'foo']")
)

expect_length(exprs$expressions[[2L]]$xml_find_function_calls("foo"), 0L)
expect_length(exprs$expressions[[2L]]$xml_find_function_calls("bar"), 1L)

expect_length(exprs$expressions[[4L]]$xml_find_function_calls("foo"), 2L)
expect_length(exprs$expressions[[4L]]$xml_find_function_calls("bar"), 1L)
expect_length(exprs$expressions[[4L]]$xml_find_function_calls(c("foo", "bar")), 3L)

expect_length(exprs$expressions[[5L]]$xml_find_function_calls("foo"), 4L)
AshesITR marked this conversation as resolved.
Show resolved Hide resolved
expect_length(exprs$expressions[[5L]]$xml_find_function_calls("bar"), 2L)
expect_length(exprs$expressions[[5L]]$xml_find_function_calls(c("foo", "bar")), 6L)

# Also check order is retained:
expect_identical(
exprs$expressions[[5L]]$xml_find_function_calls(c("foo", "bar")),
xml_find_all(exprs$expressions[[5L]]$full_xml_parsed_content, "//SYMBOL_FUNCTION_CALL")
)
})

test_that("#1262: xml_parsed_content gets returned as missing even if there's no parsed_content", {
tempfile <- withr::local_tempfile(lines = '"\\R"')

Expand Down
Loading