Don't lint expression(paste(., sep = "")) in paste_linter #2955
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## main #2955 +/- ## ======================================= Coverage 99.24% 99.24% ======================================= Files 128 128 Lines 7291 7307 +16 ======================================= + Hits 7236 7252 +16 Misses 55 55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| I've changed the implementation so that we report a different message if |
R/paste_linter.R Outdated
| #' ) | ||
| #' | ||
| #' lint( | ||
| #' text = 'expression(paste("a", "b", sep = ""))', |
There was a problem hiding this comment.
this should be in the "will produce lints" section; our approach generally is to "pair" linting expressions with their "clean" equivalents across the "top" and bottom (under "okay") parts of the documentation.
(it's a bit messy for this file in particular because paste_linter() does a lot)
| | ||
| if (!allow_empty_sep) { | ||
| ## check if we are inside an expression() | ||
| expression_paste_sep_expr <- xml_find_all(paste_calls, expression_paste_sep_xpath) |
There was a problem hiding this comment.
this looks off -- what happens for something like this?
c( expression(paste(x, y, sep="")), paste(x, y, sep=""), expression(paste(a, b, sep="")), paste(a, b, sep=""), expression(c(paste(d, e, sep=""), paste(f, g), paste(h, i, sep=""))) )(generally xml_find_all() is 🚩 to me since it is not 1:1 -- for y = xml_find_all(x, ...), y[i] cannot predictably be associated with x[i] -- usually within linters we want to use xml_find_first())
2 participants
Fixes #2945.