Don't lint expression(paste(., sep = "")) in paste_linter#2955
Don't lint expression(paste(., sep = "")) in paste_linter#2955mcol wants to merge 8 commits intor-lib:mainfrom
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.