Skip to content

Commit

Permalink
custom message for top-level printing
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico committed Dec 11, 2023
1 parent 51913f1 commit 42b1e26
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
* `string_boundary_linter()` recognizes regular expression calls like `grepl("^abc$", x)` that can be replaced by using `==` instead (#1613, @MichaelChirico).
* `unreachable_code_linter()` has an argument `allow_comment_regex` for customizing which "terminal" comments to exclude (#2327, @MichaelChirico). `# nolint end` comments are always excluded, as are {covr} exclusions (e.g. `# nocov end`) by default.
* `format()` and `print()` methods for `lint` and `lints` classes get a new option `width` to control the printing width of lint messages (#1884, @MichaelChirico). The default is controlled by a new option `lintr.format_width`; if unset, no wrapping occurs (matching earlier behavior).
* `implicit_assignment_linter()` gets a custom message for the case of using `(` to induce printing like `(x <- foo())`; use an explicit call to `print()` for clarity (#2257, @MichaelChirico).

### New linters

Expand Down
14 changes: 9 additions & 5 deletions R/implicit_assignment_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,25 @@ implicit_assignment_linter <- function(except = c("bquote", "expression", "expr"
)
}

implicit_message <- paste(
"Avoid implicit assignments in function calls.",
"For example, instead of `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`."
)

print_message <- "Call print() explicitly instead of relying on implicit printing behavior via '('."

Linter(linter_level = "file", function(source_expression) {
# need the full file to also catch usages at the top level
xml <- source_expression$full_xml_parsed_content

bad_expr <- xml_find_all(xml, xpath)

lint_message <- paste(
"Avoid implicit assignments in function calls.",
"For example, instead of `if (x <- 1L) { ... }`, write `x <- 1L; if (x) { ... }`."
)
print_only <- !is.na(xml_find_first(bad_expr, "parent::expr[parent::exprlist and *[1][self::OP-LEFT-PAREN]]"))

xml_nodes_to_lints(
bad_expr,
source_expression = source_expression,
lint_message = lint_message,
lint_message = ifelse(print_only, print_message, implicit_message),
type = "warning"
)
})
Expand Down
32 changes: 26 additions & 6 deletions tests/testthat/test-implicit_assignment_linter.R
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,11 @@ test_that("implicit_assignment_linter works as expected with pipes and walrus op
})

test_that("parenthetical assignments are caught", {
linter <- implicit_assignment_linter()
lint_message <- rex::rex("Avoid implicit assignments in function calls.")

expect_lint("(x <- 1:10)", lint_message, linter)
expect_lint("if (A && (B <- foo())) { }", lint_message, linter)
expect_lint(
"if (A && (B <- foo())) { }",
rex::rex("Avoid implicit assignments in function calls."),
implicit_assignment_linter()
)
})

test_that("allow_lazy lets lazy assignments through", {
Expand Down Expand Up @@ -439,7 +439,6 @@ test_that("allow_scoped skips scoped assignments", {
)

# outside of branching, doesn't matter
expect_lint("(idx <- foo()); bar()", lint_message, linter)
expect_lint("foo(idx <- bar()); baz()", lint_message, linter)
expect_lint("foo(x, idx <- bar()); baz()", lint_message, linter)
})
Expand Down Expand Up @@ -477,3 +476,24 @@ test_that("interaction of allow_lazy and allow_scoped", {
linter
)
})

test_that("call-less '(' mentions avoiding implicit printing", {
linter <- implicit_assignment_linter()
implicit_msg <- rex::rex("Avoid implicit assignments in function calls.")
print_msg <- rex::rex("Call print() explicitly instead of relying on implicit printing behavior via '('.")

expect_lint("(a <- foo())", print_msg, linter)

# only for top-level assignments; withAutoprint() ignored
expect_lint("for (xi in x) (z <- foo(xi))", implicit_msg, linter)

# mixed messages
expect_lint(
trim_some("
(a <- foo())
bar(z <- baz(a))
"),
list(print_msg, implicit_msg),
linter
)
})

0 comments on commit 42b1e26

Please sign in to comment.