Skip to content

Commit c9b3408

Browse files
authored
fix nested tidy function call indentation (#1948)
1 parent c6099d5 commit c9b3408

File tree

3 files changed

+64
-2
lines changed

3 files changed

+64
-2
lines changed

NEWS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@
137137

138138
* `routine_registration_linter()` for identifying native routines that don't use registration (`useDynLib` in the `NAMESPACE`; @MichaelChirico)
139139

140-
* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792).
140+
* `indentation_linter()` for checking that the indentation conforms to 2-space Tidyverse-style (@AshesITR and @dgkf, #1411, #1792, #1898).
141141

142142
* `unnecessary_nested_if_linter()` for checking unnecessary nested `if` statements where a single
143143
`if` statement with appropriate conditional expression would suffice (@IndrajeetPatil and @AshesITR, #1778).

R/indentation_linter.R

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,9 @@ indentation_linter <- function(indent = 2L, hanging_indent_style = c("tidy", "al
298298
}
299299

300300
find_new_indent <- function(current_indent, change_type, indent, hanging_indent) {
301-
if (change_type == "hanging") {
301+
if (change_type == "suppress") {
302+
current_indent
303+
} else if (change_type == "hanging") {
302304
hanging_indent
303305
} else if (change_type == "double") {
304306
current_indent + 2L * indent
@@ -311,6 +313,7 @@ build_indentation_style_tidy <- function() {
311313
paren_tokens_left <- c("OP-LEFT-BRACE", "OP-LEFT-PAREN", "OP-LEFT-BRACKET", "LBB")
312314
paren_tokens_right <- c("OP-RIGHT-BRACE", "OP-RIGHT-PAREN", "OP-RIGHT-BRACKET", "OP-RIGHT-BRACKET")
313315
xp_last_on_line <- "@line1 != following-sibling::*[not(self::COMMENT)][1]/@line1"
316+
xp_inner_expr <- "preceding-sibling::*[1][self::expr and expr[SYMBOL_FUNCTION_CALL]]/*[not(self::COMMENT)]"
314317

315318
# double indent is tidyverse style for function definitions
316319
# 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() {
333336
parent::expr[FUNCTION and not(@line1 = SYMBOL_FORMALS/@line1)]
334337
/OP-RIGHT-PAREN[@line1 = preceding-sibling::*[not(self::COMMENT)][1]/@line2]
335338
"
339+
340+
xp_suppress <- paste(
341+
glue::glue("
342+
self::{paren_tokens_left}[
343+
@line1 = following-sibling::{paren_tokens_right}/{xp_inner_expr}[position() = 1]/@line1
344+
]/following-sibling::{paren_tokens_right}[
345+
@line1 > {xp_inner_expr}[position() = last() - 1]/@line2
346+
]"
347+
),
348+
collapse = " | "
349+
)
350+
336351
xp_is_not_hanging <- paste(
337352
c(
338353
glue::glue(
@@ -346,6 +361,8 @@ build_indentation_style_tidy <- function() {
346361
function(change) {
347362
if (length(xml2::xml_find_first(change, xp_is_double_indent)) > 0L) {
348363
"double"
364+
} else if (length(xml2::xml_find_first(change, xp_suppress)) > 0L) {
365+
"suppress"
349366
} else if (length(xml2::xml_find_first(change, xp_is_not_hanging)) == 0L) {
350367
"hanging"
351368
} else {

tests/testthat/test-indentation_linter.R

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,51 @@ test_that("hanging_indent_stlye works", {
632632
expect_lint(code_hanging_same_line, NULL, tidy_linter)
633633
expect_lint(code_hanging_same_line, NULL, hanging_linter)
634634
expect_lint(code_hanging_same_line, "Indent", non_hanging_linter)
635+
636+
# regression test for #1898
637+
expect_lint(
638+
trim_some("
639+
outer_fun(inner_fun(x,
640+
one_indent = 42L
641+
))
642+
"),
643+
NULL,
644+
tidy_linter
645+
)
646+
647+
expect_lint(
648+
trim_some("
649+
outer_fun(inner_fun(x, # this is first arg
650+
one_indent = 42L # this is second arg
651+
))
652+
"),
653+
NULL,
654+
tidy_linter
655+
)
656+
657+
expect_lint(
658+
trim_some("
659+
outer_fun(inner_fun(
660+
x,
661+
one_indent = 42L
662+
))
663+
"),
664+
NULL,
665+
tidy_linter
666+
)
667+
668+
expect_lint(
669+
trim_some("
670+
outer_fun(
671+
inner_fun(
672+
x,
673+
one_indent = 42L
674+
)
675+
)
676+
"),
677+
NULL,
678+
tidy_linter
679+
)
635680
})
636681

637682
test_that("assignment_as_infix works", {

0 commit comments

Comments
 (0)