Commas trailing (#2104)

NEWS.md

@@ -40,6 +40,7 @@
 * `keyword_quote_linter()` for finding unnecessary or discouraged quoting of symbols in assignment, function arguments, or extraction (part of #884, @MichaelChirico). Quoting is unnecessary when the target is a valid R name, e.g. `c("a" = 1)` can be `c(a = 1)`. The same goes to assignment (`"a" <- 1`) and extraction (`x$"a"`). Where quoting is necessary, the linter encourages doing so with backticks (e.g. `` x$`a b` `` instead of `x$"a b"`).
 * `length_levels_linter()` for using the specific function `nlevels()` instead of checking `length(levels(x))` (part of #884, @MichaelChirico).
 * `if_not_else_linter()` for encouraging `if` statements to be structured as `if (A) x else y` instead of `if (!A) y else x` (part of #884, @MichaelChirico).
+* `commas_linter()` gains an option `allow_trailing` (default `FALSE`) to allow trailing commas while indexing. (#2104, @MEO265)
 
 ## Changes to defaults
 

R/commas_linter.R

@@ -2,6 +2,9 @@
 #'
 #' Check that all commas are followed by spaces, but do not have spaces before them.
 #'
+#' @param allow_trailing If `TRUE`, the linter allows a comma to be followed
+#' directly by a closing bracket without a space.
+#'
 #' @examples
 #' # will produce lints
 #' lint(
@@ -19,6 +22,11 @@
 #'   linters = commas_linter()
 #' )
 #'
+#' lint(
+#'   text = "x[1,]",
+#'   linters = commas_linter()
+#' )
+#'
 #' # okay
 #' lint(
 #'   text = "switch(op, x = foo, y = bar)",
@@ -40,12 +48,17 @@
 #'   linters = commas_linter()
 #' )
 #'
+#' lint(
+#'   text = "x[1,]",
+#'   linters = commas_linter(allow_trailing = TRUE)
+#' )
+#'
 #' @evalRd rd_tags("commas_linter")
 #' @seealso
 #' - [linters] for a complete list of linters available in lintr.
 #' - <https://style.tidyverse.org/syntax.html#commas>
 #' @export
-commas_linter <- function() {
+commas_linter <- function(allow_trailing = FALSE) {
   # conditions are in carefully-chosen order for performance --
   #   an expression like c(a,b,c,....) with many elements can have
   #   a huge number of preceding-siblings and the performance of
@@ -58,7 +71,11 @@ commas_linter <- function() {
     @line1 = preceding-sibling::*[1]/@line1 and
     not(preceding-sibling::*[1][self::OP-COMMA or self::EQ_SUB])
   ]"
-  xpath_after <- "//OP-COMMA[@line1 = following-sibling::*[1]/@line1 and @col1 = following-sibling::*[1]/@col1 - 1]"
+  xpath_after <- paste0(
+    "//OP-COMMA[@line1 = following-sibling::*[1]/@line1 and @col1 = following-sibling::*[1]/@col1 - 1 ",
+    if (allow_trailing) "and not(following-sibling::*[1][self::OP-RIGHT-BRACKET or self::RBB or self::OP-RIGHT-PAREN])",
+    "]"
+  )
 
   Linter(function(source_expression) {
     if (!is_lint_level(source_expression, "expression")) {

inst/lintr/linters.csv

@@ -8,7 +8,7 @@ boolean_arithmetic_linter,efficiency best_practices readability
 brace_linter,style readability default configurable
 class_equals_linter,best_practices robustness consistency
 closed_curly_linter,style readability deprecated configurable
-commas_linter,style readability default
+commas_linter,style readability default configurable
 commented_code_linter,style readability best_practices default
 condition_message_linter,best_practices consistency
 conjunct_test_linter,package_development best_practices readability configurable pkg_testthat

tests/testthat/test-commas_linter.R

@@ -1,4 +1,4 @@
-test_that("returns the correct linting", {
+test_that("returns the correct linting (with default parameters)", {
   linter <- commas_linter()
   msg_after <- rex::rex("Commas should always have a space after.")
   msg_before <- rex::rex("Commas should never have a space before.")
@@ -13,6 +13,9 @@ test_that("returns the correct linting", {
   expect_lint("fun(1\n,1)", msg_after, linter)
   expect_lint("fun(1,1)", msg_after, linter)
   expect_lint("\nfun(1,1)", msg_after, linter)
+  expect_lint("a(1,)", msg_after, linter)
+  expect_lint("a[1,]", msg_after, linter)
+  expect_lint("a[[1,]]", msg_after, linter)
   expect_lint(
     "fun(1 ,1)",
     list(
@@ -46,3 +49,55 @@ test_that("returns the correct linting", {
     linter
   )
 })
+
+test_that("returns the correct linting (with 'allow_trailing' set)", {
+  linter <- commas_linter(allow_trailing = TRUE)
+  msg_after <- rex::rex("Commas should always have a space after.")
+  msg_before <- rex::rex("Commas should never have a space before.")
+
+  expect_lint("blah", NULL, linter)
+  expect_lint("fun(1, 1)", NULL, linter)
+  expect_lint("fun(1,\n  1)", NULL, linter)
+  expect_lint("fun(1,\n1)", NULL, linter)
+  expect_lint("fun(1\n,\n1)", NULL, linter)
+  expect_lint("fun(1\n  ,\n1)", NULL, linter)
+  expect_lint("a[1,]", NULL, linter)
+  expect_lint("a(1,)", NULL, linter)
+
+  expect_lint("fun(1\n,1)", msg_after, linter)
+  expect_lint("fun(1,1)", msg_after, linter)
+  expect_lint("\nfun(1,1)", msg_after, linter)
+  expect_lint(
+    "fun(1 ,1)",
+    list(
+      msg_before,
+      msg_after
+    ),
+    linter
+  )
+
+  expect_lint("\"fun(1 ,1)\"", NULL, linter)
+  expect_lint("a[1, , 2]", NULL, linter)
+  expect_lint("a[1, , 2, , 3]", NULL, linter)
+  expect_lint("a[[1,]]", NULL, linter)
+
+  expect_lint("switch(op, x = foo, y = bar)", NULL, linter)
+  expect_lint("switch(op, x = , y = bar)", NULL, linter)
+  expect_lint("switch(op, \"x\" = , y = bar)", NULL, linter)
+  expect_lint("switch(op, x = ,\ny = bar)", NULL, linter)
+
+  expect_lint("switch(op, x = foo , y = bar)", msg_before, linter)
+  expect_lint("switch(op, x = foo , y = bar)", msg_before, linter)
+  expect_lint("switch(op , x = foo, y = bar)", msg_before, linter)
+  expect_lint("switch(op, x = foo, y = bar(a = 4 , b = 5))", msg_before, linter)
+  expect_lint("fun(op, x = foo , y = switch(bar, a = 4, b = 5))", msg_before, linter)
+
+  expect_lint(
+    "fun(op    ,bar)",
+    list(
+      list(message = msg_before, column_number = 7L, ranges = list(c(7L, 10L))),
+      list(message = msg_after, column_number = 12L, ranges = list(c(12L, 12L)))
+    ),
+    linter
+  )
+})