Proposed linter for mixing logical operators without grouping

[MichaelChirico]

Consider:

x | y & z
# or
x & y | z

I think this is a common pitfall for users not well-versed in logical operator precedence rules (myself included -- I had to look up the precedence once again while writing this post 😸).

At least "&& evaluates before ||" appears consistent across languages from what I've seen; this Q&A hints that this order was chosen "to reduce the number of parentheses" (would love a citation on that), while also highlighting an interesting language (Fortress) which declines to set arbitrary operator precedences and instead enforces parentheses in any uncommon combinations:

https://softwareengineering.stackexchange.com/q/391263/221381

The above would lint unless the user adds parentheses:

x | (y & z)
(x & y) | z

would be the logical equivalents. This would also apply to && / ||.

Related: in the Google suite, we have a linter for !(x == 2) --> x != 2. To me, the precedence of ! is clear enough not to need extra parentheses when combined with &&/||, so I didn't include it here.

[IndrajeetPatil]

Love it! Would indeed be very useful, for the naive and the advanced users alike.

Related: in the Google suite, we have a linter for !(x == 2) --> x != 2. To me, the precedence of ! is clear enough not to need extra parentheses when combined with &&/||, so I didn't include it here.

The one place I have seen people really struggle to make sense of precedence is ! combined with %in%. Here, !(x %in% y) is definitely easier to reason about than !x %in% y.

[MichaelChirico]

i personally like the !x %in% tbl approach (especially in if() clauses where the added parens tend to clutter) but agree it would be a good linter to offer!

[MichaelChirico]

Related issue coming up in #1656:

---

Currently, the linter marks x %>% ... %>% length(.) > 0

This technically works as intended because it's parsed as `>`(x %>% ... %>% length(), 0) but I think the code is not quite behaving as intended.

x %>% ... %>% (length(.) > 0) is probably closer to what was intended. So for now we mark the code as a lint, but that also doesn't feel quite right (we're using this linter as a signal of a different code quality issue).

---

So perhaps this proposed linter could be something like operator_precedence_linter() that encourages grouping with () whenever operators across different "levels" of precedence are mixed without clearly denoting intended evaluation order with ()?