New linter for trailing assignments

[ashbaldry]

It would be nice to have a linter that flags whenever an assignment is at the end of the line. It can make function calls disjoined and harder to read.

Examples:

y <- mean(x, na.rm =
  TRUE
)

some_long_variable_name <-
  function_name(...)

[IndrajeetPatil]

@ashbaldry Can you please check that the assignment that tidyverse style guide deems to be okay (the first example below) doesn't trigger a lint with this new linter? Also, please add test for the same.

[ashbaldry]

Ah, I didn't see that section. Currently it will flag both the first and third situation. Let me add some tests in and see how to pass the situations above.

[ashbaldry]

@IndrajeetPatil I have included tests for all 3 versions and they are now all passing. Personally, I prefer to have the variable name and assignment on the same line so I have included a parameter that will flag the first and third situation, but by default it follows the tidyverse style. Hope that is alright, if not it is easy to remove.

[IndrajeetPatil]

Looks great, @ashbaldry! :)

library(lintr)

lint(
  text = "x <- 5",
  linters = trailing_assignment_linter()
)

lint(
  text = "x <-
  5",
  linters = trailing_assignment_linter()
)
#> <text>:1:1: style: [trailing_assignment_linter] Assignment `<-` should not be trailing at end of line
#> x <-
#> ^

lint(
  text = "y <- mean(x, na.rm =
            TRUE
          )",
  linters = trailing_assignment_linter()
)
#> <text>:1:1: style: [trailing_assignment_linter] Assignment `=` should not be trailing at end of line
#> y <- mean(x, na.rm =
#> ^

lint(
  text = "some_long_variable_name <-
  function_name(...)",
  linters = trailing_assignment_linter()
)
#> <text>:1:1: style: [trailing_assignment_linter] Assignment `<-` should not be trailing at end of line
#> some_long_variable_name <-
#> ^

lint(
  text = "iris_long <-
  iris %>%
  gather(measure, value, -Species) %>%
  arrange(-value)"
  ,
  linters = trailing_assignment_linter()
)

lint(
  text = "iris_long <- iris %>%
  gather(measure, value, -Species) %>%
  arrange(-value)",
  linters = trailing_assignment_linter()
)

lint(
  text = "iris %>%
  gather(measure, value, -Species) %>%
  arrange(-value) ->
  iris_long",
  linters = trailing_assignment_linter()
)

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

I think it looks good to me. Let's see if @MichaelChirico has any other concerns.

[AshesITR]

The third case is also triggered by assignment_linter(), so I suggest to extend the existing assignment linter by adding allow_trailing = TRUE to the mix, which would, if set to FALSE lint trailing assignment operators.
Otherwise, case 3 will produce two lints at the same location when using linters_with_defaults(trailing_assignment_linter()).

[MichaelChirico]

I suggest to extend the existing assignment linter by adding allow_trailing = TRUE

oh, great idea. my biggest hesitation here was about adding a new linter for something I'm not sure has a v. broad appeal. which opens the can of worms of what criteria we want to use to assess new linters for non-tidyverse rules.

much easier to agree to a new argument to an existing linter.

[ashbaldry]

Thanks for the feedback, I'll update the PR to add the argument to assignment_linter(), making sure that it doesn't affect behaviour when set to TRUE.
To try and keep it as consistent as I can with the tidyverse style, I'll ignore assignments at the start/end of a pipe (preventing further arguments), and just look at cases like the examples at the start of this issue

[AshesITR]

Sorry for being a bit unclear there. I think it's still valuable to look at all assignments regarding line breaks.
However, assignment_linter() disallows all usages of -> by default IIRC, so we only need to look for line breaks following any allowed assignment operator to avoid duplicate lints.