Skip to content

fixed_regex_linter() doesn't fail with "\\/" #1846

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Dec 16, 2022
Merged

fixed_regex_linter() doesn't fail with "\\/" #1846

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e615066
`fixed_regex_linter()` doesn't fail with `"\\/"`
IndrajeetPatil Dec 13, 2022
634328a
include more characters
IndrajeetPatil Dec 13, 2022
20d439e
push new ones to the end
IndrajeetPatil Dec 13, 2022
906add9
remove duplicates
IndrajeetPatil Dec 13, 2022
58d25c0
minor cleanup
IndrajeetPatil Dec 13, 2022
7bfc1b4
add parametrized tests
IndrajeetPatil Dec 16, 2022
e0a5109
Merge branch 'main' into 1845_bug_fix
IndrajeetPatil Dec 16, 2022
e562b21
Merge branch 'main' into 1845_bug_fix
IndrajeetPatil Dec 16, 2022
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
2 changes: 1 addition & 1 deletion NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Bug fixes

* `fixed_regex_linter()` no longer fails with regular expression pattern `"\\;"` (#1545, @IndrajeetPatil).
* `fixed_regex_linter()` no longer fails with regular expression patterns `"\\;"` and `\\/` (#1545, #1845, @IndrajeetPatil).

* `get_source_expressions()` can handle Sweave/Rmarkdown documents with reference chunks like `<<ref_file>>` (#779, @MichaelChirico).
Note that these are simply skipped, rather than attempting to retrieve the reference and also lint it.
Expand Down
10 changes: 7 additions & 3 deletions R/fixed_regex_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ fixed_regex_linter <- function() {

# regular expression pattern is the second argument
pos_2_regex_funs <- xp_text_in_table(c(
"strsplit", "tstrsplit",
# stringr functions. even though the user action is different
# base functions.
"strsplit",
# data.table functions.
"tstrsplit",
# stringr functions.
# even though the user action is different
# (setting fixed=TRUE vs. wrapping stringr::fixed()),
# detection of the lint is the same
"str_count", "str_detect", "str_ends", "str_extract", "str_extract_all",
Expand Down Expand Up @@ -231,7 +235,7 @@ get_token_replacement <- function(token_content, token_type) {
token_content
}
} else { # char_escape token
if (rex::re_matches(token_content, rex::rex("\\", one_of("^${}().*+?|[]\\<>:;")))) {
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, '"')))
Expand Down
2 changes: 2 additions & 0 deletions tests/testthat/test-fixed_regex_linter.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ test_that("fixed_regex_linter blocks simple disallowed usages", {
expect_lint("regexec('\\\\$', x)", lint_msg, linter)
expect_lint("grep('\n', x)", lint_msg, linter)
expect_lint("grep('\\\\;', x)", lint_msg, linter)
expect_lint("grep('\\\\/', x)", lint_msg, linter)

# naming the argument doesn't matter (if it's still used positionally)
expect_lint("gregexpr(pattern = 'a-z', y)", lint_msg, linter)
Expand Down Expand Up @@ -78,6 +79,7 @@ test_that("fixed_regex_linter catches calls to strsplit as well", {
lint_msg <- rex::rex("This regular expression is static")

expect_lint("strsplit('a;b', '\\\\;')", lint_msg, linter)
expect_lint("strsplit('a/b', '\\\\/')", lint_msg, linter)
expect_lint("strsplit(x, '\\\\.')", lint_msg, linter)
expect_lint("tstrsplit(x, 'abcdefg')", lint_msg, linter)
expect_lint("strsplit(x, '[.]')", lint_msg, linter)
Expand Down