r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.2k stars 187 forks source link

False positive in pipe_continuation_linter when code has nested pipes #2631

Open javieriserte opened 4 months ago

javieriserte commented 4 months ago

When a piece of code has two pipes, one nested inside another like in example code below:

c("1", "2", "3", "4") |>
  sapply(function(x) x |> runif() |> mean()) |>
  as.character()

the linter produces a false positive:

<text>:2:46: style: [pipe_continuation_linter] `|>` should always have a space before it and a new line after it, unless the full pipeline fits on one line.
  sapply(function(x) x |> runif() |> mean()) |>
                                         ^~

I think that the linter may consider the expression like a single pipe, instead of two nested pipes.

I'm using lintr 3.1.2.

AshesITR commented 3 months ago

Does the following work for you?

c("1", "2", "3", "4") |>
  sapply(
    function(x) x |> runif() |> mean()
  ) |>
  as.character()

I think linting he inner pipeline could be made optional. Personally I prefer keeping the lint because regardless of syntax, discerning the multiple pipe characters in one line is hard on my eyes.

MichaelChirico commented 3 months ago

discerning the multiple pipe characters in one line is hard on my eyes.

I agree, but for this we already have https://github.com/r-lib/lintr/blob/73e55d31f6ead2908fcd9a49d6ccf46524b1cccc/R/nested_pipe_linter.R

I'll mark this as "help wanted" as I'm not sure it's worth the effort, but if this is driving you nuts feel free to have a go at it.

javieriserte commented 3 months ago

Thank you!,

I'll look into it.

AshesITR commented 3 months ago

Keeping this open to track the false positive