Deprecate extraction_operator_linter()

DESCRIPTION

@@ -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'

NEWS.md

@@ -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 microoptimization benefits, it can lead to non-idiomatic code in many contexts (e.g. R6 classes, Shiny applications, etc.) (#2409, @IndrajeetPatil).
 
 ## Bug fixes
 

R/lintr-deprecated.R

@@ -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"
+    )
+  })
+}

inst/lintr/linters.csv

@@ -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

tests/testthat/test-extraction_operator_linter.R

@@ -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)
@@ -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.")