From 18d64a09dda122d6e722ed1fd13b63ba806df789 Mon Sep 17 00:00:00 2001 From: MrMallIronmaker <725mrm@gmail.com> Date: Fri, 5 Jun 2020 12:13:24 -0700 Subject: [PATCH 1/3] handle empty arguments in switch statements --- NAMESPACE | 1 + R/commas_linter.R | 26 +++++++++++++++++++++++--- tests/testthat/test-commas_linter.R | 29 +++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) 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/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..f86846bf8 100644 --- a/tests/testthat/test-commas_linter.R +++ b/tests/testthat/test-commas_linter.R @@ -43,4 +43,33 @@ 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("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) + }) From 41bbd0b596b412ce68ea7fa4b50a7baf9331ef7d Mon Sep 17 00:00:00 2001 From: MrMallIronmaker <725mrm@gmail.com> Date: Fri, 5 Jun 2020 12:29:55 -0700 Subject: [PATCH 2/3] ensure special switch treatment is restricted to named missing arguments --- tests/testthat/test-commas_linter.R | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/testthat/test-commas_linter.R b/tests/testthat/test-commas_linter.R index f86846bf8..8fafb255f 100644 --- a/tests/testthat/test-commas_linter.R +++ b/tests/testthat/test-commas_linter.R @@ -60,6 +60,14 @@ test_that("returns the correct linting", { 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) From 1490b142237f36960041d7d0da9ce00a37a04c1a Mon Sep 17 00:00:00 2001 From: MrMallIronmaker <725mrm@gmail.com> Date: Tue, 23 Jun 2020 11:32:02 -0700 Subject: [PATCH 3/3] update news --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 88d8b68d5..5dd5f3b10 100644 --- a/NEWS.md +++ b/NEWS.md @@ -23,6 +23,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