From 14c0ca1423448f07546fb3adff898a641a5ce3ea Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 11 Sep 2023 00:14:52 +0000 Subject: [PATCH 1/2] use space in xml2lang --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 4741b1662..505eeed59 100644 --- a/R/utils.R +++ b/R/utils.R @@ -257,7 +257,7 @@ get_r_string <- function(s, xpath = NULL) { #' @noRd xml2lang <- function(x) { x_strip_comments <- xml_find_all(x, ".//*[not(self::COMMENT or self::expr)]") - str2lang(paste(xml_text(x_strip_comments), collapse = "")) + str2lang(paste(xml_text(x_strip_comments), collapse = " ")) } is_linter <- function(x) inherits(x, "linter") From dd88ff2ee6dc2495331a330f32c76cf16866c94e Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Mon, 11 Sep 2023 00:30:01 +0000 Subject: [PATCH 2/2] add test & NEWS --- NEWS.md | 5 +++-- tests/testthat/test-sprintf_linter.R | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 34e08ab6d..c4c2a657e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,8 +6,7 @@ ## Bug fixes -* `inner_combine_linter()` no longer throws on length-1 calls to `c()` like `c(exp(2))` or `c(log(3))` (#2017, @MichaelChirico). Such usage is discouraged by `unnecessary_concatenation_linter()`, but `inner_combine_linter()` _per se_ does not apply. -* `condition_message_linter()` ignores usages of extracted calls like `env$stop(paste(a, b))` (#1455, @MichaelChirico). +* `sprintf_linter()` doesn't error in cases where whitespace in `...` arguments is significant, e.g. `sprintf("%s", if (A) "" else y)`, which won't parse if whitespace is removed (#2131, @MichaelChirico). ## New and improved features @@ -44,6 +43,8 @@ * `unreachable_code_linter()` + finds unreachable code even in the presence of a comment or semicolon after `return()` or `stop()` (#2127, @MEO265). + checks for code inside `if (FALSE)` and other conditional loops with deterministically false conditions (#1428, @ME0265). +* `inner_combine_linter()` no longer throws on length-1 calls to `c()` like `c(exp(2))` or `c(log(3))` (#2017, @MichaelChirico). Such usage is discouraged by `unnecessary_concatenation_linter()`, but `inner_combine_linter()` _per se_ does not apply. +* `condition_message_linter()` ignores usages of extracted calls like `env$stop(paste(a, b))` (#1455, @MichaelChirico). ### New linters diff --git a/tests/testthat/test-sprintf_linter.R b/tests/testthat/test-sprintf_linter.R index d1921018d..3e9b9c4cd 100644 --- a/tests/testthat/test-sprintf_linter.R +++ b/tests/testthat/test-sprintf_linter.R @@ -90,6 +90,9 @@ test_that("edge cases are detected correctly", { list(message = rex::rex("reference to non-existent argument 3")), linter ) + + # #2131: xml2lang stripped necessary whitespace + expect_lint("sprintf('%s', if (A) '' else y)", NULL, linter) }) local({