implicit_assignment_linter misses non-call assignments #2138

Activity

1. MichaelChirico

   added

   false-negativecode that should lint, but doesn'tcode that should lint, but doesn't

   on Sep 11, 2023
2. MichaelChirico

   added this to the 3.1.1 milestone on Sep 11, 2023
3. MichaelChirico commented on Sep 11, 2023

   

   MichaelChirico

   on Sep 11, 2023

   Last edited by MichaelChirico

   CollaboratorAuthor

   More actions

   There are two existing tests that would be affected by the simplest fix:

   lintr/tests/testthat/test-implicit_assignment_linter.R

   Line 84 in 8ac582c

   .f = ~ (x <- .x + 1)

   lintr/tests/testthat/test-implicit_assignment_linter.R

   Line 295 in 8ac582c

   expect_lint("mean(x <- (y <- 1:3) + 1L)", lint_message, linter)

   The latter looks like it should be two lints to me.

   The former is tougher. Should we carve out an exception for {purrr} mappers? OTOH this is like a violation of function_return_linter() -- why have an assignment in a one-line function? If it is <<- (which is also covered by //LEFT_ASSIGN), it's more interesting, but we could still force { to be used instead:

   x <- 1
   map(
     .x = 1:4,
     .f = ~ {x <<- .x + x}
   )

   Of course, that's a brace_linter() violation now, so it would have to be:

   x <- 1
   map(
     .x = 1:4,
     .f = ~ {
       x <<- .x + x
     }
   )

   cc @IndrajeetPatil
4. MichaelChirico

   closed this as completedin #2139on Sep 13, 2023