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 1 commit
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
6 changes: 5 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@
* `string_boundary_linter()` recognizes regular expression calls like `grepl("^abc$", x)` that can be replaced by using `==` instead (#1613, @MichaelChirico).
* `unreachable_code_linter()` has an argument `allow_comment_regex` for customizing which "terminal" comments to exclude (#2327, @MichaelChirico). `# nolint end` comments are always excluded, as are {covr} exclusions (e.g. `# nocov end`) by default.
* `format()` and `print()` methods for `lint` and `lints` classes get a new option `width` to control the printing width of lint messages (#1884, @MichaelChirico). The default is controlled by a new option `lintr.format_width`; if unset, no wrapping occurs (matching earlier behavior).
* The `source_expression` passed to linters gains a fast way to query function call nodes using `source_expression$xml_find_function_calls()`. Use this to speed up linters using XPaths that start with `//SYMBOL_FUNCTION_CALL`. (@AshesITR, #2357)
* The full linter suite is roughly 14% faster due to caching of the frequently used `//SMBOL_FUNCTION_CALL` XPath to examine function calls. (@AshesITR, #2357)
AshesITR marked this conversation as resolved.
Show resolved Hide resolved
MichaelChirico marked this conversation as resolved.
Show resolved Hide resolved
+ The `source_expression` passed to linters gains a fast way to query function call nodes using `source_expression$xml_find_function_calls()`. Use this to speed up linters using XPaths that start with `//SYMBOL_FUNCTION_CALL`.
+ The vignette on creating linters contains additional information on how to use it.
+ Instead of `xml_find_all(source_expression$xml_parsed_content, "//SYMBOL_FUNCTION_CALL[text() = 'foo' or text() = 'bar']`, use `source_expression$xml_find_function_calls(c("foo", "bar"))`.
+ Instead of `make_linter_from_xpath(xpath = "//SYMBOL_FUNCTION_CALL[text() = 'foo' or text() = 'bar']/cond")`, use the new `make_linter_from_function_xpath(function_names = c("foo", "bar"), xpath = "cond")`.

### New linters

Expand Down
22 changes: 12 additions & 10 deletions R/system_file_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,23 @@
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
system_file_linter <- function() {
funs <- c("system.file", "file.path")
# either system.file(file.path(...)) or file.path(system.file(...))
xpath_parts <- glue("
parent::expr[following-sibling::expr/expr/SYMBOL_FUNCTION_CALL[text() = '{rev(funs)}']]
file_path_xpath <- "
parent::expr[following-sibling::expr/expr/SYMBOL_FUNCTION_CALL[text() = 'system.file']]
/parent::expr
")
"
system_file_xpath <- "
parent::expr[following-sibling::expr/expr/SYMBOL_FUNCTION_CALL[text() = 'file.path']]
/parent::expr
"

Linter(linter_level = "expression", function(source_expression) {
xml_calls <- list(
source_expression$xml_find_function_calls(funs[1L]),
source_expression$xml_find_function_calls(funs[2L])
)
file_path_calls <- source_expression$xml_find_function_calls("file.path")
system_file_calls <- source_expression$xml_find_function_calls("system.file")

bad_expr <- combine_nodesets(
xml_find_all(xml_calls[[1L]], xpath_parts[1L]),
xml_find_all(xml_calls[[2L]], xpath_parts[2L])
xml_find_all(file_path_calls, file_path_xpath),
xml_find_all(system_file_calls, system_file_xpath)
)

outer_call <- xp_call_name(bad_expr)
Expand Down
5 changes: 5 additions & 0 deletions vignettes/creating_linters.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,11 @@ and so they've been tested and demonstrated their utility already.
call_xpath <- "parent::expr/some/cond"
xml_find_all(xml_calls, call_xpath)
```
* `make_linter_from_xpath()` and `make_linter_from_function_xpath()`: Whenever your
linter can be expressed by a static XPath and a static message, use `make_linter_from_xpath()`
or, if the XPath starts with `//SYMBOL_FUNCTION_CALL`, use `make_linter_from_function_xpath()`.
Instead of `make_linter_from_xpath(xpath = "//SYMBOL_FUNCTION_CALL[text() = 'foo' or text() = 'bar']/cond")`,
use `make_linter_from_function_xpath(function_names = c("foo", "bar"), xpath = "cond")`.

## Contributing to `{lintr}`

Expand Down
Loading