Skip to content
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

Add exceptions for duplicate_argument_linter() #1550

Merged
merged 15 commits into from
Sep 25, 2022
Merged
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

## Changes to defaults

* Set the default for the `except` argument in `duplicate_argument_linter()` to `c("mutate", "transmute")`.
This allows sequential updates like `x |> mutate(a = b + 1, a = log(a))` (#1345, @IndrajeetPatil).

* `object_usage_linter()` gains `skip_with` argument to skip code in `with()` expressions.
To be consistent with `R CMD check`, it defaults to `TRUE` (#941, #1458, @IndrajeetPatil).

Expand Down
6 changes: 4 additions & 2 deletions R/duplicate_argument_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
#' Duplicate-named objects are hard to work with programmatically and
#' should typically be avoided.
#'
#' @param except a character vector of function names as exceptions.
#' @param except A character vector of function names as exceptions. Defaults to
#' functions that allow sequential updates to variables, currently `dplyr::mutate()`
#' and `dplyr::transmute()`.
#' @evalRd rd_tags("duplicate_argument_linter")
#' @seealso [linters] for a complete list of linters available in lintr.
#' @export
duplicate_argument_linter <- function(except = character()) {
duplicate_argument_linter <- function(except = c("mutate", "transmute")) {
xpath_call_with_args <- "//expr[EQ_SUB]"
xpath_arg_name <- "./EQ_SUB/preceding-sibling::*[1]"

Expand Down
6 changes: 4 additions & 2 deletions man/duplicate_argument_linter.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions tests/testthat/test-any_duplicated_linter.R
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
test_that("any_duplicated_linter skips allowed usages", {
expect_lint("x <- any(y)", NULL, any_duplicated_linter())
linter <- any_duplicated_linter()

expect_lint("y <- duplicated(z)", NULL, any_duplicated_linter())
expect_lint("x <- any(y)", NULL, linter)
expect_lint("y <- duplicated(z)", NULL, linter)

# extended usage of any is not covered
expect_lint("any(duplicated(y), b)", NULL, any_duplicated_linter())
expect_lint("any(b, duplicated(y))", NULL, any_duplicated_linter())
expect_lint("any(duplicated(y), b)", NULL, linter)
expect_lint("any(b, duplicated(y))", NULL, linter)
})

test_that("any_duplicated_linter blocks simple disallowed usages", {
Expand Down
122 changes: 92 additions & 30 deletions tests/testthat/test-duplicate_argument_linter.R
Original file line number Diff line number Diff line change
@@ -1,82 +1,144 @@
test_that("returns the correct linting", {
expect_lint("fun(arg = 1)",
expect_lint(
"fun(arg = 1)",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("fun('arg' = 1)",
expect_lint(
"fun('arg' = 1)",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("fun(`arg` = 1)",
expect_lint(
"fun(`arg` = 1)",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("'fun'(arg = 1)",
expect_lint(
"'fun'(arg = 1)",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("(function(x, y) x + y)(x = 1)",
expect_lint(
"(function(x, y) x + y)(x = 1)",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("dt[i = 1]",
expect_lint(
"dt[i = 1]",
NULL,
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("fun(arg = 1, arg = 2)",
expect_lint(
"fun(arg = 1, arg = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("fun(arg = 1, 'arg' = 2)",
expect_lint(
"fun(arg = 1, 'arg' = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("fun(arg = 1, `arg` = 2)",
expect_lint(
"fun(arg = 1, `arg` = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("'fun'(arg = 1, arg = 2)",
expect_lint(
"'fun'(arg = 1, arg = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("(function(x, y) x + y)(x = 1, x = 2)",
expect_lint(
"(function(x, y) x + y)(x = 1, x = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint("dt[i = 1, i = 2]",
expect_lint(
"dt[i = 1, i = 2]",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint(
"list(
var = 1,
var = 2
)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter()
)

expect_lint(
"list(
var = 1,
var = 2
)",
NULL,
duplicate_argument_linter(except = "list"))
duplicate_argument_linter(except = "list")
)

expect_lint(
"(function(x, y) x + y)(x = 1)
list(var = 1, var = 2)",
NULL,
duplicate_argument_linter(except = "list"))
duplicate_argument_linter(except = "list")
)

expect_lint(
"fun(`
` = 1, `
` = 2)",
list(message = rex("Duplicate arguments in function call.")),
duplicate_argument_linter())
duplicate_argument_linter(except = character())
)

expect_lint("function(arg = 1, arg = 1) {}",
expect_lint(
"function(arg = 1, arg = 1) {}",
list(message = rex("Repeated formal argument 'arg'.")),
duplicate_argument_linter())
duplicate_argument_linter(except = character())
)
})

test_that("doesn't lint duplicated arguments in allowed functions", {
IndrajeetPatil marked this conversation as resolved.
Show resolved Hide resolved
expect_lint(
"x %>%
dplyr::mutate(
col = a + b,
col = col + d
)",
NULL,
duplicate_argument_linter()
)

expect_lint(
"x %>%
dplyr::transmute(
col = a + b,
col = col / 2.5
)",
NULL,
duplicate_argument_linter()
)

skip_if_not_r_version("4.1")
expect_lint(
"x |>
dplyr::mutate(
col = col |> str_replace('t', '') |> str_replace('\\\\s+$', 'xxx')
)",
NULL,
duplicate_argument_linter()
)
})