implicit_integer_linter(allow_colon=TRUE) ignores negative inputs

NEWS.md

@@ -14,6 +14,10 @@
 
 * `brace_linter()`' has a new argument `function_bodies` (default `"multi_line"`) which controls when to require function bodies to be wrapped in curly braces, with the options `"always"`, `"multi_line"` (only require curly braces when a function body spans multiple lines), `"not_inline"` (only require curly braces when a function body starts on a new line) and `"never"` (#1807, #2240, @salim-b).
 
+### Lint accuracy fixes: removing false positives
+
+* `implicit_integer_linter(allow_colon = TRUE)` is OK with negative literals, e.g. `-1:1` or `1:-1` (#2673, @MichaelChirico).
+
 ## Notes
 
 * `expect_lint_free()` and other functions that rely on the {testthat} framework now have a consistent error message. (#2585, @F-Noelle).

R/implicit_integer_linter.R

@@ -47,7 +47,10 @@
 #' @export
 implicit_integer_linter <- function(allow_colon = FALSE) {
   if (allow_colon) {
-    xpath <- "//NUM_CONST[not(parent::expr/parent::expr/OP-COLON)]"
+    xpath <- "//NUM_CONST[not(parent::expr/parent::expr[
+      OP-COLON
+      or (OP-MINUS and parent::expr/OP-COLON)
+    ])]"
   } else {
     xpath <- "//NUM_CONST"
   }

tests/testthat/test-implicit_integer_linter.R

@@ -85,22 +85,31 @@ test_that("linter returns the correct linting", {
 })
 
 skip_if_not_installed("tibble")
-patrick::with_parameters_test_that(
-  "numbers in a:b input are optionally not linted",
-  expect_lint(
-    paste0(left, ":", right),
-    if (n_lints > 0L) rep(list(rex::rex("Use 1L or 1.0")), n_lints),
-    implicit_integer_linter(allow_colon = allow_colon)
-  ),
-  .cases = tibble::tribble(
-    ~left,  ~right, ~n_lints, ~allow_colon, ~.test_name,
-    "1",    "1",    2L,       FALSE,        "1:1, !allow_colon",
-    "1",    "1",    0L,       TRUE,         "1:1, allow_colon",
-    "1",    "1L",   1L,       FALSE,        "1:1L, !allow_colon",
-    "1",    "1L",   0L,       TRUE,         "1:1L, allow_colon",
-    "1L",   "1",    1L,       FALSE,        "1L:1, !allow_colon",
-    "1L",   "1",    0L,       TRUE,         "1L:1, allow_colon",
-    "1L",   "1L",   0L,       FALSE,        "1L:1L, !allow_colon",
-    "1L",   "1L",   0L,       TRUE,         "1L:1L, allow_colon"
+local({
+  .cases <- expand.grid(
+    left = c("1", "-1", "1L", "-1L"),
+    right = c("1", "-1", "1L", "-1L"),
+    allow_colon = c(FALSE, TRUE),
+    stringsAsFactors = FALSE
+  )
+  .cases <- within(.cases, {
+    left_is_double <- !endsWith(left, "L")
+    right_is_double <- !endsWith(right, "L")
+    n_lints <- (!allow_colon) * (left_is_double + right_is_double)
+    left_is_negative <- startsWith(left, "-")
+    right_is_negative <- startsWith(right, "-")
+    make_rex <- function(s) rex::rex("Use ", s, "L or ", s, ".0")
+    left_replacement <- vapply(left, make_rex, character(1L))
+    right_replacement <- vapply(right, make_rex, character(1L))
+    rm(make_rex)
+  })
+  patrick::with_parameters_test_that(
+    "Under allow_colon={allow_colon}, {left}:{right} throws {n_lints} lints",
+    expect_lint(
+      paste0(left, ":", right),
+      if (n_lints > 0L) c(if (left_is_double) list(left_replacement), if (right_is_double) list(right_replacement)),
+      implicit_integer_linter(allow_colon = allow_colon)
+    ),
+    .cases = .cases
   )
-)
+})