Extend scalar_in_linter() for other clear scalar RHS

[MichaelChirico]

Follow-up to #2084

Some cases to consider as lints:

x %in% c(1)
x %in% (1)
x %in% (1 + 3i)
x %in% c(1 + 3i)

[AshesITR]

The c() cases are handled by unneeded concatenation linter, no?

[MichaelChirico]

Yes, but if easy to do we should try and decouple the linters where possible. Not the highest priority of course.

We don't necessarily have to replicate the fully unnecessary_concatenation_linter() logic, just a quick check for c() with one *CONST

[AshesITR]

I would argue in favor of generating less lints on the same code, especially with default linters involved.

[MichaelChirico]

The flip side of it is chained lints, where you fix a lint (c(1) -> 1) and now a new lint pops up.

We definitely have other examples of linters that throw in the presence of would-be lints... the first thing that comes to mind is handling of RIGHT_ASSIGN, e.g.

grep -Er "^[^#]*RIGHT_ASSIGN" R --exclude=assignment_linter.R --exclude=shared_constants.R

R/T_and_F_symbol_linter.R:    "parent::expr[following-sibling::LEFT_ASSIGN or preceding-sibling::RIGHT_ASSIGN or following-sibling::EQ_ASSIGN]"
R/empty_assignment_linter.R:      or following-sibling::RIGHT_ASSIGN
R/regex_subset_linter.R:        and not(parent::*[LEFT_ASSIGN or EQ_ASSIGN or RIGHT_ASSIGN])
R/implicit_assignment_linter.R:    "RIGHT_ASSIGN" # e.g. mean(1:4 -> x)
R/keyword_quote_linter.R:  | //RIGHT_ASSIGN/following-sibling::expr[{ assignment_candidate_cond }]
R/function_return_linter.R:    /parent::expr/parent::expr/expr[LEFT_ASSIGN or RIGHT_ASSIGN]

So linting here would be consistent with that.

[AshesITR]

All right.

[MichaelChirico]

Copying over from the PR:

---

We could lint x %in% NA though to suggest is.na(x).

I'd make that a message customization as a follow-up

Keep in mind that will generate incorrect advice since the behavior of == differs from %in% there. It would be nice to restrict this linter to only this case btw because x %in% NA vs. is.na(x) is more clearly a lint IMO. The logic should be quite simple - extract the constant as an R value and test it using is.na() or just hard-code the possible NAs, maybe offering capabilities for extension to NAs from other packages such as lubridate::NA_Date_.

[AshesITR]

API suggestion

scalar_in_linter(use_equals = FALSE, na_values = c("NA", "NA_integer_", "NA_real_", "NA_character_"))

Where use_equals = TRUE turns on lints for x %in% 1 and na_values controls the symbols linted to suggest is.na(x) instead.

I have no strong preference on the default value of use_equals unless we want to make it a default linter.