Skip to content

Commit

Permalink
Omit anchor-esque pattern in string_boundary_linter() (#2757)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico authored Feb 17, 2025
1 parent a0233d2 commit 15b75d8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 5 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
### Lint accuracy fixes: removing false positives

* `unnecessary_nesting_linter()` treats function bodies under the shorthand lambda (`\()`) the same as normal function bodies (#2748, @MichaelChirico).
* `string_boundary_linter()` omits lints of patterns like `\\^` which have an anchor but are not regular expressions (#2636, @MichaelChirico).
* `implicit_integer_linter(allow_colon = TRUE)` is OK with negative literals, e.g. `-1:1` or `1:-1` (#2673, @MichaelChirico).

## Notes
Expand Down
9 changes: 5 additions & 4 deletions R/string_boundary_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,17 @@ string_boundary_linter <- function(allow_grepl = FALSE) {
terminal_anchor <- endsWith(patterns, "$")
search_start <- 1L + initial_anchor
search_end <- nchar(patterns) - terminal_anchor
can_replace <- is_not_regex(substr(patterns, search_start, search_end))
initial_anchor <- initial_anchor[can_replace]
terminal_anchor <- terminal_anchor[can_replace]
should_lint <- (initial_anchor | terminal_anchor) &
is_not_regex(substr(patterns, search_start, search_end))
initial_anchor <- initial_anchor[should_lint]
terminal_anchor <- terminal_anchor[should_lint]

lint_type <- character(length(initial_anchor))

lint_type[initial_anchor & terminal_anchor] <- "both"
lint_type[initial_anchor & !terminal_anchor] <- "initial"
lint_type[!initial_anchor & terminal_anchor] <- "terminal"
list(lint_expr = expr[can_replace], lint_type = lint_type)
list(lint_expr = expr[should_lint], lint_type = lint_type)
}

substr_xpath <- glue("
Expand Down
18 changes: 17 additions & 1 deletion tests/testthat/test-string_boundary_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ test_that("string_boundary_linter skips allowed grepl() usages", {
# regex pattern --> no lint
expect_no_lint("grepl('^[a-z]', x)", linter)
expect_no_lint("grepl('[a-z]$', x)", linter)
# anchor-ish but not a regex --> no lint (#2636)
expect_no_lint(R"[grepl("\\^", x)]", linter)
expect_no_lint(R"[grepl("\\\\^", x)]", linter)
expect_no_lint(R"[grepl("$\\\\", x)]", linter)

# ignore.case --> no lint
expect_no_lint("grepl('^abc', x, ignore.case = TRUE)", linter)
Expand Down Expand Up @@ -151,6 +155,8 @@ test_that("whole-string regex recommends ==, not {starts,ends}With()", {
})

test_that("vectorization + metadata work as intended", {
linter <- string_boundary_linter()

expect_lint(
trim_some("{
substring(a, 1, 3) == 'abc'
Expand All @@ -176,6 +182,16 @@ test_that("vectorization + metadata work as intended", {
list("endsWith", line_number = 10L),
list("==", line_number = 11L)
),
string_boundary_linter()
linter
)

# some but not all anchor-esque as in #2636
expect_lint(
trim_some(R"[{
grepl("\\^", x)
grepl("^abc", x)
}]"),
list("startsWith", line_number = 3L),
linter
)
})

0 comments on commit 15b75d8

Please sign in to comment.