Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .lintr
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ linters: all_linters(
)),
unnecessary_concatenation_linter(allow_single_expression = FALSE),
absolute_path_linter = NULL,
extraction_operator_linter = NULL,
library_call_linter = NULL,
nonportable_path_linter = NULL,
todo_comment_linter = NULL,
Expand Down
1 change: 0 additions & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ Collate:
'expect_true_false_linter.R'
'expect_type_linter.R'
'extract.R'
'extraction_operator_linter.R'
'fixed_regex_linter.R'
'for_loop_index_linter.R'
'function_argument_linter.R'
Expand Down
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
+ Helper `with_defaults()`.
* `all_linters()` has signature `all_linters(..., packages)` rather than `all_linters(packages, ...)` (#2332, @MichaelChirico). This forces `packages=` to be supplied by name and will break users who rely on supplying `packages=` positionally, of which we found none searching GitHub.
* Adjusted various lint messages for consistency in readability (#1330, @MichaelChirico). In general, we favor lint messages to be phrased like "Action, reason" to but the "what" piece of the message front-and-center. This may be a breaking change for code that tests the specific phrasing of lints.
* `extraction_operator_linter()` is deprecated. Although switching from `$` to `[[` has some robustness benefits for package code, it can lead to non-idiomatic code in many contexts (e.g. R6 classes, Shiny applications, etc.) (#2409, @IndrajeetPatil). To enable the detection of the `$` operator for extraction through partial matching, use `options(warnPartialMatchDollar = TRUE)`.

## Bug fixes

Expand Down
77 changes: 0 additions & 77 deletions R/extraction_operator_linter.R

This file was deleted.

36 changes: 36 additions & 0 deletions R/lintr-deprecated.R
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,39 @@ no_tab_linter <- function() {
)
whitespace_linter()
}

#' Extraction operator linter
#' @rdname lintr-deprecated
#' @export
extraction_operator_linter <- function() {
lintr_deprecated(
what = "extraction_operator_linter",
version = "3.2.0",
type = "Linter",
signal = "warning"
)

constant_nodes_in_brackets <- paste0("self::", c("expr", "OP-PLUS", "NUM_CONST", "STR_CONST"))
xpath <- glue("
//OP-DOLLAR[not(preceding-sibling::expr[1]/SYMBOL[text() = 'self' or text() = '.self'])]
|
//OP-LEFT-BRACKET[
not(following-sibling::expr[1]/descendant::*[not({xp_or(constant_nodes_in_brackets)})]) and
not(following-sibling::OP-COMMA)
]
")

Linter(linter_level = "expression", function(source_expression) {
xml <- source_expression$xml_parsed_content

bad_exprs <- xml_find_all(xml, xpath)
msgs <- sprintf("Use `[[` instead of `%s` to extract an element.", xml_text(bad_exprs))

xml_nodes_to_lints(
bad_exprs,
source_expression = source_expression,
lint_message = msgs,
type = "warning"
)
})
}
2 changes: 1 addition & 1 deletion inst/lintr/linters.csv
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ expect_s3_class_linter,package_development best_practices pkg_testthat
expect_s4_class_linter,package_development best_practices pkg_testthat
expect_true_false_linter,package_development best_practices readability pkg_testthat
expect_type_linter,package_development best_practices pkg_testthat
extraction_operator_linter,style best_practices
extraction_operator_linter,style best_practices deprecated
fixed_regex_linter,best_practices readability efficiency configurable regex
for_loop_index_linter,best_practices readability robustness
function_argument_linter,style consistency best_practices
Expand Down
1 change: 0 additions & 1 deletion man/best_practices_linters.Rd

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

1 change: 1 addition & 0 deletions man/deprecated_linters.Rd

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

62 changes: 0 additions & 62 deletions man/extraction_operator_linter.Rd

This file was deleted.

7 changes: 3 additions & 4 deletions man/linters.Rd

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

3 changes: 3 additions & 0 deletions man/lintr-deprecated.Rd

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

1 change: 0 additions & 1 deletion man/style_linters.Rd

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

15 changes: 13 additions & 2 deletions tests/testthat/test-extraction_operator_linter.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
test_that("extraction_operator_linter generates deprecation warning", {
expect_warning(
extraction_operator_linter(),
rex::rex("Linter extraction_operator_linter was deprecated")
)
})

test_that("extraction_operator_linter skips allowed usages", {
linter <- extraction_operator_linter()
expect_warning({
linter <- extraction_operator_linter()
})

expect_lint("x[[1]]", NULL, linter)
expect_lint("x[-1]", NULL, linter)
Expand All @@ -10,7 +19,9 @@ test_that("extraction_operator_linter skips allowed usages", {
})

test_that("extraction_operator_linter blocks disallowed usages", {
linter <- extraction_operator_linter()
expect_warning({
linter <- extraction_operator_linter()
})
msg_b <- rex::escape("Use `[[` instead of `[` to extract an element.")
msg_d <- rex::escape("Use `[[` instead of `$` to extract an element.")

Expand Down