-
Notifications
You must be signed in to change notification settings - Fork 186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate out tests for allowed assignments with braces #1872
Conversation
For `implicit_assignment_linter()`
test_that("implicit_assignment_linter makes exceptions for functions that capture side-effects", { | ||
linter <- implicit_assignment_linter() | ||
test_that("implicit_assignment_linter skips allowed usages with braces", { | ||
linter <- implicit_assignment_linter(except = character(0L)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that no exceptions are included here.
Codecov Report
@@ Coverage Diff @@
## main #1872 +/- ##
=======================================
Coverage 98.86% 98.86%
=======================================
Files 112 112
Lines 4841 4841
=======================================
Hits 4786 4786
Misses 55 55 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
expect_lint("quote(a <- 1L)", NULL, linter) | ||
expect_lint("bquote(a <- 1L)", NULL, linter) | ||
expect_lint("expression(a <- 1L)", NULL, linter) | ||
expect_lint("output <- capture.output({ x <- f() })", NULL, linter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like these are all side-effect functions... my recollection (CMIIW) was we wanted to allow <-
within braces no matter the parent function.
If that's the case, could you make some of these tests use a generic function like foo({ a <- 1L })
?
It may also be preferable to keep the code aligned to the rest of the default linters, which means using
foo({
a <- 1L
})
because of brace_linter()
.
For
implicit_assignment_linter()