Add pipe consistency linter

[bairdj]

New linter to check for the consistent use of pipe operators - either the magrittr %>% or the native |>.

This linter is quite simple - it counts the occurrences of each operator, and if both of the counts are non-zero, it generates lints. The lints are generated at each occurrence of the pipe operator.

Fixes #1052

[MichaelChirico]

Thanks for the PR!

I first have a high-level concern. the design here is slightly different than quotes_linter(), which defaults to enforcing a specific quote character instead of consistency.

I think that makes sense given the current state of the style guide (which doesn't offer a preference for any particular pipe). But what about if/when (see tidyverse/style#188) in the future the style guide becomes more opinionated about pipes?

I think it may make more sense to name this pipes_linter(), which makes the design similar to quotes_linter(). Then we can make the parameter (pipe?) have three options, "consistency" (naming?), "%>%", '|>'. We can also add the sentinel for consistency to quotes_linter() to allow authors to choose "I don't care what quote character, as long as it's file-level consistent".

CC @IndrajeetPatil @AshesITR for design insights

[bairdj]

Hi,

I think this is a good idea. I did think about implementing a preference argument in this (I suspect many organisations already have internal style preferences for pipes), but didn't as I thought it might be better as a separate linter as it would be a slightly different rule it's enforcing. However, your suggestion covers all use cases to do with pipes. It would make sense to make quotes_linter match the consistency pattern too

[AshesITR]

Agreed, this seems like a good idea to control via argument.

The automatic option could be pipe = "auto" and used as default for now.

[codecov-commenter]

Codecov Report

Merging #2018 (cc2cfc2) into main (8d59c57) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head cc2cfc2 differs from pull request most recent head b21e686. Consider uploading reports for the commit b21e686 to get more accurate results

@@           Coverage Diff           @@
##             main    #2018   +/-   ##
=======================================
  Coverage   99.64%   99.65%           
=======================================
  Files         112      113    +1     
  Lines        5134     5166   +32     
=======================================
+ Hits         5116     5148   +32     
  Misses         18       18           

Files Changed	Coverage Δ
R/pipe_consistency_linter.R	100.00% <100.00%> (ø)

[AshesITR]

Well done. Looks good to me, just not sure about the wording in the docs. @MichaelChirico PTAL and feel free to merge.

[bairdj]

I have added the support for the other magrittr types. One issue that this raises is the lint messages, as these refer specifically to %>%. When |> is used as the pipe argument, the lint message is "Use the |> pipe operator instead of the %>% pipe operator." - this could be misleading as %>% wasn't necessarily used, nor may the other pipe be interchangeable with |>.

Does extra handling need to be added to identify other magrittr type use, and show appropriate messages?

[AshesITR]

Since pipe = "|>" is a conscious choice, I would keep it simple.
I'm not seeing the other magrittr pipes used that often, except for maybe %<>%, and even for that I wouldn't deem a custom message necessary.

[MichaelChirico]

Hi @bairdj, we're closing in on sealing up a release (lintr 3.1.1). If you'd like this PR included, please take a chance to wrap up. If you'd like it included and won't have time to work on it in the next few weeks, let us know and we can try taking over from here. Thanks!

[bairdj]

Hi @MichaelChirico, I think everything has been addressed so this should be ready to go