Apply condition_call_linter() only on string literals#2888
Apply condition_call_linter() only on string literals#2888MichaelChirico merged 3 commits intomainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2888 +/- ##
=======================================
Coverage 99.27% 99.27%
=======================================
Files 128 128
Lines 7160 7164 +4
=======================================
+ Hits 7108 7112 +4
Misses 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MichaelChirico
left a comment
There was a problem hiding this comment.
This was discussed a bit in the issue, but it might be worth considering if this behavior should/could be optional -- I'm thinking of cases like:
msg <- gettextf(...)
stop(msg)Those match the spirit of the original lint -- call. should be used there just as much as for stop('...'). So now users might have inconsistent usage of stop() in their code base if they obey this linter -- some will get linted, others won't.
My example also gives a common case that won't be linted but maybe should:
stop(gettextf(...))I'm still not sure the right way forward. I also don't really use this linter myself, so I will allow you to ruminate on how best to improve it later :)
| condition_call_linter <- function(display_call = FALSE) { | ||
| call_xpath <- glue::glue(" | ||
| following-sibling::SYMBOL_SUB[text() = 'call.'] | ||
| following-sibling::expr/STR_CONST |
There was a problem hiding this comment.
oh, sorry @Bisaloo this warrants a NEWS entry, please handle as a follow-up :)
Fix #2647