From f5997bacc8df49212841fbc0a810850eff163583 Mon Sep 17 00:00:00 2001 From: Alexander Rosenstock Date: Mon, 3 Apr 2023 23:28:09 +0200 Subject: [PATCH 1/3] fix nested tidy function call indentation --- NEWS.md | 2 +- R/indentation_linter.R | 19 ++++++++++++++++++- tests/testthat/test-indentation_linter.R | 11 +++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 16592f067a..e75222bcfc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -137,7 +137,7 @@ * `routine_registration_linter()` for identifying native routines that don't use registration (`useDynLib` in the `NAMESPACE`; @MichaelChirico) -* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792). +* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792, #1898). * `unnecessary_nested_if_linter()` for checking unnecessary nested `if` statements where a single `if` statement with appropriate conditional expression would suffice (@IndrajeetPatil and @AshesITR, #1778). diff --git a/R/indentation_linter.R b/R/indentation_linter.R index 7d4e9c8557..1f88223072 100644 --- a/R/indentation_linter.R +++ b/R/indentation_linter.R @@ -298,7 +298,9 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al } find_new_indent <- function(current_indent, change_type, indent, hanging_indent) { - if (change_type == "hanging") { + if (change_type == "suppress") { + current_indent + } else if (change_type == "hanging") { hanging_indent } else if (change_type == "double") { current_indent + 2L * indent @@ -311,6 +313,7 @@ build_indentation_style_tidy <- function() { paren_tokens_left <- c("OP-LEFT-BRACE", "OP-LEFT-PAREN", "OP-LEFT-BRACKET", "LBB") paren_tokens_right <- c("OP-RIGHT-BRACE", "OP-RIGHT-PAREN", "OP-RIGHT-BRACKET", "OP-RIGHT-BRACKET") xp_last_on_line <- "@line1 != following-sibling::*[not(self::COMMENT)][1]/@line1" + xp_inner_expr <- "preceding-sibling::*[1][self::expr]/*[not(self::COMMENT)]" # double indent is tidyverse style for function definitions # triggered only if the closing parenthesis of the function definition is not on its own line and the opening @@ -333,6 +336,18 @@ build_indentation_style_tidy <- function() { parent::expr[FUNCTION and not(@line1 = SYMBOL_FORMALS/@line1)] /OP-RIGHT-PAREN[@line1 = preceding-sibling::*[not(self::COMMENT)][1]/@line2] " + + xp_suppress <- paste( + glue::glue(" + self::{paren_tokens_left}[ + @line1 = following-sibling::{paren_tokens_right}/{xp_inner_expr}[position() = 1]/@line1 + ]/following-sibling::{paren_tokens_right}[ + @line1 > {xp_inner_expr}[position() = last() - 1]/@line2 + ]" + ), + collapse = " | " + ) + xp_is_not_hanging <- paste( c( glue::glue( @@ -346,6 +361,8 @@ build_indentation_style_tidy <- function() { function(change) { if (length(xml2::xml_find_first(change, xp_is_double_indent)) > 0L) { "double" + } else if (length(xml2::xml_find_first(change, xp_suppress)) > 0L) { + "suppress" } else if (length(xml2::xml_find_first(change, xp_is_not_hanging)) == 0L) { "hanging" } else { diff --git a/tests/testthat/test-indentation_linter.R b/tests/testthat/test-indentation_linter.R index 12bb4a7a25..a95daeaa9d 100644 --- a/tests/testthat/test-indentation_linter.R +++ b/tests/testthat/test-indentation_linter.R @@ -632,6 +632,17 @@ test_that("hanging_indent_stlye works", { expect_lint(code_hanging_same_line, NULL, tidy_linter) expect_lint(code_hanging_same_line, NULL, hanging_linter) expect_lint(code_hanging_same_line, "Indent", non_hanging_linter) + + # regression test for #1898 + expect_lint( + trim_some(" + outer_fun(inner_fun(x, + one_indent = 42L + )) + "), + NULL, + tidy_linter + ) }) test_that("assignment_as_infix works", { From 37d6bb3ce8c707361b0ed21fa8849c2adca00219 Mon Sep 17 00:00:00 2001 From: Alexander Rosenstock Date: Mon, 3 Apr 2023 23:41:19 +0200 Subject: [PATCH 2/3] restrict suppression to function calls for now --- R/indentation_linter.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/indentation_linter.R b/R/indentation_linter.R index 1f88223072..9f98a6e2d8 100644 --- a/R/indentation_linter.R +++ b/R/indentation_linter.R @@ -313,7 +313,7 @@ build_indentation_style_tidy <- function() { paren_tokens_left <- c("OP-LEFT-BRACE", "OP-LEFT-PAREN", "OP-LEFT-BRACKET", "LBB") paren_tokens_right <- c("OP-RIGHT-BRACE", "OP-RIGHT-PAREN", "OP-RIGHT-BRACKET", "OP-RIGHT-BRACKET") xp_last_on_line <- "@line1 != following-sibling::*[not(self::COMMENT)][1]/@line1" - xp_inner_expr <- "preceding-sibling::*[1][self::expr]/*[not(self::COMMENT)]" + xp_inner_expr <- "preceding-sibling::*[1][self::expr and expr[SYMBOL_FUNCTION_CALL]]/*[not(self::COMMENT)]" # double indent is tidyverse style for function definitions # triggered only if the closing parenthesis of the function definition is not on its own line and the opening From 0bf4029f5969d512966ae36508ba5c4e9738ec68 Mon Sep 17 00:00:00 2001 From: Alexander Rosenstock Date: Tue, 4 Apr 2023 10:37:59 +0200 Subject: [PATCH 3/3] more thorough tests --- tests/testthat/test-indentation_linter.R | 34 ++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/testthat/test-indentation_linter.R b/tests/testthat/test-indentation_linter.R index a95daeaa9d..6d271a7574 100644 --- a/tests/testthat/test-indentation_linter.R +++ b/tests/testthat/test-indentation_linter.R @@ -643,6 +643,40 @@ test_that("hanging_indent_stlye works", { NULL, tidy_linter ) + + expect_lint( + trim_some(" + outer_fun(inner_fun(x, # this is first arg + one_indent = 42L # this is second arg + )) + "), + NULL, + tidy_linter + ) + + expect_lint( + trim_some(" + outer_fun(inner_fun( + x, + one_indent = 42L + )) + "), + NULL, + tidy_linter + ) + + expect_lint( + trim_some(" + outer_fun( + inner_fun( + x, + one_indent = 42L + ) + ) + "), + NULL, + tidy_linter + ) }) test_that("assignment_as_infix works", {