Can `expect_comparison_linter` drop the lint for `==`?

[TimTaylor]

The implementations of the expect_lt(), expect_lte() et al do eventually call the underlying comparison operator so it makes sense to suggest these where appropriate, see https://github.com/r-lib/testthat/blob/fa29578e09f9eeafc6b9822364e29b07c8bd7a8c/R/expect-comparison.R#L22-L63).

However all of the comparison operators are generics which means the suggestion of expect_identical() for == can easily be erroneous, e.g.

a <- b <- Sys.Date()
storage.mode(b) <- "integer"
testthat::expect_true(a == b)
lintr::lint(
    text = "expect_true(a == b)",
    linters = lintr::expect_comparison_linter()
)
#> <text>:1:1: warning: [expect_comparison_linter] expect_identical(x, y) is better than expect_true(x == y).
#> expect_true(a == b)
#> ^~~~~~~~~~~~~~~~~~~
testthat::expect_identical(a, b)
#> Error: `a` not identical to `b`.
#> Objects equal but not identical

My preference would be to remove the suggestion but, if you did want to keep this behaviour, would you consider adding an equality parameter? It could be something like:

expect_comparison_linter(equality = c("ignore", "expect_identical"))

[MichaelChirico]

I guess this is just a lint metadata problem.

{testthat} wants you to use expect_equal() here, not expect_identical(), c.f. https://github.com/r-lib/waldo/issues/215.

[TimTaylor]

That would be better but the generic nature of Ops still give me pause. It's somewhat blurring a lint for all.equal() usage. If the metadata gets updated to expect_equal() I think it would still be useful if the lint gained an escape hatch argument (although I'd now be a little less worried about the default), e.g

expect_comparison_linter(equality = c("expect_equal", "ignore"))