diff --git a/NAMESPACE b/NAMESPACE index 3b384b85a..839c583d7 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -67,6 +67,7 @@ importFrom(cyclocomp,cyclocomp) importFrom(stats,na.omit) importFrom(utils,capture.output) importFrom(utils,getParseData) +importFrom(utils,head) importFrom(utils,relist) importFrom(utils,tail) importFrom(xml2,as_list) diff --git a/NEWS.md b/NEWS.md index fa13028ef..2aaa463a5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -25,6 +25,8 @@ * `object_usage_linter` has been changed to ensure lint-position is indicated relative to the start of the file, rather than the start of a defining function (#432, @russHyde). +* `commas_linter` now allows spaces to come before a comma when used to denote a + fall-through in a switch statement (#499, @MrMallIronmaker) # lintr 2.0.0 diff --git a/R/commas_linter.R b/R/commas_linter.R index fba60174e..e56d4a491 100644 --- a/R/commas_linter.R +++ b/R/commas_linter.R @@ -1,5 +1,6 @@ #' @describeIn linters check that all commas are followed by spaces, but do not #' have spaces before them. +#' @importFrom utils head #' @export commas_linter <- function(source_file) { @@ -26,14 +27,33 @@ commas_linter <- function(source_file) { if (space_before) { - has_token <- any(source_file$parsed_content$line1 == line_number & - source_file$parsed_content$col1 == comma_loc & + comma_loc_filter <- source_file$parsed_content$line1 == line_number & + source_file$parsed_content$col1 == comma_loc + + has_token <- any(comma_loc_filter & source_file$parsed_content$token == "','") start_of_line <- re_matches(line, rex(start, spaces, ",")) empty_comma <- substr(line, comma_loc - 2L, comma_loc - 1L) %==% ", " - if (has_token && !start_of_line && !empty_comma) { + + parent <- source_file$parsed_content$parent + parent <- replace(parent, parent==0, NA) + + # a variable that is true for every node who has a grandchild that is switch, + # i.e, any expression that starts with the function call to switch. + switch_grandparents <- source_file$parsed_content[ + # as.character allows interpretation as row indexes rather than row numbers + as.character(parent[source_file$parsed_content$text == "switch"]), + ]$parent + + is_blank_switch <- any(comma_loc_filter & + (source_file$parsed_content$parent %in% switch_grandparents) & + c(NA, head(source_file$parsed_content$token, -1)) == "EQ_SUB", + na.rm=TRUE + ) + + if (has_token && !start_of_line && !empty_comma && !is_blank_switch) { lints[[length(lints) + 1L]] <- Lint( diff --git a/tests/testthat/test-commas_linter.R b/tests/testthat/test-commas_linter.R index 3dcad6a0e..8fafb255f 100644 --- a/tests/testthat/test-commas_linter.R +++ b/tests/testthat/test-commas_linter.R @@ -43,4 +43,41 @@ test_that("returns the correct linting", { expect_lint("a[1, , 2, , 3]", NULL, commas_linter) + + expect_lint("switch(op, x = foo, y = bar)", + NULL, + commas_linter) + + expect_lint("switch(op, x = foo , y = bar)", + rex("Commas should never have a space before."), + commas_linter) + + expect_lint("switch(op, x = , y = bar)", + NULL, + commas_linter) + + expect_lint("switch(op, \"x\" = , y = bar)", + NULL, + commas_linter) + + expect_lint("switch(op, x = foo , y = bar)", + rex("Commas should never have a space before."), + commas_linter) + + expect_lint("switch(op , x = foo, y = bar)", + rex("Commas should never have a space before."), + commas_linter) + + expect_lint("fun(op, x = foo , y = switch(bar, a = 4, b = 5))", + rex("Commas should never have a space before."), + commas_linter) + + expect_lint("switch(op, x = foo, y = bar(a = 4 , b = 5))", + rex("Commas should never have a space before."), + commas_linter) + + expect_lint("switch(op, x = ,\ny = bar)", + NULL, + commas_linter) + })