False positive in extraction_operator_linter() with R6 class method calls

[IndrajeetPatil]

This shouldn't produce a lint:

library(lintr)

lint(
  text = "Person <- R6Class('Person', list(
          name = NULL,
          initialize = function(name) self$name <- name
          ))
  
  Bob <- Person$new('Bob')",
  linters = extraction_operator_linter()
)
#> <text>:6:16: warning: [extraction_operator_linter] Use `[[` instead of `$` to extract an element.
#>   Bob <- Person$new('Bob')
#>                ^

^{Created on 2022-07-29 by the reprex package (v2.0.1.9000)}

And, although the suggested replacement would work here, this would be an extremely unusual way of calling R6 class methods:

library(R6)

Person <- R6Class('Person', list(
  name = NULL,
  initialize = function(name) self$name <- name
))

Person[["new"]]('Bob')
#> <Person>
#>   Public:
#>     clone: function (deep = FALSE) 
#>     initialize: function (name) 
#>     name: Bob

^{Created on 2022-07-29 by the reprex package (v2.0.1.9000)}

[MichaelChirico]

IMO this linter is just fundamentally problematic -- it's why we disabled it for {lintr}:

lintr/.lintr

Line 17 in 6d2b327

extraction_operator_linter = NULL,

If it were up to me I'd deprecate this linter. But if you do use it, then yes, the awkward Person[["new"]]("Bob") is indeed the correct usage. I'm not even sure how we'd parameterize our way out of this (just allow $new?)...

[IndrajeetPatil]

Is deprecating this linter an option?! Please let's do that then.

This is problematic for R6, Shiny, and in so many other contexts. I haven't encountered a single codebase where I have run all linters and then decided to turn off extraction linter because it had produced so many false positives that attending to other, more relevant lints became a difficult task.

[AshesITR]

Maybe allow all extractions that contain a call?
The utility of this linter is questionable at the moment, I agree.

I would also support deprecation if we don't find relevant usages in the wild.

[MichaelChirico]

cc original author @fangly for whether we're missing something / he can see a good path forward for the linter. I'll also do a Mastodon post.

[IndrajeetPatil]

FWIW, maybe codegrip will provide a convenient way to do this conversion ($ -> [[) where relevant.

[Bisaloo]

Maybe allow all extractions that contain a call?

Another common awkward case that would be handled by excluding calls:

knitr::opts_chunk$set(
  echo = TRUE
)

That said, I've disabled this linter in our organization as well because I find the [[ syntax more difficult to read in many cases, so deprecating it sounds good to me as well.

[IndrajeetPatil]

Ditto!

I have turned off this linter in all personal repos and every organization I am part of.