Remove false negatives with seq() in seq_linter()

[IndrajeetPatil]

Closes #1467

TODO

• Get rid of false negative
• Tests should pass
• Correct lint message should be produced
• Update documentation (needs bumping roxygen2 version)
• Address code review comments

Example

library(lintr)

lint(
  text = "seq(length(mtcars))",
  linters = seq_linter()
)
#> <text>:1:1: warning: [seq_linter] seq(length(...)) is likely to be wrong in the empty edge case. Use seq_along(...) instead.
#> seq(length(mtcars))
#> ^~~~~~~~~~~~~~~~~~~

lint(
  text = "seq(nrow(mtcars))",
  linters = seq_linter()
)
#> <text>:1:1: warning: [seq_linter] seq(nrow(...)) is likely to be wrong in the empty edge case. Use seq_len(...) instead.
#> seq(nrow(mtcars))
#> ^~~~~~~~~~~~~~~~~

^{Created on 2022-07-24 by the reprex package (v2.0.1)}

[IndrajeetPatil]

@MichaelChirico I think I've addressed all of your comments. So this should be all set.

[MichaelChirico]

could you add a test for seq(dim(x)[1]) which is how dim would usually be used? it's pretty rare (never seen it before myself), so if the current approach doesn't work, just file a follow-up issue.

but if the current xpath is good enough just add tests to prevent regression

[IndrajeetPatil]

if the current xpath is good enough just add tests to prevent regression

The current approach does work. Added a test.

[IndrajeetPatil]

Oh, wait. This should actually produce a lint:

seq(dim(data.frame())[1])
#> [1] 1 0

^{Created on 2022-07-26 by the reprex package (v2.0.1)}

[MichaelChirico]

right... don't worry about it though. just please file a follow-up issue

[MichaelChirico]

and plz revert the incorrect test :)

[IndrajeetPatil]

Is it okay if I add the correct test but comment it out for now?

[IndrajeetPatil]

Created issue to track this: #1474

[MichaelChirico]

yep that's fine, though we'll have to be wary of comment_linter

[MichaelChirico]

see :)